conflicts: allow missing newlines at end of file (based on [noeol] marker)
#5181
+383
−110
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
scott2000
force-pushed
the
conflict-missing-newline
branch
from
9c2adc1 to
44f1b0d
Compare
|
Another option is to reconstruct the missing eol state from the original tree. Something like, if the last parsed hunk is a conflict, check if the corresponding file content had newline at end, if not, strip newline from the parsed content. |
4 tasks
scott2000
changed the title
[noeol] marker)
Yeah, that could also work. I made a separate PR with a possible implementation of that here: #5186. It's definitely a lot simpler, but it does have the downside that it's no longer possible to compare the changes to the EOL state between the sides by looking at the materialized file. |
scott2000
force-pushed
the
conflict-missing-newline
branch
from
44f1b0d to
0c2528a
Compare
Using a chain of methods on `ConflictMarkerLine` will make it easier to add a method for setting a `no_eol` flag on a conflict marker in the next commit.
Currently, conflict markers are materialized in a format that cannot be parsed if a conflict appears at the end of the file, and there is a non-empty line which doesn't end with a newline character. To fix this issue, we can add an extra newline to every term of the conflict when this occurs and then ignore the added newlines when parsing the conflict. To ensure unambiguous parsing, we can add a "[noeol]" tag to the start conflict marker (`<<<<<<<`) when this occurs. I think this is a good solution to the problem because it should remain understandable to users even if they aren't familiar with file encodings. Many editors represent newlines at the end of files by showing an empty line at the end of the editor, so it should be intuitive that a term ending with an empty line represents a newline, while a term that doesn't end with an empty line represents the lack of a newline. Also, adding a newline to every term of the hunk is easier than keeping track of which terms have added newlines and which don't, so it makes materialization/parsing easier. For instance, a conflict with terms `["left\n", "base", "base\nright"]` would look like: ``` <<<<<<< Conflict 1 of 1 [noeol] +++++++ Contents of side jj-vcs#1 left %%%%%%% Changes from base to side jj-vcs#2 base +right >>>>>>> ```
scott2000
force-pushed
the
conflict-missing-newline
branch
from
0c2528a to
af4e2d2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3968 (this is an alternative to #4088). This is a rough draft of one possible solution. It's similar to what @yuja proposed with adding the "noeol" indicator to the comment part of the conflict marker, but I only added it to the
<<<<<<<start marker. I think this makes the materialization and parsing easier since we don't need to handle adding markers for both the removes and adds in%%%%%%%diff sections, but I'm open to comments and other approaches.Checklist
If applicable:
CHANGELOG.md