-
Notifications
You must be signed in to change notification settings - Fork 344
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
conflcts.rs: label conflict markers with conflict #, side #, whether it's a side or a diff #3459
Conversation
453fac6
to
ac73da5
Compare
0a16107
to
72c5e79
Compare
96528c1
to
1698e37
Compare
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'd really like to see this. Just a quick nit after a basic first pass.
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.
Makes sense. Thanks! Sorry it took me so long to get to this PR.
…by a label The format is 7 characters of the separator followed by a space and arbitrary text, followed by a newline. Separator followed by a newline is also allowed. E.g.: <<<<<<< Random text %%%%%%% Random text line 2 -line 3 +left line 4 +++++++ Random text right %%%%%%% Random text line 2 +forward line 3 line 4 >>>>>>> Random text This commit only allows reading such conflicts. I considered allowing longer separators (`<<<<<<<<<<<<<< Random text`), but we wouldn't currently write them, so let's be strict for now. 7 characters if they are followed by a space and arbitrary text
For example, ``` <<<<<<< Conflict 1 of 3 +++++++ Contents of side #1 left 3.1 left 3.2 left 3.3 %%%%%%% Changes from base to side #2 -line 3 +right 3.1 >>>>>>> ``` or ``` <<<<<<< Conflict 1 of 1 %%%%%%% Changes from base to side #1 -line 3 +right 3.1 +++++++ Contents of side #2 left 3.1 left 3.2 left 3.3 >>>>>>> ``` Currently, there is no way to disable these, this is TODO for a future PR. Other TODOs for future PRs: make these labels configurable. After that, we could support a `diff3/git`-like conflict format as well, in principle. Counting conflicts helps with knowing whether you fixed all the conflicts while you are in the editor. While labeling "side #1", etc, does not tell you the commit id or description as requested in jj-vcs#1176, I still think it's an improvement. Most importantly, I hope this will make `jj`'s conflict format less scary-looking for new users. I've used this for a bit, and I like it. Without the labels, I would see that the two conflicts have a different order of conflict markers, but I wouldn't be able to remember what that means. For longer diffs, it can be tricky for me to quickly tell that it's a diff as opposed to one of the sides. This also creates some hope of being able to navigate a conflict with more than 2 sides. Another not-so-secret goal for this is explained in jj-vcs#3109 (comment). The idea is a little weird, but I *think* it could be helpful, and I'd like to experiment with it.
Thank you for the review! |
1 similar comment
Thank you for the review! |
This follows up on #3459 and adds a label to the closing delimeter of each conflict, e.g. "Conflict 1 of 3 ends". I didn't initially put any label at the ending delimeter since the starting delimeter is already marked with "Conflict 1 of 3". However, I'm now realizing that when I resolve conflicts, I usually go from top to bottom. The first thing I do is delete the starting conflict delimeter. It is when I get to the *end* of the conflict that I wonder whether there are any more conflicts left in the file.
For example,
or
Before this PR, I found the difference between such conflicts to be quite hard to keep track of as a user.
Currently, there is no way to disable these, this is TODO for a future PR. Other TODOs for future PRs: make these labels configurable. After that, we could
support a
diff3/git
-like conflict format as well, in principle.Counting conflicts helps with knowing whether you fixed all the conflicts while
you are in the editor.
While labeling "side #1", etc, does not tell you the commit id or description as
requested in #1176, I still think it's an improvement. Most importantly, I hope this will
make
jj
's conflict format less scary-looking for new users.I used it for a bit, and I like it. Without the labels, I would see that the two
conflicts have a different order of conflict markers, but I wouldn't be able to
remember what that means. For longer diffs, it can be tricky for me to quickly
tell that it's a diff as opposed to one of the sides. This also creates some
hope of being able to navigate a conflict with more than 2 sides.
Another not-so-secret goal for this is explained in
#3109 (comment). The idea is
a little weird, but I think it could be helpful, and I'd like to experiment
with it.
Checklist
If applicable:
CHANGELOG.md