Unfreeze Git Commit Message grammar II #173195

walles · 2023-02-02T20:07:10Z

Before this change, the upstream for the VSCode Git grammar was dead.
Also, the test setup for that project has been EOL since 2014, so even
just running their tests was difficult.

The replacement grammar, unlike the current grammar:

Has a vscode-tmgrammar-test test suite that is runnable and passing
and that will run in CI for any PRs (in the upstream project)
Has diff highlighting for Swedish as well as English (No diff colors in non-English Git commit message #133888)
Highlights touched files both in Swedish and in English

Fixes #133888
Fixes #168847

Ref: https://github.com/walles/git-commit-message-plus

For the record, I was the one setting up the new Git Commit Message project. And it was fun!

After

Sample message to test this yourselves:


# Ange incheckningsmeddelandet för dina ändringar. Rader som inleds
# med "#" kommer ignoreras, och ett tomt meddelande avbryter incheckningen.
#
# På grenen main
# Ändringar att checka in:
#	ändrad:        hej
#
# ------------------------ >8 ------------------------
# Raden ovan får inte ändras eller tas bort.
# Allt under den kommer tas bort.
diff --git hej hej
index 257cc56..5716ca5 100644
--- hej
+++ hej
@@ -1 +1 @@
-foo
+bar

Before

Note lack of file names highlighting and diff highlighting.

Before this change, the upstream for the VSCode Git grammar was dead. Also, the test setup for that project has been EOL since 2014, so even just running the tests was difficult. The replacement grammar, unlike the current grammar: * Has a vscode-tmgrammar-test test suite that is runnable and passing and that will run in CI for any PRs (in the upstream project) * Has diff highlighting for Swedish as well as English (microsoft#133888) * Highlights touched files both in Swedish and in English Fixes microsoft#133888 Fixes microsoft#168847 Ref: <https://github.com/walles/git-commit-message-plus> And for the record, I was the one setting up the new Git Commit Message project. And it was fun!

alexr00 · 2023-02-03T13:36:59Z

@walles thank you for the PR! Normally I prefer to see that the grammar has some wide use as an extension before adopting it, but given that this grammar is very small I'm fine with merging it anyway once it's ready.

I'm seeing a pretty big difference in how the "Changes to be committed" lines are colored (current on the left, yours on the right):

Are you able to restore the current behavior?

alexr00 · 2023-02-03T09:56:24Z

extensions/git-base/syntaxes/git-commit.tmLanguage.json

-								},
-								{
-									"match": ".{73,}$",
-									"name": "invalid.illegal.line-too-long.git-commit"


I see you no longer have a line-too-long scope. Did you intentionally remove this?

Yes, that was intentional.

I replaced those scopes with diagnostics and Quick Fixes.

This is what it looks like in the upstream project:

Most users won't have your whole extension installed, so having the line-too-long scope in the syntax highlighting would still be beneficial to them. Would you be willing to bring it back?

Done, just pushed a new change for that: 15defef

walles · 2023-02-03T18:16:46Z

I'm seeing a pretty big difference in how the "Changes to be committed" lines are colored (current on the left, yours on the right)

Are you able to restore the current behavior?

No, I don't think that's possible.

The current behavior depends on setting the color based on keywords in English.

Since I want this grammar to work for other languages as well (Swedish in particular), I think keyword.other.file-type.git-commit and string.unquoted.filename.git-commit is as good as it gets.

If you try the current highlighting with a Swedish Git commit template, you can see that the current highlighting just doesn't work, but my new one does:

This is the summary line. It can't be too long.
After I can write a much more detailed description without quite the same restrictions on length.

# Ange incheckningsmeddelandet för dina ändringar. Rader som inleds
# med "#" kommer ignoreras, och ett tomt meddelande avbryter incheckningen.
#
# På grenen main
# Ändringar att checka in:
#	ändrad:        hej
#
# ------------------------ >8 ------------------------
# Raden ovan får inte ändras eller tas bort.
# Allt under den kommer tas bort.
diff --git hej hej
index 257cc56..5716ca5 100644
--- hej
+++ hej
@@ -1 +1 @@
-foo
+bar

alexr00 · 2023-02-06T11:55:16Z

The current behavior depends on setting the color based on keywords in English.

I see your point, thanks for the context. It seems like it should be possible to keep the extra highlighting for English and still provide the good highlighting you've added for all other languages. Would that be possible?

Ref: microsoft/vscode#173195 (comment)

Retain the line-too-long subject line highlighting. Improved to highlight only the too-long part, but same idea still. Special case English language file operations keywords and retain the previous classification of those. But fallback to op-and-filename classification when that fails (like it will for Swedish git for example).

walles · 2023-02-06T19:01:14Z

It seems like it should be possible to keep the extra highlighting for English and still provide the good highlighting you've added for all other languages. Would that be possible?

Yes, just pushed a new change for that: 15defef

alexr00

Thank you for the changes @walles! We usually update grammars via a script, so I've made the updates required to keep that working (and ran the script so that git-commit.tmLanguage.json would be consistent with our other grammar files). I've also updated our test results and the cgmanifest file (which tracks where code came from).

Looks great!

walles · 2023-02-07T14:10:26Z

Thank you @alexr00 for the constructive review process, this was a pleasure!

* Unfreeze Git Commit Message grammar II Before this change, the upstream for the VSCode Git grammar was dead. Also, the test setup for that project has been EOL since 2014, so even just running the tests was difficult. The replacement grammar, unlike the current grammar: * Has a vscode-tmgrammar-test test suite that is runnable and passing and that will run in CI for any PRs (in the upstream project) * Has diff highlighting for Swedish as well as English (microsoft#133888) * Highlights touched files both in Swedish and in English Fixes microsoft#133888 Fixes microsoft#168847 Ref: <https://github.com/walles/git-commit-message-plus> And for the record, I was the one setting up the new Git Commit Message project. And it was fun! * Remedy review feedback Retain the line-too-long subject line highlighting. Improved to highlight only the too-long part, but same idea still. Special case English language file operations keywords and retain the previous classification of those. But fallback to op-and-filename classification when that fails (like it will for Swedish git for example). * Update colorize test result * Update script and cgmanifest --------- Co-authored-by: Alex Ross <alros@microsoft.com>

Ref: microsoft/vscode#173195

walles force-pushed the johan/replace-git-commit-grammar branch from 2d4317c to 761a485 Compare February 2, 2023 20:08

walles mentioned this pull request Feb 2, 2023

Unfreeze Git grammars #168698

Closed

walles marked this pull request as ready for review February 2, 2023 20:43

VSCodeTriageBot assigned lszomoru Feb 2, 2023

alexr00 assigned alexr00 and unassigned lszomoru Feb 3, 2023

alexr00 requested changes Feb 3, 2023

View reviewed changes

walles added a commit to walles/git-commit-message-plus that referenced this pull request Feb 6, 2023

Revive line-too-long matching in the grammar

f73545b

Ref: microsoft/vscode#173195 (comment)

alexr00 added 2 commits February 7, 2023 11:42

Update colorize test result

eeee08b

Update script and cgmanifest

8dad073

alexr00 approved these changes Feb 7, 2023

View reviewed changes

alexr00 added this to the February 2023 milestone Feb 7, 2023

hediet approved these changes Feb 7, 2023

View reviewed changes

alexr00 merged commit f8c3f89 into microsoft:main Feb 7, 2023

walles deleted the johan/replace-git-commit-grammar branch February 7, 2023 14:09

walles added a commit to walles/git-commit-message-plus that referenced this pull request Mar 4, 2023

We're upstream, so diff highlighting is no more an added feature

a4f7fa6

Ref: microsoft/vscode#173195

github-actions bot locked and limited conversation to collaborators Mar 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unfreeze Git Commit Message grammar II #173195

Unfreeze Git Commit Message grammar II #173195

walles commented Feb 2, 2023 •

edited

alexr00 commented Feb 3, 2023

alexr00 Feb 3, 2023

walles Feb 3, 2023 •

edited

alexr00 Feb 6, 2023

walles Feb 6, 2023 •

edited

alexr00 Feb 7, 2023

walles commented Feb 3, 2023 •

edited

alexr00 commented Feb 6, 2023

walles commented Feb 6, 2023

alexr00 left a comment

walles commented Feb 7, 2023

Unfreeze Git Commit Message grammar II #173195

Unfreeze Git Commit Message grammar II #173195

Conversation

walles commented Feb 2, 2023 • edited

After

Before

alexr00 commented Feb 3, 2023

alexr00 Feb 3, 2023

Choose a reason for hiding this comment

walles Feb 3, 2023 • edited

Choose a reason for hiding this comment

alexr00 Feb 6, 2023

Choose a reason for hiding this comment

walles Feb 6, 2023 • edited

Choose a reason for hiding this comment

alexr00 Feb 7, 2023

Choose a reason for hiding this comment

walles commented Feb 3, 2023 • edited

alexr00 commented Feb 6, 2023

walles commented Feb 6, 2023

alexr00 left a comment

Choose a reason for hiding this comment

walles commented Feb 7, 2023

walles commented Feb 2, 2023 •

edited

walles Feb 3, 2023 •

edited

walles Feb 6, 2023 •

edited

walles commented Feb 3, 2023 •

edited