would it be possible for Parinfer to be a standalone executable that other editors can call out to?
@rgdelato I don’t see why not - do you have a use case in mind?
I guess you’d pass it a file path to be processed, and it could return the edits as we discussed above?
not particularly, just that it would mean that there wouldn't have to be ports of the code to different langs. write it once in something fast
Depending on the editor, it’s not always that easy to shell out to something.
Or at least, to get a copy of the document efficiently. I guess you could always save + apply it, but that would mean that the user’s doc would be being saved continuously.
yeah, that was what I was worried about. okay.
Unless you write out a temp file, and then it doesn’t matter how fast your algorithm is 🙂
But I think having the reference impl in something that’s more amenable to either manual or automatic translation might be a good step worth investigating.
So I have a draft of the change to process the whole file and apply edits. Most of the indent mode tests work now, I’m gradually working through the failures. Hopefully I’ll have something to show tomorrow.
If I get stuck debugging any of the problems I’ll push a WIP.
thanks!
for parinfer.js?
Right.
can’t wait to see it
Actually I can push a WIP now if you’re keen - give me 5 mins or so.
👍
this would help parinfer reach notepad++ as well
chris called it the most popular editor in the world
According to StackOverflow it is, yeah
@shaunlebron https://github.com/cursive-ide/parinfer/commit/2a50b2ee3fd26465853f6a0448ced95095c975f4
There’s still a fair amount of tidying up to do, including things like handling \r\n
correctly and probably revising some of the names (origText -> text etc)
Most of the indent mode tests pass now, I haven’t looked at the paren or smart mode ones yet but a reasonable number of them pass too.
So hopefully the approach is basically sound and there are probably just some bugs lurking there.
I have to head out now, but if you have questions let me know and I’ll try to get to them later this evening.
I’m also planning to revise the edit accumulation and application to the result text, I’m not sure that catches all cases when e.g. edits overlap.
Ok, GTG
thanks! did a quick lookover—not a lot of changes!
No, it’s relatively small.
In my initial version I tried to use offset in more places rather than lineStart/x combos, but that made the change larger than it needed to be.
If this becomes the definitive version that might be something to consider.
I’m hopeful that the remaining bugs are relatively straightforward.
Seems likely, given that a large chunk of the tests pass.
i’ll give it a more thorough look in the morning. i can probably fix the rest quickly. i’m pretty convinced this is the right move btw
thanks for keeping the diff small, very helpful
There are probably some edge cases to consider around the processing at EOF if the file doesn’t end in \n
too - some of the processing that was at the end of processLine
is now in onNewline
and also in finalizeResult
. If those calls are not idempotent there might need to be some more state there to track that.
i saw some of the calls shifted around for that—did it help this?
Did it help what, sorry?
oh I mean was it important to this edits feature to move it around?
gonna get some sleep and try in the morning
Cool, GTG too, talk tomorrow
streaming my work on it now
current issue is that the edit from correctParenTrail seems to be interfering with previous edits that remove unmatched close-parens on the spot
Ok, can’t really get on now (looking after my daughter) but the issue might be in how the edits are applied in resultText
np, streams are recorded if you wanna look back
just helps me focus too
Ok, will be around in an hour or two
👍
having trouble picking a starting point
i fixed one test and broke all the others
going to try to keep the same processLine logic to be sure that stuff is happening in the right order
wanna try replacing inputLines iteration with origText iteration first
then changing replaceWithinLine to append an edit, then building result text
will help me isolate what went wrong
@shaunlebron Sorry, that took longer than expected - had to take the cat to the vet
I’ll pull in your changes and then take a look.
np
changes are not ready. was starting over later to things more incrementally
@shaunlebron Have you ever tried floobits?
We could give that a go if you have some more time to look at it later.
i tried installing it for atom and it never initialized correctly
i can try it for another editor
I’ll sign up for a free plan and install it for IntelliJ
Looks like they have neovim
is anyone available for testing atom-parinfer on master?
the options screen isn’t showing up for chris, and I’d like to see if it’s working for others
I pulled master, restarted Atom, and I'm seeing the settings screen
thanks!
what version are you on?
1.18.0 x64 on OSX and Atom claims it's up to date
chris couldn’t get it to show up on 1.17.2
Or 1.17.0
and the config API between them seem the same
i wonder if people are upgrading their editors
so Chris is just seeing "Uninstall" and "Disable" buttons and no "Settings" button?
i installed 1.17.2 from github releases, and deleted 1.18.0
@chrisoakman can you send us a screenshot?
why is that showing the parinfer readme instead of the atom-parinfer readme
you are symlinking the package to the wrong directory
I was about to say, that Readme looks totally different from mine as well
I fixed whatever was going on with that. I still have no settings.
Do you see settings with v1.17.2?
the screenshot shows the version page above it
@chrisoakman make sure Parinfer
is capitalized
@shaunlebron Got a question when you have a minute
actually should it be capitalized?
config-key
should namespace the config keys to the case of the package I’m guessing
looks like it should be lowercased https://atom.io/packages/parinfer, not sure why I had it capitalized?
in general, it's kinda weird that it's capitalized since it puts Parinfer before all my other packages in alphabetical order
i’m not sure why i ever capitalized it. the readme instructions even give wrong instructions? shouldn’t it be ln -s <atom-parinfer dir> ~/.atom/packages/parinfer
?
I run apm link
from inside the atom-parinfer
directory
that explains the capitalization, the package.json name is “Parinfer”
submitting a PR
I already fixed it.
you renamed the package?