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

Attempt at a unified diff output format for use with --replace #2149

Closed
wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 23, 2022

This is a draft for what could be a unified diff output for ripgrep when using the --replace option.

The chance for acceptance is low considering the feature request #74 was closed,
however this is an implementation of @samuelcolvin suggestion to produce a diff output format
instead of actually writing to files. The job of changing files can then be handed to something like patch or git apply.

I wanted to at least investigate what this would look like and managed to get something working,
so I hope this can be a way to discuss around some actual code. @BurntSushi stated they were
actively looking to moving more functionality into a library, so I'm not sure if this was totally dismissed.

I ran into a few problems with the replacement buffer which seems to contain more than the original match
and I managed to reproduce this with ripgrep 13.0.0, notes are in the code. I managed to get it working by
modifying the replacement, not sure but it could be related to #2095.
There is also no support for additional context, as that would require keeping track of it before and after a match
to then produce the correct hunk header with context included, and I'm not very familiar with the code or rust
to come up with a nice way to do that.

The implementation is mostly taken from the JSON printer with necessary parts replaced.
It can handle multiline replacements that differ in the amount of lines before/after replacement.

I've tested mostly with running the following in the ripgrep repo:

rg -U '\.A\n' --replace 'BB'
rg -U '.A\nB' --replace 'BB' UNLICENSE

as these also produced the spurious output I found with rg 13.0.0.

Outstanding stuff to leave draft status:

  • unit tests
  • better flag handling (--diff should somehow require you to use --replace)

@ghost ghost marked this pull request as draft February 23, 2022 20:14
@ghost ghost changed the title attempt to hack together a diff output Attempt at a unified diff output format for use with --replace Feb 23, 2022
// not removed. This seems to fix that, but it's not pretty and might be wrong.
dst.truncate(dst.len() - extension_len);
} else {
// Restore the line terminator.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably wrong, but I'm relying on the original line terminators instead of reproducing it in the printer. I'm not sure how to fix it properly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least all tests seem to pass.

Skip matches beyond the original range when
replacing a buffer with captures.
@BurntSushi
Copy link
Owner

Thanks for working on this. But as discussed in #74, this really isn't something I want to support. It's the kind of feature that begs more features. I don't want it in ripgrep. I think search-and-replace is a complicated enough task that some other tool should try to solve it. (And indeed, several have. But the UX is difficult to get right I think. Yet another reason I don't want to touch this particular problem.)

@BurntSushi BurntSushi closed this Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant