Refactor sly-cl-indent.el so it doesn't clobber cl-indent.el #312
Conversation
This is the only usage of `rx` in all of sly so it seems silly to `require` it for that one simple regexp.
Old version hogged memory by repeatedly calling `reverse` on a fresh list. This version reuses the cons, and uses the primitive `assoc` instead of `cl-assoc`. It's faster and way more memory efficient.
And use `buffer-substring` instead of `buffer-substring-no-properties`, since `string-to-number` doesn't care about string properties, and `buffer-substring` has an opcode. This is considered a hot spot, so it seems appropriate to try to optimize it.
Removed `sly-length>`, as its only usage was checking if a list had a cdr. Made `sly-length=` only accept a list (the only argument type it's used with), and only use sparingly.
It's a hot spot, and the only call in sly.el was wrapped in `inline`, which is not supposed to be used in source code.
|
Thanks very much @SwiftLawnGnome for all these enhacements. I know I'm very late in reviewing, but they aren't forgotten! |
| @@ -1674,10 +1678,6 @@ EVAL'd by Lisp." | |||
| (and (>= (buffer-size) 6) | |||
| (>= (- (buffer-size) 6) (sly-net-decode-length)))) | |||
|
|
|||
| (defun sly-run-when-idle (function &rest args) | |||
There was a problem hiding this comment.
I like this: these one-off half-decent abstractions (in the words of the former SLIME maintainer) aren't really useful and complicate reading. I welcome this.
Forgot the TESTFN argument to `assoc` was a recent addition.
|
What's the status of this? |
|
Jonas Bernoulli <notifications@github.com> writes:
What's the status of this?
Nearly done, I'd say. From what I've seen, it looks fantastic. And
lots of cleanup work elsewhere in SLY, too. I just have to review it,
and I don't have a lot of time now. Or I give Zach commit permissions
(though I'd have to bother him with a couple of boring details, such as
GNU-style commit messages).
João
|
* (sly--lisp-indent-function-1, sly--lisp-indent-259): Use `char-before`.
Awesome!
I think it's definitely worth a review, I'm sure there are issues with it as it stands.
Please do bother me! I've been self conscious about my commits, I'd like to keep the style in line with Sly's. I'll start formatting them like Regarding sly-cl-indent, I was thinking there may need to be more backward compatibility since this refactor will inevitably break some people's private configs. The |
* contrib/sly-autodoc.el (sly-autodoc--parsing-safe-p): Remove. (sly-autodoc--parse-context): Don't use `sly-autodoc--parsing-safe-p`. (sly-autodoc--cache-get, sly-autodoc--cache-put): Remove. (sly-autodoc): Don't use `sly-autodoc--cache-get`. (sly-autodoc--async%): Don't use `sly-autodoc--cache-put`. (sly-autodoc--fontify): Fix indentation.
With `readtable-case` :invert, `tokenize-symbol-thoroughly` was blindly inverting every character in a string, but :invert only inverts the case when the entire token is the same case. So `SYMBOL` is read as `symbol`, and vice versa, but `Symbol` is read as-is. This is now handled correctly. * slynk/slynk.lisp (casify-char): Remove. (char-casifier): New function. Takes a string and returns a function which will apply the appropriate case to the characters in the string, according to `readtable-case`. (tokenize-symbol-thoroughly): Use `char-casifier`.
|
The most important cl-indent.el bits are already in master. The rest I have restructured and put them in a branch called |
Regarding this. I think we can cautiously assume that SLY users won't have so many of these configs. But surely here and there, a complaint might pop up. We'll hope they report it. |
* lib/sly-cl-indent.el (common-lisp-style): Remove alias to `sly-common-lisp-style`. All uses of `sly-common-lisp-style` now fall back on `common-lisp-style` if it's `bound-and-true-p`.
Lots of changes here. The biggest is with respect to the LOOP keyword regexps. The main reason I felt compelled to do this was that those regexps weren't terminated with "\\_>" (symbol end), so `thereis` was being indented like `the`. Then I just started fixing up a bunch of other regexp related stuff while I was at it. * lib/sly-cl-indent.el (sly--lisp-loop-keyword-regexp): New function. Creates a regexp matching a keyword or (maybe interned) symbol whose name is exactly one of its arguments. It's now used to generate the values of the following variables. (sly--common-lisp-body-introducing-loop-macro-keyword): Renamed to `sly--lisp-indent-body-loop-macro-keyword`. (sly--common-lisp-accumlation-loop-macro-keyword): Renamed to `sly--lisp-indent-accumulation-loop-macro-keyword`. (sly--common-lisp-prefix-loop-macro-keyword): Renamed to `sly--lisp-indent-prefix-loop-macro-keyword`. (sly--common-lisp-indent-clause-joining-loop-macro-keyword): renamed to `sly--lisp-indent-joining-loop-macro-keyword`. (sly--common-lisp-indent-indented-loop-macro-keyword): Renamed to `sly--lisp-indent-indented-loop-macro-keyword`, and added some missing keywords (probably still missing a few). (sly--common-lisp-indenting-loop-macro-keyword): Renamed to `sly--lisp-indent-conditional-loop-macro-keyword`. (sly--lisp-indent-loop-macro-else-keyword): Renamed to `sly--lisp-indent-else-loop-macro-keyword`. (sly--lisp-indent-lambda-list-keywords-regexp): Use `regexp-opt`, cuz why not. (sly--lisp-indent-feature-expr-regexp): Fixed the regexp (#!+ is not a valid feature expression as far as I know). (sly--lisp-indent-loop-advance-past-keyword-on-line): Use `skip-syntax-forward`, instead of `looking-at` every character in a loop. (sly--lisp-indent-function-lambda-hack): Fixed the regexp. Before it would treat `(function-foo (lambda ()))` the same as `(function (lambda ()))`. (sly--lisp-indent-beginning-of-defmethod-qualifiers): Trivial change to the regexp, and use `match-beginning` instead of `match-string` to test which group matched.
Before it would think "FOO-BAR" aught not to be downcased since the '-' isn't UPPER-CASE-P.
I broke the whole point of the function in the last commit for no reason, oops. * lib/sly-cl-indent.el (sly--lisp-indent-beginning-of-defmethod-qualifiers): Group "def" and ":" in the regexp for finding the start of a method definition, so the two forms can be differentiated.
Yes, sorry about that. I keep forgetting to branch. Are there any commits here you'd still like to see or did you cherry all the ones that matter? |
I don't know. I'm kinda lost now to be honest. I just think that the code you contribute to SLY is pretty good and would like to review it separately. Can you try rebase your "most current" branch on top of SLY's origin/master and showing me that in a new pull request? |
Ha, yeah I made a real mess.
I just attempted to do that here but it looks like I may have done something wrong. I'm not exactly a git expert. If this isn't right I can try to just branch from master and add the one or two significant commits to that, it looks like a large majority of the meaningful ones were already merged. |
No description provided.