I ran across this while writing some doc/manual stuff, so I haven’t taken the time to look into it too deeply; in some casesbut if Alice and Bob begin with the file:

fn length(name : String) -> usize {
  unimplemented!()
}

fn special_chars(name : String) -> usize {
  unimplemented!()
}

If Alice deletes special_chars and records, while Bob adds a third function and records:

fn numbers(name : String) -> usize {
  unimplemented!()
}

When Bob tries to merge Alice’s change, he ends up with:

fn length(name : String) -> usize {
  unimplemented!()
}

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
  unimplemented!()
}

fn numbers(name : String) -> usize {
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

Even if the function bodies are different, the closing bracket for Bob’s numbers still ends up above the beginning of the function declaration.

This isn’t really related to conflict markers: if you look at the changes involves (see the new conflict notation), you can see that the diffs have the same problem (I guess, otherwise it’s a bug).

@pmeunier Thanks, I guess this implicates the #530 changes you linked to in the other discussion.

@potocpav Do you have any input on whether this is the intended behavior?

I hope I’m not off the base, but this is how I see it. Alice’s changes look like this, which is expected:

  fn length(name : String) -> usize {
    unimplemented!()
  }
- 
- fn special_chars(name : String) -> usize {
-   unimplemented!()
- }

Bob’s changes look like this. This is not very natural, but it’s correct. It’s just because the diffing algorithm isn’t fine-tuned to produce natural results.

  fn length(name : String) -> usize {
    unimplemented!()
  }
  
  fn special_chars(name : String) -> usize {
+   unimplemented!()
+ }
+ 
+ fn numbers(name : String) -> usize {
    unimplemented!()
  }

If we try to combine these changes, we get something like this:

   fn length(name : String) -> usize {
     unimplemented!()
   }
-  
-  fn special_chars(name : String) -> usize {
 +   unimplemented!()
 + }
 + 
 + fn numbers(name : String) -> usize {
-    unimplemented!()
-  }

Bob’s addition ends up being “inside” the block of code deleted by Alice. We now have a dilemma: should this addition be deleted too, or not? This is what the conflict is about.

So I think this is the expected behavior, even if it is confusing. A workaround is probably to make the diffing algorithm smarter. I’m not sure if there is any other way to make the conflict more intuitive.

@potocpav Is this a product of the diff algorithm or pijul’s change format (or an interaction between the two)? This isn’t an area I’m too familiar with, but git shows the more intuitive diff using both Myers and Patience, so I’m curious what causes them to achieve different results.

@potocpav although this isn’t the scenario I was thinking when I wrote 621 I think the general idea of having conflict markers for removed lines might be useful in both cases.