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

feat: add nasm language #6068

Merged
merged 4 commits into from
Feb 25, 2023
Merged

Conversation

mtoohey31
Copy link
Contributor

@mtoohey31 mtoohey31 commented Feb 20, 2023

This pull request adds nasm assembly as a language.

Closes #3329.

Under the issue mentioned above, there is a discussion about how to support various assembly flavours. I think the best approach is to add flavour-specific language definitions with the appropriate grammars, then users can override the file-types values as desired in their local languages.toml files so that the .asm extension (or whichever extension they're using) is recognized as the flavour that they're working with. Since nasm will be the only flavour supported so far after the changes in this merge request, I've included the .asm extension under nasm's file-types so that there is at least some syntax highlighting out of the box, even though it might not be for the right flavour. If there's a different approach that might work better though, just let me know and I can rework things.

Also, CC @naclsn (the original author of the grammar and queries). I hope you don't mind me including your work here; I've copied the queries pretty much verbatim, only I've removed some todo-related comments that probably don't belong in the version inside this repository, and I've uncommented the (ERROR) @error highlight.

Edit: oh, and here's a screenshot:

helix-nasm-nord

Co-authored-by: Grenier Celestin <[email protected]>
@the-mikedavis the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. A-language-support Area: Support for programming/text languages labels Feb 20, 2023
@naclsn
Copy link
Contributor

naclsn commented Feb 21, 2023

Neat, thanks for using my work!

Like most language with (C-like) preprocessor, the written code can be syntactically invalid despite being correct. This is why I had the same approach as the C highlight queries to not have @error at all.

@mtoohey31
Copy link
Contributor Author

mtoohey31 commented Feb 21, 2023

Neat, thanks for using my work!

Like most language with (C-like) preprocessor, the written code can be syntactically invalid despite being correct. This is why I had the same approach as the C highlight queries to not have @error at all.

Ah, that makes sense, thanks for taking a look at the changes! I've removed that query in eda2cc6.

runtime/queries/nasm/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/nasm/highlights.scm Outdated Show resolved Hide resolved
(#match? @constant.builtin "^__\\?[A-Z_a-z0-9]+\\?__$"))
(word) @variable

(preproc_arg) @keyword
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the preprocessor stuff is like the C preprocessor, these might fall under @keyword.directive https://docs.helix-editor.com/master/themes.html#syntax-highlighting

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right that @keyword.directive makes more sense. I've switched it to that in df21375 along with a few other improvements.

@the-mikedavis the-mikedavis merged commit a4049e6 into helix-editor:master Feb 25, 2023
@mtoohey31 mtoohey31 deleted the feat/nasm branch February 26, 2023 19:51
estin pushed a commit to estin/helix that referenced this pull request Mar 4, 2023
wes-adams pushed a commit to wes-adams/helix that referenced this pull request Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-support Area: Support for programming/text languages S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add highlighting for assembler (.s)
3 participants