[Diff] Refactor for Diff3, Git, and conflicts
Big refactor of Diff syntax according to the GNU and SVN documentation. New Git Diff syntax, also used by Git Log.
Thank you to the reviewers!
Notable changes
- Lots of documentation links
- Lots of new tests, mostly pulled from relevant documentation
- Some scope tweaking, but preserving the stacking from #2178 for backwards-compatibility
- New syntax for Git Diff emails (
git amandgit format-patch)- When the normal Diff syntax detects the "magic" first line, it delegates out to Git Diff instead.
- New support for
- Diff3 formats
- Conflict sections
- "Combined" unified diffs, only in Git Diff
Potentially controversial
- Conflicts somewhat with #4047, but should now be nearly compatible.
storage.modifierdistinguishes between the two Git file modes (namely644and755)- Markup scopes color the
++++----and similar, even though they're not actually markup. - Users and emails are delegated out to Mailmap.
- Whatever you noticed.
Am I supposed to see output on the failing unit tests? I don't think anything's failing locally.
Am I supposed to see output on the failing unit tests? I don't think anything's failing locally.
Presumably it is the error about Diff mixed inheritance versions at the very top of the CI console output: https://github.com/sublimehq/Packages/actions/runs/12216318274/job/34079277181#step:4:6
(Bit weird that the error comes before the packages path information etc)
error parsing lexer: Packages/Diff/Diff.sublime-syntax: syntax inheritance does not allow for mixed versions at line 5 column 1
Should be fixed by updating Packages/PHP/Embeddings/Diff (for PHP Interpolated).sublime-syntax to version 2.
🤦♂️ I think my ad blocker was preventing display of the test output. Thanks!
a few thoughts:
- deathaxe started moving variables to the very bottom of syntax files, you might as well
- similarly with contexts within the prototype section, moving that section to the bottom of contexts
- I'd appreciate a bit more use of whitespace in syntax files, makes them easier to read
- it might make sense to replace some of those multi-level pushes with pushes to named contexts, so it's easier to override or extend
Currently you're only matching English headers. They might not be in English as Git supports a few languages. You'd have to look that up.
Currently you're only matching English headers. They might not be in English as Git supports a few languages. You'd have to look that up.
For this, you are referring to the Signed-off-by:, etc., yes?
I haven't checked what exactly needs translations, didn't have the time. I just recall deathaxe adding huge walls of variables with translations to those other Git syntaxes.
Should we merge Conflict.sublime-syntax into the Diff syntax like #4047?
Unsure. As both don't share anything and merge conflicts appear without anything diff related, a stand-alone syntax is ok, too, I guess. Normal diffs won't ever contain conflict markers, would they?
Normal diffs won't ever contain conflict markers, would they?
No, but after I filed "Conflict?" into my broad organization of diff types, I'm much more willing to concede merging it. Almost none of the other diff types share any commonality with each other. They're just simple enough that they can coexist in the same file with no issue. Conflict could do the same (though I would change the variable names).
I'm considering adding a side-by-side diff highlighter, too, although I haven't looked too hard. (I think it'd be hard to do perfect, but easy to do good-enough.)
I am fine with both, keeping or merging.
I've brought Conflict.sublime-syntax back in, ~~along with a conservative side-by-side highlighter. A more risky side-by-side syntax at https://github.com/michaelblyons/SublimeSyntax-Defaults/pull/10~~ Edit: Deleted side-by-side now that we have overrideable beginning-of-line.
Rework of Git Log to use Git Diff contexts at https://github.com/michaelblyons/SublimeSyntax-Defaults/pull/11
let us know when/if you need any review or feedback 😁
let us know when/if you need any review or feedback 😁
Haha. Sorry if you're flooded in notifications. The PR is a lot better after you and DeathAxe's reviews. I think if you're looking for something, ~~check my PRs to this PR (michaelblyons#11 and michaelblyons#10) and~~ decide if you care about any of the things in the "controversial" heading in OP.
Thanks again.
Empty lines between contexts is ok, but starting arbritary emptly lines between patterns is too much.
There are places where I have considered newlines in stacked scopes, but I am not planning to add an empty line between meta and includes. PackageDev scoping is sufficient to make that distinction very obvious.
Hooray! Thanks, everyone.