Repository navigation
Add support for Jujutsu conflict markers #64750
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -530,6 +530,15 @@ func (s *Scanner) Scan() ast.Kind { | |
| case '`': | ||
| s.token = s.scanTemplateAndSetTokenValue(false /*shouldEmitInvalidEscapeError*/) | ||
| case '%': | ||
| if s.charAt(1) == '%' && isConflictMarkerTrivia(s.text, s.pos) { | ||
| s.pos = scanConflictMarkerTrivia(s.text, s.pos, s.errorAt) | ||
| if s.skipTrivia { | ||
| continue | ||
| } else { | ||
| s.token = ast.KindConflictMarkerTrivia | ||
| return s.token | ||
| } | ||
| } | ||
| if s.charAt(1) == '=' { | ||
| s.pos += 2 | ||
| s.token = ast.KindPercentEqualsToken | ||
|
|
@@ -584,6 +593,15 @@ func (s *Scanner) Scan() ast.Kind { | |
| s.token = ast.KindAsteriskToken | ||
| } | ||
| case '+': | ||
| if s.charAt(1) == '+' && isConflictMarkerTrivia(s.text, s.pos) { | ||
| s.pos = scanConflictMarkerTrivia(s.text, s.pos, s.errorAt) | ||
| if s.skipTrivia { | ||
| continue | ||
| } else { | ||
| s.token = ast.KindConflictMarkerTrivia | ||
| return s.token | ||
| } | ||
| } | ||
| next := s.charAt(1) | ||
| if next == '=' { | ||
| s.pos += 2 | ||
|
|
@@ -896,6 +914,15 @@ func (s *Scanner) Scan() ast.Kind { | |
| s.pos++ | ||
| s.token = ast.KindAtToken | ||
| case '\\': | ||
| if s.charAt(1) == '\\' && isConflictMarkerTrivia(s.text, s.pos) { | ||
| s.pos = scanConflictMarkerTrivia(s.text, s.pos, s.errorAt) | ||
| if s.skipTrivia { | ||
| continue | ||
| } else { | ||
| s.token = ast.KindConflictMarkerTrivia | ||
| return s.token | ||
| } | ||
| } | ||
| if s.scanIdentifier(0, identifierVariantStandard) { | ||
| s.token = GetIdentifierToken(s.tokenValue) | ||
| } else { | ||
|
|
@@ -2288,7 +2315,7 @@ func couldStartTrivia(text string, pos int) bool { | |
| // Characters that could start normal trivia | ||
| case '\r', '\n', '\t', '\v', '\f', ' ', '/', | ||
| // Characters that could start conflict marker trivia | ||
| '<', '|', '=', '>': | ||
| '<', '|', '=', '%', '\\', '+', '>': | ||
| return true | ||
| case '#': | ||
| // Only if its the beginning can we have #! trivia | ||
|
|
@@ -2371,7 +2398,7 @@ func SkipTriviaEx(text string, pos int, options *SkipTriviaOptions) int { | |
| continue | ||
| } | ||
| } | ||
| case '<', '|', '=', '>': | ||
| case '<', '|', '=', '%', '\\', '+', '>': | ||
| if isConflictMarkerTrivia(text, pos) { | ||
| pos = scanConflictMarkerTrivia(text, pos, nil) | ||
| canConsumeStar = false | ||
|
|
@@ -2412,8 +2439,8 @@ func isConflictMarkerTrivia(text string, pos int) bool { | |
| } | ||
|
|
||
| // Fast reject: a conflict marker is the same byte repeated seven times. If the | ||
| // second byte differs (the overwhelmingly common case for `<`, `>`, `=`, `|` | ||
| // tokens), it cannot be a marker, so skip the line-start check entirely. | ||
| // second byte differs (the overwhelmingly common case), it cannot be a marker, | ||
| // so skip the line-start check entirely. | ||
| if pos+1 >= len(text) || text[pos+1] != text[pos] { | ||
| return false | ||
| } | ||
|
|
@@ -2454,14 +2481,14 @@ func scanConflictMarkerTrivia(text string, pos int, reportError func(diag *diagn | |
| ch, size = utf8.DecodeRuneInString(text[pos:]) | ||
| } | ||
| } else { | ||
| if ch != '|' && ch != '=' { | ||
| panic("Assertion failed: ch must be either '|' or '='") | ||
| if ch != '|' && ch != '=' && ch != '%' && ch != '\\' && ch != '+' { | ||
| panic("Assertion failed: ch must be '|', '=', '%', '\\', or '+'") | ||
| } | ||
| // Consume everything from the start of a ||||||| or ======= marker to the start | ||
| // of the next ======= or >>>>>>> marker. | ||
| // Consume everything from the start of an intermediate marker to the | ||
| // start of the next intermediate marker or the overall >>>>>>> marker. | ||
| for pos < length { | ||
| currentChar := text[pos] | ||
| if (currentChar == '=' || currentChar == '>') && rune(currentChar) != ch && isConflictMarkerTrivia(text, pos) { | ||
| if (currentChar == '=' || currentChar == '%' || currentChar == '\\' || currentChar == '+' || currentChar == '>') && rune(currentChar) != ch && isConflictMarkerTrivia(text, pos) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe snapshot style is also missing some delimiter diagnostics. In this example, https://docs.jj-vcs.dev/latest/conflicts/#alternative-conflict-marker-styles
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this also affects
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, this is easy to fix by adding |
||
| break | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| conflictMarkerJujutsuTrivia1.ts(3,1): error TS1185: Merge conflict marker encountered. | ||
| conflictMarkerJujutsuTrivia1.ts(4,1): error TS1185: Merge conflict marker encountered. | ||
| conflictMarkerJujutsuTrivia1.ts(5,1): error TS1185: Merge conflict marker encountered. | ||
| conflictMarkerJujutsuTrivia1.ts(8,1): error TS1185: Merge conflict marker encountered. | ||
| conflictMarkerJujutsuTrivia1.ts(10,1): error TS1185: Merge conflict marker encountered. | ||
|
|
||
|
|
||
| ==== conflictMarkerJujutsuTrivia1.ts (5 errors) ==== | ||
| class C { | ||
| foo() { | ||
| <<<<<<< conflict 1 of 1 | ||
| ~~~~~~~ | ||
| !!! error TS1185: Merge conflict marker encountered. | ||
| %%%%%%% diff from: puqzxvrm 9be6077e "call c()" | ||
| ~~~~~~~ | ||
| !!! error TS1185: Merge conflict marker encountered. | ||
| \\\\\\\ to: lnqukurm 3504bbcc "call a()" | ||
| ~~~~~~~ | ||
| !!! error TS1185: Merge conflict marker encountered. | ||
| - c(); | ||
| + a(); | ||
| +++++++ uxvuvmmx 392ce8b5 "call b()" | ||
| ~~~~~~~ | ||
| !!! error TS1185: Merge conflict marker encountered. | ||
| b(); | ||
| >>>>>>> conflict 1 of 1 ends | ||
| ~~~~~~~ | ||
| !!! error TS1185: Merge conflict marker encountered. | ||
| } | ||
|
|
||
| public bar() { } | ||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| //// [tests/cases/compiler/conflictMarkerJujutsuTrivia1.ts] //// | ||
|
|
||
| //// [conflictMarkerJujutsuTrivia1.ts] | ||
| class C { | ||
| foo() { | ||
| <<<<<<< conflict 1 of 1 | ||
| %%%%%%% diff from: puqzxvrm 9be6077e "call c()" | ||
| \\\\\\\ to: lnqukurm 3504bbcc "call a()" | ||
| - c(); | ||
| + a(); | ||
| +++++++ uxvuvmmx 392ce8b5 "call b()" | ||
| b(); | ||
| >>>>>>> conflict 1 of 1 ends | ||
| } | ||
|
|
||
| public bar() { } | ||
| } | ||
|
|
||
|
|
||
| //// [conflictMarkerJujutsuTrivia1.js] | ||
| "use strict"; | ||
| class C { | ||
| foo() { | ||
| } | ||
| bar() { } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| //// [tests/cases/compiler/conflictMarkerJujutsuTrivia1.ts] //// | ||
|
|
||
| === conflictMarkerJujutsuTrivia1.ts === | ||
| class C { | ||
| >C : Symbol(C, Decl(conflictMarkerJujutsuTrivia1.ts, 0, 0)) | ||
|
|
||
| foo() { | ||
| >foo : Symbol(C.foo, Decl(conflictMarkerJujutsuTrivia1.ts, 0, 9)) | ||
|
|
||
| <<<<<<< conflict 1 of 1 | ||
| %%%%%%% diff from: puqzxvrm 9be6077e "call c()" | ||
| \\\\\\\ to: lnqukurm 3504bbcc "call a()" | ||
| - c(); | ||
| + a(); | ||
| +++++++ uxvuvmmx 392ce8b5 "call b()" | ||
| b(); | ||
| >>>>>>> conflict 1 of 1 ends | ||
| } | ||
|
|
||
| public bar() { } | ||
| >bar : Symbol(C.bar, Decl(conflictMarkerJujutsuTrivia1.ts, 10, 3)) | ||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| //// [tests/cases/compiler/conflictMarkerJujutsuTrivia1.ts] //// | ||
|
|
||
| === conflictMarkerJujutsuTrivia1.ts === | ||
| class C { | ||
| >C : C | ||
|
|
||
| foo() { | ||
| >foo : () => void | ||
|
|
||
| <<<<<<< conflict 1 of 1 | ||
| %%%%%%% diff from: puqzxvrm 9be6077e "call c()" | ||
| \\\\\\\ to: lnqukurm 3504bbcc "call a()" | ||
| - c(); | ||
| + a(); | ||
| +++++++ uxvuvmmx 392ce8b5 "call b()" | ||
| b(); | ||
| >>>>>>> conflict 1 of 1 ends | ||
| } | ||
|
|
||
| public bar() { } | ||
| >bar : () => void | ||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| // @target: es2015 | ||
| class C { | ||
| foo() { | ||
| <<<<<<< conflict 1 of 1 | ||
| %%%%%%% diff from: puqzxvrm 9be6077e "call c()" | ||
| \\\\\\\ to: lnqukurm 3504bbcc "call a()" | ||
| - c(); | ||
| + a(); | ||
| +++++++ uxvuvmmx 392ce8b5 "call b()" | ||
| b(); | ||
| >>>>>>> conflict 1 of 1 ends | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the missing-newline example would be useful coverage too: it puts https://docs.jj-vcs.dev/latest/conflicts/#conflicts-with-missing-terminating-newline |
||
| } | ||
|
|
||
| public bar() { } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this still misses Jujutsu’s longer markers, e.g.
%%%%%%%%%%%%%%% diff from: base, because the existing check expects a space after the seventh character.https://docs.jj-vcs.dev/latest/conflicts/#long-conflict-markers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think just accepting “seven or more” characters would still be problematic: jj lengthens markers so shorter marker-like lines can remain ordinary content. For example,
===========can appear inside a conflict delimited by fifteen-character markers, so we’d need to match the delimiter width.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I wonder if this is really necessary. TS doesn't try to account for ambiguous/misleading markers in its current Git support, right?
I guess it's possible to construct a situation where a
>>>>>>>inside a string or whatever tricks the parser into thinking it's a conflict marker, but it seems really unlikely to happen by accident, and the consequences wouldn't be that bad. I'm not sure if it's worth adding extra state in the scanner just to avoid it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's not worth it if it rarely happens; I was just reading up the docs since I wasn't sure what stuff jj did extra over git.