Skip to content
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

Materialized conflicts in a deleted file have no context lines #1244

Closed
chooglen opened this issue Feb 14, 2023 · 2 comments
Closed

Materialized conflicts in a deleted file have no context lines #1244

chooglen opened this issue Feb 14, 2023 · 2 comments
Labels
🐛bug Something isn't working good first issue Good for newcomers

Comments

@chooglen
Copy link
Contributor

Description

When jj materializes a conflict in a file that was deleted, no context lines are shown, e.g. the file is materialized like:

>>>>>>>
<<<<<<<
%%%%%%%
+    enum config_scope parsing_scope;
>>>>>>>

instead of seeing the change in its original context, like:

struct config_stack {
    struct config_source *head;
+    enum config_scope parsing_scope;
};

I typically encounter this when I rebase a change onto a renamed file. e.g. prior to #1061 getting merged, I had WIP changes on commands.rs. That PR renames commands.rs -> commands/mod.rs (and more), so when I rebased my WIP changes onto that PR, I see a context-less conflict on commands.rs.

Since the lines have no context, it takes me a while to figure out where I should move my changes to. I end up resorting to jj obslog to find the lines in some previous version and then manually applying the changes to the right place.

Besides rename detection, a possible fix is to show context lines from the undeleted side. This would require special-casing in the event of a deleted file, since we otherwise always show the left side as the base.

@chooglen chooglen added the 🐛bug Something isn't working label Feb 14, 2023
@martinvonz
Copy link
Member

@martinvonz martinvonz added the good first issue Good for newcomers label Feb 14, 2023
@martinvonz
Copy link
Member

In case anyone wants to reproduce this, you can simply create a file with some lines in a base commit, then delete it one child, modify it in a sibling, and then merge the two commits.

martinvonz added a commit that referenced this issue Feb 17, 2023
When we materialize modify/delete conflicts, we currently don't
include any context lines. That's because modify/delete conflicts have
only two sides, so there's no common base to compare to. Hunks that
are unchanged on the "modify" side are therefore not considered
conflicting, and since they they don't contribute new changes, they're
simply skipped (here:
https://github.com/martinvonz/jj/blob/3dfedf581458f88982289f58db884bc65f1e635b/lib/src/files.rs#L228-L230).

It seems more useful to instead pretend that the missing side is an
empty file. That way we'll get a conflict in the entire file.

We can still decide later to make e.g. `jj resolve` prompt the user on
modify/delete conflicts just like `hg resolve` does (or maybe it
actually happens earlier there, I don't remember).

Closes #1244.
martinvonz added a commit that referenced this issue Feb 17, 2023
When we materialize modify/delete conflicts, we currently don't
include any context lines. That's because modify/delete conflicts have
only two sides, so there's no common base to compare to. Hunks that
are unchanged on the "modify" side are therefore not considered
conflicting, and since they they don't contribute new changes, they're
simply skipped (here:
https://github.com/martinvonz/jj/blob/3dfedf581458f88982289f58db884bc65f1e635b/lib/src/files.rs#L228-L230).

It seems more useful to instead pretend that the missing side is an
empty file. That way we'll get a conflict in the entire file.

We can still decide later to make e.g. `jj resolve` prompt the user on
modify/delete conflicts just like `hg resolve` does (or maybe it
actually happens earlier there, I don't remember).

Closes #1244.
martinvonz added a commit that referenced this issue Feb 17, 2023
When we materialize modify/delete conflicts, we currently don't
include any context lines. That's because modify/delete conflicts have
only two sides, so there's no common base to compare to. Hunks that
are unchanged on the "modify" side are therefore not considered
conflicting, and since they they don't contribute new changes, they're
simply skipped (here:
https://github.com/martinvonz/jj/blob/3dfedf581458f88982289f58db884bc65f1e635b/lib/src/files.rs#L228-L230).

It seems more useful to instead pretend that the missing side is an
empty file. That way we'll get a conflict in the entire file.

We can still decide later to make e.g. `jj resolve` prompt the user on
modify/delete conflicts just like `hg resolve` does (or maybe it
actually happens earlier there, I don't remember).

Closes #1244.
martinvonz added a commit that referenced this issue Feb 17, 2023
When we materialize modify/delete conflicts, we currently don't
include any context lines. That's because modify/delete conflicts have
only two sides, so there's no common base to compare to. Hunks that
are unchanged on the "modify" side are therefore not considered
conflicting, and since they they don't contribute new changes, they're
simply skipped (here:
https://github.com/martinvonz/jj/blob/3dfedf581458f88982289f58db884bc65f1e635b/lib/src/files.rs#L228-L230).

It seems more useful to instead pretend that the missing side is an
empty file. That way we'll get a conflict in the entire file.

We can still decide later to make e.g. `jj resolve` prompt the user on
modify/delete conflicts just like `hg resolve` does (or maybe it
actually happens earlier there, I don't remember).

Closes #1244.
martinvonz added a commit that referenced this issue Feb 18, 2023
When we materialize modify/delete conflicts, we currently don't
include any context lines. That's because modify/delete conflicts have
only two sides, so there's no common base to compare to. Hunks that
are unchanged on the "modify" side are therefore not considered
conflicting, and since they they don't contribute new changes, they're
simply skipped (here:
https://github.com/martinvonz/jj/blob/3dfedf581458f88982289f58db884bc65f1e635b/lib/src/files.rs#L228-L230).

It seems more useful to instead pretend that the missing side is an
empty file. That way we'll get a conflict in the entire file.

We can still decide later to make e.g. `jj resolve` prompt the user on
modify/delete conflicts just like `hg resolve` does (or maybe it
actually happens earlier there, I don't remember).

Closes #1244.
martinvonz added a commit that referenced this issue Feb 18, 2023
When we materialize modify/delete conflicts, we currently don't
include any context lines. That's because modify/delete conflicts have
only two sides, so there's no common base to compare to. Hunks that
are unchanged on the "modify" side are therefore not considered
conflicting, and since they they don't contribute new changes, they're
simply skipped (here:
https://github.com/martinvonz/jj/blob/3dfedf581458f88982289f58db884bc65f1e635b/lib/src/files.rs#L228-L230).

It seems more useful to instead pretend that the missing side is an
empty file. That way we'll get a conflict in the entire file.

We can still decide later to make e.g. `jj resolve` prompt the user on
modify/delete conflicts just like `hg resolve` does (or maybe it
actually happens earlier there, I don't remember).

Closes #1244.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants