Closed tapeinosyne closed 10 years ago
Another implementation would be to call (move-up :left) repeatedly until there is no change. This would remove the reliance on canonical indentation (column 0 mentioned above) and would re-use code that has recently been merged into Paredit.
That was my very first thought as well, but recursively calling move-up (and thus form-boundary, and thus paired-scan) turned out to be rather slow.
Hitting the top level with form-boundary causes a blocking delay of about 100-150ms on my machine (even if it feels much longer than that; perhaps there are other factors at play that are not being measured by the stopwatch). The most likely cause is paired-scan searching within the full search-range [(- (:line loc) 100) (+ (:line loc) 100)]
.
This is not an issue with LT's implementation, by the way: Emacs' own manipulator has the same problem.
Oh. Good point :) Wouldn't it be nice if a data structure could be kept up-to-date with every editor event so we wouldn't have to parse?
Paired-scan should stop at the first pair it finds (at least until you do the very last one), if it doesn't that's a bug.
Writing a real parser and maintaing a parse tree is likely the ultimate thing that needs to happen. It'll just be run in the background.
I think Subpar for CodeMirror keeps an index, but Chris mentioned somewhere that it became untenably slow with large files.
That's because it regenerated the whole tree on every change and it did it on the UI thread.
I believe that's how the emacs version of paredit is implemented.
@ibdknox When you are at a given level, isn't paired-scan supposed to search all the way for a parens pair that closes that level? If so, I reckon it's behaving as intended; it's just that it starts desperately running around —arms flailing— looking for a higher-level form that happens not to be there, exactly as it should do.
@ndr-qef Heh, I am the author of Subpar and Chris is exactly right -- Subpar builds the whole tree every single time. I didn't (and still don't) know how to put something on a different thread, or how to keep state, in this setup. It feels like we should do the parser/background thing instead of continuing to parse for every new feature.
@ndr-qef yeah, that's what it's supposed to do :)
@achengs I should point out that your implementation is beautiful, it just needs to be made incremental and could likely undergo a few small changes to be faster. In terms of running in the background, that's actually really simple :) Here's how the tokenizer for the words-in-a-file works for auto-complete: https://github.com/LightTable/LightTable/blob/master/src/lt/plugins/auto_complete.cljs#L86
It'd basically need to do the same thing as what happens there. In terms of storing state, you can always just store the tree in each editor object. Though in the long run, it may make more sense to store it in the background process so you don't have to constantly pass it back and forth all the time.
@achengs I agree that implementing the background parser should be our goal… but while we sit on our ambition —gazing longingly into the distance of abstraction, efficiency and beauty— there are plugin devs out there that need and crave their next dose of cursor manipulation, and if we don't give it to them, they'll just cook some up themselves with no regard for quality or safety.
The above is, of course, absolutely not related to what I, in strictly hypothetical terms, might have possibly done.
@ndr-qef Sorry, I should have thought more before my original comment on this PR -- you put more thought and actual testing into the possible use of (move-up :left) than I did :-P I now realize the one-off use when the cursor is already at the top of a form seems acceptable because the delay is unnoticeable -- the cursor isn't supposed to move and doesn't move (and so it's more difficult to tell when it's done searching the 100 lines vs still doing the work). Had I put as much thought or testing into it as you did, I would have realized and then not have written that first comment.
As for making a distinction between one kind of dev and another... I think generally folks do have a regard for quality and safety (but this is just a supposition of mine; perhaps you know better since you have contributed on github multiple orders of magnitude more than I have :-P) ... I think the gradient is more along the lines of how much a dev knows (or doesn't know) about setting something up in a good way (using more advanced language features, using a separate thread, storing state, using a real parser, facility with the BOT architecture of LT, etc).
I'll look at the link Chris shared and inch my way up this gradient. I'm at the lower end of the gradient.
I agree with the part of your latest comment about getting something in place that works for now ... I cobbled together a (monstrous and shameful but not without regard to quality/safety) backspace / delete in another PR. The thing I'm itching for most is moving forward / backward by expression / token. It would be nice to know what people are working on so we don't duplicate effort.
For the interim, let's roll with this.
An additional cursor-manipulation facility that allows the user to quickly move to the top level, regardless of form depth. It is not as mature or as general as Emacs' beginning-of-defun, but it is useful.
This basic implementation has its limits, the most significant one being a reliance on canonical indentation: if the opening parens of the top-level form is not at column 0, the command blissfully misses it and goes its merry way to the preceding one.
As an organizational note, I should add that beginning-of-defun and end-of-defun are not part of the original paredit, but rather of Emacs' own LISP mode. However, the Paredit plugin seems the most relevant codebase for the command in LT, since objs/editor is mostly concerned with general utilities and CodeMirror wrapping. Let me know if you think otherwise.