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

FR: Materialize files with conflict markers (or any files) in a way that can be parsed #3975

Open
ilyagr opened this issue Jun 26, 2024 · 2 comments · May be fixed by #4088
Open

FR: Materialize files with conflict markers (or any files) in a way that can be parsed #3975

ilyagr opened this issue Jun 26, 2024 · 2 comments · May be fixed by #4088

Comments

@ilyagr
Copy link
Contributor

ilyagr commented Jun 26, 2024

This is a tracking issue for this bug/FR. I'm promoting this discussion from #3968 (comment). I think I also have a plan now that could work, so I am posting an outline it here in case people have comments.

The alternative would be to declare that it is not a goal to be able to materialize any text file conflict, and show a materialization error instead. I was leaning towards this, but I think the following approach is simple enough to work.



Originally posted by @ilyagr in #3968 (comment)

Wouldn't you want to preserve the invariant that you can reconstruct any of the file sides directly from the conflicted contents, rather than having to consult an out-of-band store?

This issue has been nerd-sniping me for a while; in addition to trailing newlines (Update: in non-empty files; we actually support files that are 0 or more lines, each ending with a newline), it's also a matter of encoding conflict markers inside files


TODO: find link. I remember discussing it with Martin recently in some PR, haven't found it yet. TLDR: jj currently can't handle conflicts in out own tutorial.md because of

https://github.com/martinvonz/jj/blob/638649b435795bbb991d840e9ad1750648d9b74a/docs/tutorial.md?plain=1#L284-L291


In the shortest term, I'll probably ignore both issues and just add newlines to the end of files.

However, I think I finally figured out a syntax that will work, will not require a complicated parser, and even be somewhat readable. Before computing conflicts (and only if a file is conflicted), we'd encode file contents as follows:

  1. A mildly pathological file
JJ Verbatim Line:<<<<<<<<<<<
blahblah
JJ EOF: No newline at the end of file
  1. A moderately pathological file
JJ Verbatim Line:<<<<<<<<<<<
JJ Verbatim Line:JJ Verbatim Line:<<<<<<<<<<<
blahblah
JJ Verbatim Line:JJ EOF: No newline at the end of file
  1. Another fun file
blah
JJ Verbatim Line:JJ EOF: No newline at the end of file
JJ EOF: No newline at the end of file

Fun, huh?


This relies on conflict markers having to start at the beginning of the line. If we ever have a notion of conflicts that happen in the middle of lines, we'd need a different syntax, of course. I think that would be nice, but also would probably need a whole different UI to be usable; I'm not sure it can be very usable in the terminal or as a text file.

@ilyagr
Copy link
Contributor Author

ilyagr commented Jun 26, 2024

TODO: Check if there are problems with diffing non-empty files that don't end in a newline.

If we do JJ EOF: No newline at the end of file for conflicts, it would be nice to do it for diffs as well, but that's a slightly separate issue.

ilyagr added a commit to ilyagr/jj that referenced this issue Jun 26, 2024
This happens for non-empty files only; the materialization code can
handle files and hunks with 0 or more lines, but each line should
terminate in a newline.

This is an imperfect fix to jj-vcs#3968, a better fix is TODO; see jj-vcs#3975.
ilyagr added a commit to ilyagr/jj that referenced this issue Jun 26, 2024
This happens for non-empty files only; the materialization code can
handle files and hunks with 0 or more lines, but each line should
terminate in a newline.

This is an imperfect fix to jj-vcs#3968, a better fix is TODO; see jj-vcs#3975.
ilyagr added a commit to ilyagr/jj that referenced this issue Jun 27, 2024
This happens for non-empty files only; the materialization code can
handle files and hunks with 0 or more lines, but each line should
terminate in a newline.

This is an imperfect fix to jj-vcs#3968, a better fix is TODO; see jj-vcs#3975.

This also implements FromIterator for Merge. I could split it into a
separate commit, but then it'd have no uses.
ilyagr added a commit to ilyagr/jj that referenced this issue Jun 27, 2024
This happens for non-empty files only; the materialization code can
handle files and hunks with 0 or more lines, but each line should
terminate in a newline.

This is an imperfect fix to jj-vcs#3968, a better fix is TODO; see jj-vcs#3975.

This also implements FromIterator for Merge. I could split it into a
separate commit, but then it'd have no uses.
ilyagr added a commit to ilyagr/jj that referenced this issue Jun 27, 2024
This happens for non-empty files only; the materialization code can
handle files and hunks with 0 or more lines, but each line should
terminate in a newline.

This is an imperfect fix to jj-vcs#3968, a better fix is TODO; see jj-vcs#3975.

This also implements FromIterator for Merge. I could split it into a
separate commit, but then it'd have no uses.
ilyagr added a commit to ilyagr/jj that referenced this issue Jun 27, 2024
This happens for non-empty files only; the materialization code can
handle files and hunks with 0 or more lines, but each line should
terminate in a newline.

This is an imperfect fix to jj-vcs#3968, a better fix is TODO; see jj-vcs#3975.

This also implements FromIterator for Merge. I could split it into a
separate commit, but then it'd have no uses.
ilyagr added a commit to ilyagr/jj that referenced this issue Jul 15, 2024
ilyagr added a commit to ilyagr/jj that referenced this issue Jul 15, 2024
ilyagr added a commit to ilyagr/jj that referenced this issue Jul 15, 2024
ilyagr added a commit to ilyagr/jj that referenced this issue Jul 15, 2024
ilyagr added a commit to ilyagr/jj that referenced this issue Jul 15, 2024
ilyagr added a commit to ilyagr/jj that referenced this issue Jul 15, 2024
ilyagr added a commit to ilyagr/jj that referenced this issue Jul 15, 2024
ilyagr added a commit to ilyagr/jj that referenced this issue Jul 15, 2024
ilyagr added a commit to ilyagr/jj that referenced this issue Jul 15, 2024
ilyagr added a commit to ilyagr/jj that referenced this issue Jul 15, 2024
@ilyagr
Copy link
Contributor Author

ilyagr commented Jul 26, 2024

Just FYI, there's a long discussion with @yuja in #4088, which is leading to a different approach to this problem.

The main limitation of the approach I suggested here is that jj would have to, at some point, replace the file with \JJ Verbatim Line: markers with the "decoded" plain file when the user wants the conflicts resolved. This could happen automatically (creating some corner cases and potentially surprising the user) or require a user command.

The alternative we are discussing is to use longer conflict markers if the file with the conflict already contained some conflict marker-looking text before there were conflicts. The hope is that we can find out the necessary length of the conflict markers from the original conflict, and thus know when the file is resolved. See the discussion for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant