-
Notifications
You must be signed in to change notification settings - Fork 591
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Lisp] Rewrite Syntax #3896
[Lisp] Rewrite Syntax #3896
Conversation
@s-clerc Test drive? |
This commit adds a heuristic to match function declaration keywords of the form `def<...>fun`.
Why move the variables to the bottom of the syntax file? Seems like a weird choice, all other syntaxes do it the other way around. |
It is just annoying to always need to either fold them or scroll down half the document just to get to the contexts. Variables are less likely navigation targets. With them being out of the way, it even less hurts to format lists to 1 item per line, which may help keeping diffs clean if keywords are added. That's probably something to generally considder for all syntaxes. |
Good point. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have been all the way through the test file. It's very rigorous.
I have also discovered that I don't know Lisp loop structures at all.
;;; with-clause | ||
|
||
with var1 :float = 10.5 and var2 = true | ||
; ^^^^ keyword.declaration.variable.lisp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Someone better at Lisp than me should input whether this is right or if it's a context declaration. I would believe either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to https://www.lispworks.com/documentation/lw70/CLHS/Body/m_loop.htm#loop, this is part of the variable-clause
and looks rather like a variable declaration statement, followed by main-clauses, which describe looped statements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better than my attempt and better than the current one. It'd be nice to get Lisp-user eyes, but since none are available, I say ship it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I basically only checked it out locally and opened the syntax test file to check whether the tokens looked reasonable with my color scheme. This is most definitely not a proper review, but since we need 2 approvals and I don't see another contributor with some lisp experience appearing out of thin air, here's mine so that we can get this out there.
Thank you guys! |
Resolves #1968
Supersedes #2387
Supersedes #2312
Inspired by #2387
This PR actually started with #2387 but ended up being a complete rewrite. Hence opening a new PR seems more reasonable.
It uses rules from https://www.lispworks.com/documentation/common-lisp.html
EDIT:
It does currently not add dedicated scopes for globals (e.g.:defvar
,devparameter
ordefconstant
) (see #2319) as most other syntaxes in this repo do not treat variable definitions special or add them to index as well.