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

Improve lua grammar #140489

Closed
sumneko opened this issue Jan 11, 2022 · 12 comments · Fixed by #142107
Closed

Improve lua grammar #140489

sumneko opened this issue Jan 11, 2022 · 12 comments · Fixed by #142107
Assignees
Labels
feature-request Request for new features or functionality grammar Syntax highlighting grammar verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@sumneko
Copy link
Contributor

sumneko commented Jan 11, 2022

The current lua grammar is a little poor, such as:

  1. Many tokens are not catched:
    no-token

  2. Dose not support tokens after Lua 5.2
    old-token

  3. Dose not support EmmyLua, whitch is a type annotation system that has become the de facto standard
    no-luadoc

Since the source project is no longer active (I have created a PR there 3 years ago), I created a new project to provide the grammar file: https://github.com/sumneko/lua.tmbundle/blob/master/Syntaxes/Lua.plist

The effect of the above code after using this grammar file is:
new-syntax

The grammar file has been in my extension for several years and it works fine.
I hope that it can be set as the default lua grammar file of VSCode, so that theme developers can better optimize the color matching for it.

@alexr00
Copy link
Member

alexr00 commented Jan 12, 2022

Thanks for reaching out @sumneko. I looked for Lua.plist in https://github.com/sumneko/lua-language-server, but I wasn't able to find it. Can you point me to where in your extension you use the grammar file? Does your extension depend on https://github.com/sumneko/lua.tmbundle?

When adopting a new grammar file in VS Code we want to make sure we're moving to a better supported grammar then the old one. I can see that your extension has regular releases, which is great. I want to understand how the new grammar repo fits into the extension, and whether you'll continue to make fixes for the grammar.

@alexr00 alexr00 added grammar Syntax highlighting grammar info-needed Issue requires more information from poster labels Jan 12, 2022
@sumneko
Copy link
Contributor Author

sumneko commented Jan 12, 2022

Does your extension depend on https://github.com/sumneko/lua.tmbundle?

Yes, the Lua.plist is https://github.com/sumneko/lua.tmbundle/blob/master/Syntaxes/Lua.plist .
I used it in my extension at https://github.com/sumneko/vscode-lua/blob/master/syntaxes/lua.tmLanguage.json

I want to understand how the new grammar repo fits into the extension, and whether you'll continue to make fixes for the grammar.

In fact I maintain the grammar file by manually modifying the json file, and I used a script in the above project to convert json to plist.

I will countinue make fixes for the grammar, it has been part of my extension before.
After VSCode uses it, I will remove it from my extension and maintain the lua.tmbundle project instead.

@alexr00
Copy link
Member

alexr00 commented Jan 12, 2022

That all sounds great, thanks for the answers. We don't require a plist file, a tmLanguage.json file is also good, so no need to maintain a plist if that isn't your preference.

Let's try using https://github.com/sumneko/lua.tmbundle in VS Code at the beginning of our February iteration. That way, the changes will have the entirety of the February iteration to be used by VS Code insiders users before being released.

@alexr00 alexr00 added feature-request Request for new features or functionality and removed info-needed Issue requires more information from poster labels Jan 12, 2022
@alexr00 alexr00 added this to the February 2022 milestone Jan 12, 2022
@ghost
Copy link

ghost commented Jan 14, 2022

Is EmmyLua already the standard? I've been stuck with LDoc for a long time so will that be supported?
It'd be a matter of supporting -- prefix for doc comment and injecting into --[[ Lua block comments too.

@sumneko
Copy link
Contributor Author

sumneko commented Jan 14, 2022

Is EmmyLua already the standard? I've been stuck with LDoc for a long time so will that be supported? It'd be a matter of supporting -- prefix for doc comment and injecting into --[[ Lua block comments too.

Since I don't use LDoc, I don't support it, but PRs are welcome.
EmmyLua should be the only type annotation system for original Lua, and as far as I know LDoc cannot create or mark types.

@ghost
Copy link

ghost commented Jan 14, 2022

@type means a OOP class and @class means data type, for backwards compatibility with LuaDoc.
My main concern is that tags have to be reimplemented as repository rules injected in multiple contexts.
I can get to a PR sometime in the weekend.

@ChrisKader
Copy link

Thank you @sumneko

@alexr00
Copy link
Member

alexr00 commented Feb 4, 2022

Reopening for visibility.

@ghost
Copy link

ghost commented Feb 4, 2022

@sumneko I was not able to genericise the Lua documentation tags between block and inline comments, so I think I'll open a request on the bundle repo

Edit: LuaLS/lua.tmbundle#1

@sumneko
Copy link
Contributor Author

sumneko commented Feb 17, 2022

Sorry I just found that the issues function is disabled in https://github.com/sumneko/lua.tmbundle .
I have enabled it and any issues are welcome!

@alexr00
Copy link
Member

alexr00 commented Feb 17, 2022

I have not seen an increase in issues for lua syntax highlighting since switching to the new grammar. Closing as fixed! Thank you @sumneko.

@alexr00 alexr00 closed this as completed Feb 17, 2022
@alexr00
Copy link
Member

alexr00 commented Feb 21, 2022

Marking as verified since the colorization tests pass.

@alexr00 alexr00 added the verified Verification succeeded label Feb 21, 2022
@Tyriar Tyriar added the verification-needed Verification of issue is requested label Feb 23, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality grammar Syntax highlighting grammar verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants