Skip to content
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

Merged
merged 8 commits into from
Apr 19, 2024
Merged

Conversation

deathaxe
Copy link
Collaborator

@deathaxe deathaxe commented Dec 27, 2023

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 or defconstant) (see #2319) as most other syntaxes in this repo do not treat variable definitions special or add them to index as well.

@michaelblyons
Copy link
Collaborator

@s-clerc Test drive?

This commit adds a heuristic to match function declaration keywords
of the form `def<...>fun`.
@deathaxe deathaxe mentioned this pull request Feb 10, 2024
36 tasks
Lisp/Lisp.sublime-syntax Outdated Show resolved Hide resolved
@jrappen
Copy link
Contributor

jrappen commented Feb 14, 2024

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.

@deathaxe
Copy link
Collaborator Author

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.

@michaelblyons
Copy link
Collaborator

it even less hurts to format lists to 1 item per line, which may help keeping diffs clean if keywords are added.

Good point.

Copy link
Collaborator

@michaelblyons michaelblyons left a 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.

Lisp/tests/syntax_test_lisp.lisp Outdated Show resolved Hide resolved
Lisp/tests/syntax_test_lisp.lisp Outdated Show resolved Hide resolved
Lisp/tests/syntax_test_lisp.lisp Show resolved Hide resolved
;;; with-clause

with var1 :float = 10.5 and var2 = true
; ^^^^ keyword.declaration.variable.lisp
Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

Lisp/tests/syntax_test_lisp.lisp Show resolved Hide resolved
Copy link
Collaborator

@michaelblyons michaelblyons left a 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.

Copy link
Collaborator

@FichteFoll FichteFoll left a 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.

@deathaxe
Copy link
Collaborator Author

Thank you guys!

@deathaxe deathaxe merged commit 2cbefb2 into sublimehq:master Apr 19, 2024
2 checks passed
@deathaxe deathaxe deleted the pr/lisp/rewrite-syntax branch April 19, 2024 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Lisp] Highlighting seems very gone
4 participants