Fork me on GitHub
#parinfer
<
2017-08-07
>
rgdelato00:08:06

would it be possible for Parinfer to be a standalone executable that other editors can call out to?

cfleming00:08:42

@rgdelato I don’t see why not - do you have a use case in mind?

cfleming00:08:01

I guess you’d pass it a file path to be processed, and it could return the edits as we discussed above?

rgdelato00:08:37

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

cfleming00:08:45

Depending on the editor, it’s not always that easy to shell out to something.

cfleming00:08:24

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.

rgdelato00:08:37

yeah, that was what I was worried about. okay.

cfleming00:08:49

Unless you write out a temp file, and then it doesn’t matter how fast your algorithm is 🙂

cfleming00:08:50

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.

cfleming04:08:53

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.

cfleming04:08:26

If I get stuck debugging any of the problems I’ll push a WIP.

shaunlebron04:08:11

for parinfer.js?

shaunlebron04:08:28

can’t wait to see it

cfleming04:08:51

Actually I can push a WIP now if you’re keen - give me 5 mins or so.

shaunlebron04:08:52

this would help parinfer reach notepad++ as well

shaunlebron04:08:06

chris called it the most popular editor in the world

cfleming04:08:21

According to StackOverflow it is, yeah

cfleming05:08:58

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)

cfleming05:08:34

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.

cfleming05:08:59

So hopefully the approach is basically sound and there are probably just some bugs lurking there.

cfleming05:08:35

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.

cfleming05:08:55

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.

shaunlebron05:08:16

thanks! did a quick lookover—not a lot of changes!

cfleming05:08:30

No, it’s relatively small.

cfleming05:08:02

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.

cfleming05:08:16

If this becomes the definitive version that might be something to consider.

cfleming05:08:44

I’m hopeful that the remaining bugs are relatively straightforward.

cfleming05:08:00

Seems likely, given that a large chunk of the tests pass.

shaunlebron05:08:34

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

shaunlebron05:08:27

thanks for keeping the diff small, very helpful

cfleming05:08:53

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.

shaunlebron05:08:04

i saw some of the calls shifted around for that—did it help this?

cfleming05:08:04

Did it help what, sorry?

shaunlebron05:08:43

oh I mean was it important to this edits feature to move it around?

shaunlebron05:08:49

gonna get some sleep and try in the morning

cfleming06:08:58

Cool, GTG too, talk tomorrow

shaunlebron19:08:53

streaming my work on it now

shaunlebron19:08:21

current issue is that the edit from correctParenTrail seems to be interfering with previous edits that remove unmatched close-parens on the spot

cfleming19:08:58

Ok, can’t really get on now (looking after my daughter) but the issue might be in how the edits are applied in resultText

shaunlebron19:08:30

np, streams are recorded if you wanna look back

shaunlebron19:08:33

just helps me focus too

cfleming19:08:56

Ok, will be around in an hour or two

shaunlebron20:08:03

having trouble picking a starting point

shaunlebron20:08:25

i fixed one test and broke all the others

shaunlebron20:08:27

going to try to keep the same processLine logic to be sure that stuff is happening in the right order

shaunlebron20:08:02

wanna try replacing inputLines iteration with origText iteration first

shaunlebron20:08:16

then changing replaceWithinLine to append an edit, then building result text

shaunlebron20:08:22

will help me isolate what went wrong

cfleming22:08:57

@shaunlebron Sorry, that took longer than expected - had to take the cat to the vet

cfleming22:08:12

I’ll pull in your changes and then take a look.

shaunlebron22:08:44

changes are not ready. was starting over later to things more incrementally

cfleming22:08:58

@shaunlebron Have you ever tried floobits?

cfleming22:08:20

We could give that a go if you have some more time to look at it later.

shaunlebron22:08:57

i tried installing it for atom and it never initialized correctly

shaunlebron22:08:12

i can try it for another editor

cfleming22:08:51

I’ll sign up for a free plan and install it for IntelliJ

cfleming22:08:58

Looks like they have neovim

shaunlebron23:08:47

is anyone available for testing atom-parinfer on master?

shaunlebron23:08:10

the options screen isn’t showing up for chris, and I’d like to see if it’s working for others

rgdelato23:08:16

I pulled master, restarted Atom, and I'm seeing the settings screen

shaunlebron23:08:29

what version are you on?

rgdelato23:08:01

1.18.0 x64 on OSX and Atom claims it's up to date

shaunlebron23:08:17

chris couldn’t get it to show up on 1.17.2

shaunlebron23:08:30

and the config API between them seem the same

shaunlebron23:08:40

i wonder if people are upgrading their editors

rgdelato23:08:05

so Chris is just seeing "Uninstall" and "Disable" buttons and no "Settings" button?

shaunlebron23:08:41

i installed 1.17.2 from github releases, and deleted 1.18.0

shaunlebron23:08:50

@chrisoakman can you send us a screenshot?

shaunlebron23:08:08

why is that showing the parinfer readme instead of the atom-parinfer readme

shaunlebron23:08:31

you are symlinking the package to the wrong directory

rgdelato23:08:16

I was about to say, that Readme looks totally different from mine as well

chrisoakman23:08:14

I fixed whatever was going on with that. I still have no settings.

chrisoakman23:08:29

Do you see settings with v1.17.2?

shaunlebron23:08:48

the screenshot shows the version page above it

shaunlebron23:08:22

@chrisoakman make sure Parinfer is capitalized

cfleming23:08:05

@shaunlebron Got a question when you have a minute

shaunlebron23:08:04

actually should it be capitalized?

shaunlebron23:08:23

config-key should namespace the config keys to the case of the package I’m guessing

shaunlebron23:08:58

looks like it should be lowercased https://atom.io/packages/parinfer, not sure why I had it capitalized?

rgdelato23:08:06

in general, it's kinda weird that it's capitalized since it puts Parinfer before all my other packages in alphabetical order

shaunlebron23:08:51

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?

rgdelato23:08:20

I run apm link from inside the atom-parinfer directory

shaunlebron23:08:18

that explains the capitalization, the package.json name is “Parinfer”

shaunlebron23:08:53

submitting a PR

chrisoakman23:08:45

I already fixed it.

shaunlebron23:08:23

you renamed the package?