Re: [PATCH] add -p: obey diff.noprefix option if set

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/03/2023 08:40, Jeff King wrote:
On Sat, Mar 04, 2023 at 01:39:00PM +0100, Marcel Partap wrote:
There are two options, I think.

One is that we have a similar issue with color. To handle that, we
generate the diff twice, once with color and once without. We could
probably do the same thing here, by sticking the "--no-prefix" part with
the color setup. Though it turns out to be a little tricky to do because
of the way the code is written, and IIRC there are probably some corner
cases lurking (e.g., after splitting, I think we'll try to re-colorize
the diff headers ourselves).

The second is to just remember that we set noprefix and to add the
matching "-p0". Unfortunately we have to do so in a few places, but it's
not _too_ bad (and possibly some refactoring could make it less ugly).
Something like:

I think that is the better approach. Looking at how we handle diff.algorithm we should maybe add a "noprefix" member to "struct add_i_state" and initialize it in init_add_i_state() (which is in add-interactive.c). That way we're consistent with the existing code and we don't need to keep calling git_config_get_bool() whenever we want the value of diff.noPrefix.

diff --git a/add-patch.c b/add-patch.c
index 520faae9cba..6e5390621c0 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1189,13 +1189,16 @@ static int run_apply_check(struct add_p_state *s,
  			   struct file_diff *file_diff)
  {
  	struct child_process cp = CHILD_PROCESS_INIT;
+	int noprefix;
strbuf_reset(&s->buf);
  	reassemble_patch(s, file_diff, 1, &s->buf);
setup_child_process(s, &cp,
  			    "apply", "--check", NULL);
  	strvec_pushv(&cp.args, s->mode->apply_check_args);
+	if (!git_config_get_bool("diff.noprefix", &noprefix) && noprefix)
+		strvec_pushf(&cp.args, "-p1");

I think you meant "-p0" here

Best Wishes

Phillip

  	if (pipe_command(&cp, s->buf.buf, s->buf.len, NULL, 0, NULL, 0))
  		return error(_("'git apply --cached' failed"));
@@ -1695,7 +1698,10 @@ static int patch_update_file(struct add_p_state *s,
  			apply_for_checkout(s, &s->buf,
  					   s->mode->is_reverse);
  		else {
+			int noprefix;
  			setup_child_process(s, &cp, "apply", NULL);
+			if (!git_config_get_bool("diff.noprefix", &noprefix) && noprefix)
+				strvec_pushf(&cp.args, "-p0");
  			strvec_pushv(&cp.args, s->mode->apply_args);
  			if (pipe_command(&cp, s->buf.buf, s->buf.len,
  					 NULL, 0, NULL, 0))

  add-patch.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

We'd probably want at least one test using "add -p" with diff.noprefix
(probably in t3701). That would demonstrate that the feature works, as
well as protect it from future regressions (the test suite doesn't fail
even with your broken patch because no test sets noprefix).

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux