Fix diff highlights #38384
Fix diff highlights #38384
Conversation
|
We require contributors to sign our Contributor License Agreement, and we don't have @daivinhtran on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
|
@cla-bot check |
|
We require contributors to sign our Contributor License Agreement, and we don't have @daivinhtran on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
|
The cla-bot has been summoned, and re-checked this pull request! |
| "diff" @function | ||
| (argument) @variable.parameter) | ||
|
|
||
| (filename) @string.special.path |
There was a problem hiding this comment.
(filename) was conflicting with (old_file) and (new_file).
If we choose to keep (filename), then we should remove others.
|
@cla-bot check |
cla-bot
bot
added
the
cla-signed
|
The cla-bot has been summoned, and re-checked this pull request! |
|
Just a note for reviewer @cole-miller. 🙏 Let me know if I should have approached this differently. |
|
Hi @MrSubidubi . Do you have any feedback on this PR? |
|
@daivinhtran Will soon get to this, sorry for the delay! Didn't have enouzgh time on my hands the past week as I was mostly OOO, but will do my best to get back to you this week. Wanted to check coverage before merging, otherwise looks good! |
|
We require contributors to sign our Contributor License Agreement, and we don't have @tidefield on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
cla-bot
bot
removed
the
cla-signed
MrSubidubi
left a comment
MrSubidubi
left a comment
There was a problem hiding this comment.
Sorry to leave you hanging for so long! I initially wanted to both query all extensions and get some data on how this is available in themes as well as add proper fallback behavior, but due to time constraints, I didn't get to that 😓
I'll try to pick that up during Quality Week hopefully, for now the comments will do.
Thank you again and sorry for having you wait this long!
|
Huh, seems to be something weird with the CLA. Did you change anything in the meantime? |
|
@cla-bot check |
cla-bot
bot
added
the
cla-signed
|
The cla-bot has been summoned, and re-checked this pull request! |
|
@MrSubidubi I changed my github username 😁 should be good now |
MrSubidubi
left a comment
MrSubidubi
left a comment
There was a problem hiding this comment.
Ah, that explains it. Again, thank you for the change, sorry for the long delay and also thanks for the quick follow-up!
Lastly, congratulations to your first contribution! 🎉
Per #23371 (reply in thread), the issue is not new and I don't know how to solve the problem more holistically yet. All of the native themes don't have spec for `@diff.plus` and `@diff.minus` leaving addition and deletion not being highlighted. For diff file, the most valuable highlighting comes from exactly what we're missing. Hence, I think this is worth fixing. Perhaps, the ideal fix would be standardizing and documenting captures such as `@diff.plus` and `@diff.minus` on https://zed.dev/docs/extensions/languages#syntax-highlighting for theme writers to adopt. But the existing list of captures seems to be language-agnostic so I'm not sure if that's the best way forward. Per tree-sitter-grammars/tree-sitter-diff#18 (comment), `tree-sitter-diff`'s author prefers using `@keyword` and `@string` so that `tree-sitter highlight` can work out of the box. So it seems to be an ok choice for Zed. Another approach is just adding `@diff.plus` and `@diff.minus` to the native themes. Let me know if I should pursue this instead. Before <img width="668" height="328" alt="Screenshot 2025-09-18 at 11 16 14 AM" src="https://github.com/user-attachments/assets/d9a5b3b5-b9ef-4e74-883f-831630fb431e" /> After <img width="1011" height="404" alt="Screenshot 2025-09-18 at 12 11 15 PM" src="https://github.com/user-attachments/assets/9cf453c0-30df-4d17-99e9-f2297865f12a" /> <img width="915" height="448" alt="Screenshot 2025-09-18 at 12 12 14 PM" src="https://github.com/user-attachments/assets/9e7438a6-9009-4136-b841-1f8e1356bc9b" /> Closes zed-industries/extensions#490 Release Notes: - Fixed highlighting for addition and deletion for diff language --------- Co-authored-by: MrSubidubi <finn@zed.dev>
Per zed-industries#23371 (reply in thread), the issue is not new and I don't know how to solve the problem more holistically yet. All of the native themes don't have spec for `@diff.plus` and `@diff.minus` leaving addition and deletion not being highlighted. For diff file, the most valuable highlighting comes from exactly what we're missing. Hence, I think this is worth fixing. Perhaps, the ideal fix would be standardizing and documenting captures such as `@diff.plus` and `@diff.minus` on https://zed.dev/docs/extensions/languages#syntax-highlighting for theme writers to adopt. But the existing list of captures seems to be language-agnostic so I'm not sure if that's the best way forward. Per tree-sitter-grammars/tree-sitter-diff#18 (comment), `tree-sitter-diff`'s author prefers using `@keyword` and `@string` so that `tree-sitter highlight` can work out of the box. So it seems to be an ok choice for Zed. Another approach is just adding `@diff.plus` and `@diff.minus` to the native themes. Let me know if I should pursue this instead. Before <img width="668" height="328" alt="Screenshot 2025-09-18 at 11 16 14 AM" src="https://github.com/user-attachments/assets/d9a5b3b5-b9ef-4e74-883f-831630fb431e" /> After <img width="1011" height="404" alt="Screenshot 2025-09-18 at 12 11 15 PM" src="https://github.com/user-attachments/assets/9cf453c0-30df-4d17-99e9-f2297865f12a" /> <img width="915" height="448" alt="Screenshot 2025-09-18 at 12 12 14 PM" src="https://github.com/user-attachments/assets/9e7438a6-9009-4136-b841-1f8e1356bc9b" /> Closes zed-industries/extensions#490 Release Notes: - Fixed highlighting for addition and deletion for diff language --------- Co-authored-by: MrSubidubi <finn@zed.dev>
4 participants
Per #23371 (reply in thread), the issue is not new and I don't know how to solve the problem more holistically yet.
All of the native themes don't have spec for
@diff.plusand@diff.minusleaving addition and deletion not being highlighted. For diff file, the most valuable highlighting comes from exactly what we're missing. Hence, I think this is worth fixing.Perhaps, the ideal fix would be standardizing and documenting captures such as
@diff.plusand@diff.minuson https://zed.dev/docs/extensions/languages#syntax-highlighting for theme writers to adopt. But the existing list of captures seems to be language-agnostic so I'm not sure if that's the best way forward.Per tree-sitter-grammars/tree-sitter-diff#18 (comment),
tree-sitter-diff's author prefers using@keywordand@stringso thattree-sitter highlightcan work out of the box. So it seems to be an ok choice for Zed.Another approach is just adding
@diff.plusand@diff.minusto the native themes. Let me know if I should pursue this instead.Before
After
Closes zed-industries/extensions#490
Release Notes: