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

Use correct tm_scope for Unix Assembler #4167

Merged
merged 2 commits into from
Jul 6, 2018
Merged

Conversation

lildude
Copy link
Member

@lildude lildude commented Jun 20, 2018

Description

As pointed out in #4163, GNU/Unix Assembler files are not being syntax highlighted.

@pchaigno spotted that this is because the tm_scope is incorrect. This PR corrects that.

Checklist:

  • I am adding new or changing current functionality fixing a bug
    • I have added or updated the tests for the new or changed functionality.

Fixes #4163

@pchaigno
Copy link
Contributor

Shouldn't this have been detected by our tests?

@lildude
Copy link
Member Author

lildude commented Jun 21, 2018

Shouldn't this have been detected by our tests?

Yup, as has happened with me just changing the tm_scope, though I'm not sure why this wasn't picked up when the grammar was initially changed. Digging into it.

@lildude
Copy link
Member Author

lildude commented Jun 21, 2018

Hmmm, looks like the source.assembly.unix values may have been manually set as replacing the grammar with itself results in the correct values:

$ script/add-grammar --replace Assembly-Syntax-Definition https://github.com/calculuswhiz/Assembly-Syntax-Definition
Checking docker is installed and running
$ docker ps
Deregistering: vendor/grammars/Assembly-Syntax-Definition
$ git submodule deinit vendor/grammars/Assembly-Syntax-Definition
$ git rm -rf vendor/grammars/Assembly-Syntax-Definition
$ script/grammar-compiler update -f
Registering new submodule: vendor/grammars/Assembly-Syntax-Definition
$ git submodule add -f https://github.com/calculuswhiz/Assembly-Syntax-Definition vendor/grammars/Assembly-Syntax-Definition
$ script/grammar-compiler add vendor/grammars/Assembly-Syntax-Definition
Confirming license
$ script/licensed
Updating grammar documentation in vendor/README.md
$ bundle exec rake samples
$ script/sort-submodules
$ script/list-grammars
$
$ $ git diff grammars.yml                                                                                                    colin@github.com
diff --git a/grammars.yml b/grammars.yml
index c68d4c22..306b764c 100755
--- a/grammars.yml
+++ b/grammars.yml
@@ -10,7 +10,7 @@ vendor/grammars/Agda.tmbundle:
 vendor/grammars/Alloy.tmbundle:
 - source.alloy
 vendor/grammars/Assembly-Syntax-Definition:
-- source.assembly.unix
+- source.x86   <---- HERE
 vendor/grammars/AutoHotkey:
 - source.ahk
 vendor/grammars/BrightScript.tmbundle:
[...]

@pchaigno
Copy link
Contributor

pchaigno commented Jun 21, 2018

looks like the source.assembly.unix values may have been manually set

It should have been detected when it was set then, or are we missing a test?

@lildude
Copy link
Member Author

lildude commented Jun 21, 2018

It should have been detected when it was set then or are we missing a test?

Been dabbing this morning and I think we're missing a test as manually setting this in both gammars.yml and languages.yml results in passing tests, provided both are the same value, even if it's a bogus value.

Will look into adding a test to ensure the correct value is set as part of this PR.

@lildude
Copy link
Member Author

lildude commented Jun 21, 2018

Hmmm, seems harder to do than I thought. Might have to hold off on a test for this for the moment.

@lildude
Copy link
Member Author

lildude commented Jul 5, 2018

Not gonna add a test for the mo. Ready for review.

@lildude lildude merged commit 8540865 into master Jul 6, 2018
@lildude lildude deleted the lildude/fix-assembly branch July 6, 2018 07:25
@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GNU assembler not highlighting
3 participants