pijul_org / pijul

#44 CLI: record can crate self-conflicting patches

Opened by pmeunier, on April 10, 2017
Closed
pmeunier commented on April 10, 2017

Steps to reproduce:

  • pijul init
  • Create a single file with a few lines, and pijul add it.
  • pijul record -a
  • Change the contents of a single line in the middle of the file.
  • pijul record, and only select the additions (lines starting with a green "+").
  • pijul revert yields a conflict.

How to solve: when presenting changes to the user, show replacements together. Replacements are deletions followed by additions, or deletions followed by additions (not sure this last case can happen), where the root of the first edge of a deleted block is the up context of the addition.

lthms commented on April 10, 2017

How to solve: when presenting changes to the user, show replacements together. Replacements are deletions followed by additions, or deletions followed by additions (not sure this last case can happen), where the root of the first edge of a deleted block is the up context of the addition.

Even without considering this issue, from a UI point of view, it would be very convinent I think! Git for instance (I know you don’t like it d: but it has some virtue), git add -p has this behaviour and I find it more usable than pijul record.

pmeunier commented on April 10, 2017
  1. I neither like nor dislike git.
  2. In git, it is not a part of git, but of diff. Pijul needs a different diff algorithm because it knows about conflicts.
lthms commented on April 10, 2017

There is the underlying functionning (commit vs. patch) and there is the UI. Maybe the way git add -p is a happy consequence of how git works. I know pijul works in a (very) different manner and I can imagine you will not have the same behaviour using the same strategy. My point was just to say I would love to have this new diff algorithm even if it were not needed to solve this particular issue.

I am sorry if I express myself poorly.

pmeunier commented on April 10, 2017

I am sorry if I express myself poorly.

My previous comment was unclear and not nice, sorry about that, I was in the middle of debugging this issue : (

Now, what I meant was, the diff algorithm used by git/darcs/mercurial/svn/diff/… uses the property that lines are ordered linearly, which means the problem is equivalent to LCS (longest common subsequence).

In Pijul, this is not true, and the problem of computing the diff in our case is actually NP-complete. We use an approximation algorithm, with the same complexity as diff.

Now, unfortunately, showing the results of that to the user correctly in all cases is challenging. Even showing proper "replacements" is nontrivial.

pmeunier commented on April 10, 2017

"nontrivial" doesn't mean we won't do it, though! And the comparison with git is totally fair: if we claim we can have a better UX, we have to compare.

lthms commented on April 10, 2017

My previous comment was unclear and not nice, sorry about that, I was in the middle of debugging this issue : (

No worries! That’s the issue with plain text.

Now, unfortunately, showing the results of that to the user correctly in all cases is challenging. Even showing proper "replacements" is nontrivial.

Maybe exposing a set of “closed enough” changes and having a way to split it automatically does the trick from an UX perspective. Now, that does not fix your issue about self-conflicted patches.

Thanks for the explanation by the way! I really appreciate.

pmeunier commented on April 11, 2017

Fixed.