Clojurians Log v2

@shaunlebron Ok, thanks. My problem is that instead of returning a whole string, I just return a series of edits, since that’s much more efficient in IntelliJ (the edits are nearly always small).

I’m finding that the edits are not consistently created in one space or the other (input or output)

I think it’s probably a subtlety in the paren trail handling - I also process the whole document, I don’t break it into lines. So if result.x isn’t correctly updated when closing the paren trail out, that wouldn’t affect your processing but probably affects mine.

Returning edits also meant that I got the cursor movement for free, since that wasn’t there previously.

I think the issue is this: when replaceWithinLine is called, that then shifts the x values that you need to use for changes after the current one, unless the edits are consistently made in input space.

That happens in commitChar because ch is set to "". However it doesn’t happen in correctParenTrail, but I think it doesn’t matter for you because you’re modifying line by line.

I’m thinking I’m going to try to consistently produce the edits in input space, which is what I had previously and I already have the code for working out the correct deltas to apply the changes. The only complication is that I then need the paren trail x values in input space, hopefully that’s not too tricky.

@shaunlebron Actually, do you only use the output coords for knowing where to make your edits? If I want my edits to be in input coords, could I simplify things by only working with input coords?

i track my edits in input coords

but I’m not able to follow much of what you said actually

is the divergence in behavior due to an inconvenience in how parinfer.js is behaving, related to intellij?

i know there may be particular reasons for creating separate implementation, perhaps mostly to explore ideas that may inform core—having said that, I think it will be difficult to help you keep yours in sync with parinfer.js as changes will be inevitable due to the pace of smartMode changes

The edits I’m seeing are definitely not all in input coords. For example, edits are often created from the paren trail x values, and they’re created from result.x

Should they be created from result.inputX?

So my initial implementation diverged from how parinfer.js worked for a couple of reasons. Mostly the difference was that I process the whole input document rather than splitting it into lines, to reduce GC pressure.

Similarly, I return a series of edits to apply rather than returning a whole new document.

When I was perf testing, node was actually faster at a lot of this string manipulation than the JVM, presumably they have to optimise it based on usage patterns since JS doesn’t have a mutable StringBuffer-like thing.

That made the performance much better, and wasn’t a significant change to the algorithm.

However those changes seem to have been complicated by the new input/output coord stuff.

It may also just be a simple bug in my port, but it’s hard to tell - there’s quite a lot of code now, and it’s subtle stuff in places.

ah, I’m sorry I was thinking of changes when you said edits for some reason

No, I mean the x values passed to replaceWithinLine

But it actually seems like the paren trail x values should be in input coords anyway, since they’re compared to the cursorX values.

yeah, edits are incremental, so they have to be in output coords given what i’m currently doing

cursorX is updated when edits are made behind it

So if I wanted all my edits to be in input coords, do you think I could get rid of the x/inputX distinction altogether?

Is that likely to complicate my life?

I accumulate a list of edits, and when I’m applying them maintain a running delta, so I take care of the incremental nature at that time.

I think that running delta will probably be fine

Ok. I’ll try those changes in my port and see if that helps.

I guess you keep track the line number and column as a derivative value of your “global” index?

I’ll see afterwards if I could apply the same changes to the JS version, and send a pull request if so. It’s a fairly minor change to not require splitting into lines, and that will help the GC on JS anyway even if it’s better at optimising it.

cool! I think line splitting and array joining is super fast on JS, but probably not elsewhere?

Right, I maintain an offset which is a char offset into the original doc. Then I count lines as I see newlines, and reset the x at that point.

I just have no concept of how fast insertion and removal of characters inside a giant string can be

Right, but on a big file you’re still (at least) doubling the memory requirements - in a long-running editor that can add up.

It would be interesting to see if creating an array of edits was faster in JS too.

this is a bit of a blind spot for me—isn’t a single character insertion inside a giant string reallocating the whole thing anyway?

I suspect that in any editor that maintains non-trivial indices, applying small edits is likely much faster.

oh, I see, you’re not reallocating, you’re modifying the editor buffer in-place with a list of edits

Well, editors don’t generally use a giant string for their documents.

Right.

I like this idea

It would be interesting to know if e.g. Atom works natively with a single offset into a document rather than cols/lines - I’m willing to bet that it requires some calculation to work out line/col coords.

Although they probably do some trickery to make it efficient.

What might be possible would be an API which returns a list of changes, and then one like the current one that calls the other, and then applies the edits to the original string and returns that. Building that new string is probably still much more efficient than line splitting, since you’re only splitting/joining strings where things have actually changed.

Ok, I have to go now - I might actually look at getting the JS version working like this first, which would make my port easier, and you can take a look at it.

that would be great, thanks colin!

Then I can start from something working, and ensure the tests keep working.

good idea

Cool, seeya

bye