imara-diff icon indicating copy to clipboard operation
imara-diff copied to clipboard

feat: word diffs

Open KnorpelSenf opened this issue 4 months ago • 1 comments

Adds a word diffing feature for each hunk. The following questions need to be answered to move this out of draft state:

EDIT: all done!

  1. The current implementation is based on bytes. This is consistent with how strings are used internally in other places. Is this useful for word-diffs? We could also use .chars() or .graphemes(true) (via this crate). It's also possible to offer several of these options. (EDIT: we now use words)
  2. The current implementation reuses the diff, but it reallocates the bytes of the underlying strings for every hunk. Is this acceptable? If no, which alternative should we go with? (EDIT: no longer applies since we no longer use bytes)
  3. The current implementation reuses the diff, but it reallocates the InternedInput for every hunk. Is this acceptable? If no, which alternative should we go with? (EDIT: we now reuse the interner)
  4. The current implementation estimates the token count to be 256 for byte token sources. Please let me know if that is an unreasonable heuristic for byte token sources. (EDIT: no longer applies since we no longer use bytes)
  5. The current implementation always uses the Myers algo. Should we offer a second method which performs a minimal diff? (EDIT: no, we can add that later if requested)
  6. Is there anything else I am missing?

Closes #1.

KnorpelSenf avatar Sep 27 '25 11:09 KnorpelSenf

@pascalkuthe @Byron I have addressed all comments. This is ready for review now.

KnorpelSenf avatar Nov 07 '25 17:11 KnorpelSenf

@pascalkuthe is there a way for to me make this easier to review?

KnorpelSenf avatar Nov 21 '25 20:11 KnorpelSenf

left one cooment otherwise lgtm

pascalkuthe avatar Nov 21 '25 21:11 pascalkuthe

Thanks! Can you allow CI to run?

KnorpelSenf avatar Nov 21 '25 22:11 KnorpelSenf

Seems the build failed due to the usual need to deref

pascalkuthe avatar Nov 21 '25 23:11 pascalkuthe

Let's try again

KnorpelSenf avatar Nov 22 '25 08:11 KnorpelSenf