@cfleming, yeah chris and I paired this afternoon to fix the selection issue you mentioned

when a selection is made, or if there are multiple cursors, atom-parinfer will not update the cursor position for you

https://github.com/oakmac/atom-parinfer/blob/master/src-cljs/atom_parinfer/core.cljs#L277-L282

@shaunlebron: Ok, thanks.

I’ll replicate that logic, I think.

I’m just playing around with the idea of creating deltas instead of updating the lines in place - then the algorithm can be run in one pass over the original document, and the user could call a function to get new text with the deltas applied, or choose to receive the deltas.

Most editors won’t store the text as a single contiguous string, so applying small deltas will probably be more efficient.

If the change gets too tricky, I’ll just use the existing algorithm for now since I should get a release out sometime soon.

character-based diffs?

a nice property of parinfer is that it will never add or remove lines, just modifies existing ones

so that’s why the line-based deltas are so straightforward, i’m returning changedLines

Yeah, I saw that. Perhaps I’ll just use that to start since that’s (relatively) minimal.

just trying to understand what the delta change looks like, or what it’s doing

so instead of modifying the line in result.lines[i], you’re doing something else?

Right. Instead of scanning by line, I scan once over the whole text without splitting it into lines. I do the line bookkeeping when I see a \n

And instead of line/x combos, I just use an offset into the whole document.

Then the changes are stored as a list of start, end, replacement combos.

I’m worried about the garbage being created on every call to parinfer by taking the document and splitting it into lines.

On the JVM that used to re-use the same character array, but it now copies it instead.

I’m not sure about JS engines.

i think that’s feasible

Yeah, I don’t think the change is going to be too big.

the last version changed removeParenTrail

it used to remove the paren trail after processing every line

Ok

it’s now just popParenTrail, modifies the paren stack

It would also mean that you wouldn’t need the getLineEnding thing to re-join the lines.

anyway, no unnecessary modifications, and you can watch all line modifications at replaceWithinLine function

interesting

And it is probably feasible to apply the changes in whichever way is friendliest to the JS garbage collector.

I like this

I’m actually very impressed with how well V8 handles all the string concatenation though.

same

I guess they have to optimise it a lot since there’s no mutable option.

Anyway, I’m hoping to have it working this afternoon with a bit of luck, and we can see if we like it at that stage.

As an aside, I’m using http://wallabyjs.com to make the change, it’s lovely.

was it slow when splitting lines?

I’m not sure. I don’t know whether the JS engines re-use the backing char array or not.

The JVM used to but then they changed it since apparently people would often hold onto small substrings of very large backing arrays, which caused memory problems.

So substring() now copies instead.

But I’m not just thinking about memory pressure, I suspect that in practice applying diffs will be much better for performance in text editors, although I have no idea how to benchmark that.

i’m really liking the delta idea

are you trying this in parinfer.js first?

with wallaby somehow?

You’ll be pleased to hear that Wallaby shows great test coverage too, only a couple of edge cases not covered (tabs and \r\n)

Yes

awesome, would love to try this

I have the wallaby config if you want to try it, let me know.

yeah, i sort of changed my testing philosophy in parinfer, just focusing on the core inference stuff

Sure, that makes sense.

yeah, if you have time to share that

need to step out for an hour

awesome change btw if we can get that in

Sure, no problem

I think he has an atom plugin now, no vim sorry

Oh, and sublime too

haha, i don’t mind that

i’ll be moving to parinfer soon anyway

atom*

Hehe

ok, talk to you later

bye

@shaunlebron: Actually, one more advantage of the deltas is that you don’t actually need to track cursor or selection position - they’ll automatically be updated by the editor as the text is modified.

@cfleming: mind blown

lol, yeah, the editor should totally take care of that. way simpler

I have about half the tests passing now

There are some tricky cases with this.

I had hoped that all edits would be applied to the document in source order, but that’s not the case.

For example, in (def foo [a b]], the second ] is removed first, because it’s unmatched.

Then the paren trail is corrected, which is tricky now.