This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2016-02-23
Channels
- # admin-announcements (1)
- # announcements (1)
- # beginners (222)
- # boot (210)
- # cider (26)
- # cljs-dev (50)
- # cljsrn (19)
- # clojure (243)
- # clojure-art (12)
- # clojure-finland (1)
- # clojure-poland (43)
- # clojure-russia (46)
- # clojure-sg (13)
- # clojurescript (60)
- # core-async (14)
- # css (11)
- # datomic (9)
- # devcards (9)
- # dirac (2)
- # editors (13)
- # emacs (5)
- # euroclojure (1)
- # events (3)
- # hoplon (76)
- # immutant (10)
- # job (1)
- # jobs (2)
- # keechma (1)
- # ldnclj (33)
- # lein-figwheel (1)
- # leiningen (20)
- # luminus (26)
- # mount (31)
- # om (105)
- # onyx (56)
- # parinfer (29)
- # perun (12)
- # proton (1)
- # re-frame (14)
- # reagent (5)
- # sydney (1)
- # yada (15)
Actually, that case is not bad because the paren trail doesn’t get extended over the unmatched ]
. But I suspect a case like (foo (bar)]])
would cause problems. I have all the indent mode tests working now so there are no cases like that, I’ll try to write some around the edge cases.
@shaunlebron: I have all the test cases passing with the delta algorithm now.
This is parinfer master on my machine:
~/d/p/lib (master)> node test/perf.js
Processing long_map_with_strings : 303 lines, 4380 chars
indent: 10.331ms
paren: 4.725ms
Processing really_long_file : 2865 lines, 112431 chars
indent: 15.639ms
paren: 21.972ms
Processing really_long_file_with_unclosed_paren : 2865 lines, 112432 chars
indent: 11.439ms
paren: 9.827ms
Processing really_long_file_with_unclosed_quote : 2865 lines, 112433 chars
indent: 8.420ms
paren: 8.317ms
This is the delta algorithm:
~/d/p/lib (master)> node test/perf.js
Processing long_map_with_strings : 303 lines, 4380 chars
indent: 11.210ms
paren: 6.438ms
Processing really_long_file : 2865 lines, 112431 chars
indent: 16.363ms
paren: 19.291ms
Processing really_long_file_with_unclosed_paren : 2865 lines, 112432 chars
indent: 10.691ms
paren: 9.168ms
Processing really_long_file_with_unclosed_quote : 2865 lines, 112433 chars
indent: 9.460ms
paren: 9.965ms
i’m not understanding how the cases you mentioned would cause problems
every edit essentially introduces a new coordinate space
so an edit should be in the coordinate space that results from the previous edit
the edits wouldn’t be in source order; they would just be in edit order
in the case of (foo (bar)]])
, the edits would be:
(foo (bar)]])
^ 1. remove index 10
(foo (bar)])
^ 2. remove index 10
(foo (bar))
^^ 3. replace indices 9-10 with `))` (oops, we shouldn't replace this if it's already there)
i may not be understanding your delta algorithm though, so I’ll look at the code when you’re done
I actually still have a single breaking indent case, I had some tests commented out accidentally.
The issue is that you’re always reading from the source coordinate space, since the edits are not actually applied. So somewhere you have to maintain the delta between source and destination spaces, and that depends on where the edits have been made already.
What I have now works. Basically I make sure that the edits are always in source document order, and the only cases I’ve found where they overlap is the case I showed earlier where individual unbalanced close parens are inside a paren trail that is later removed. So when making a replacement, I just remove any current edits which are completely overwritten by the new replacement. So if the whole paren trail is replaced, the individual close paren deletions are simply removed because they’re completely overwritten by the later change.
I’ll fix the one remaining case tomorrow morning and push it to my repo, and you can take a look and see what you think. TBH I’m not sure that this change makes sense for the main parinfer implementation, but it does for me - I think I’ll base the Cursive implementation on it.
I don’t know enough about other editor implementations to know whether applying deltas would work better or not.
One nice property of this new algorithm for me is that it translates naturally to using lexer tokens instead of chars. I profiled this, and nearly all the time is spent in processChar
and updateParenTrailBounds
- this would be faster using the lexer probably since the paren trail would only be updated on token boundaries.
So I worry that the delta approach sounds similar to the stateful change diff api that parinfer originally had. In that the structure and application of the diff was built on applying it to code mirror, so porting to vim involved adding a lot of code to handle the structure and build what code mirror gave you for free. This isn't to say that it wouldn't be worth it but any editor that accepts a different way of encoding deltas (if they do at all as in vims case), would require, potentially, a good chunk of code.
@snoe: What is the complication with vim? Does it not have a function to replace from char x to char y with string z
?
I also have a function which you can use to get the complete replacement text, given the original text and the deltas, so at the worst with one more function call you can get what exists right now.