* [PATCH] git-jump: ignore (custom) prefix in diff mode @ 2012-09-17 1:21 Mischa POSLAWSKY 2012-09-17 3:01 ` Mischa POSLAWSKY 2012-09-17 5:24 ` Junio C Hamano 0 siblings, 2 replies; 10+ messages in thread From: Mischa POSLAWSKY @ 2012-09-17 1:21 UTC (permalink / raw) To: git; +Cc: Jeff King Matching the default file prefix b/ does not yield any results if config option diff.noprefix or diff.mnemonicprefix is enabled. Signed-off-by: Mischa POSLAWSKY <git@shiar.nl> --- Very useful script otherwise; thanks. contrib/git-jump/git-jump | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git contrib/git-jump/git-jump contrib/git-jump/git-jump index a33674e..dc90cd6 100755 --- contrib/git-jump/git-jump +++ contrib/git-jump/git-jump @@ -21,9 +21,9 @@ open_editor() { } mode_diff() { - git diff --relative "$@" | + git diff --no-prefix --relative "$@" | perl -ne ' - if (m{^\+\+\+ b/(.*)}) { $file = $1; next } + if (m{^\+\+\+ (.*)}) { $file = $1; next } defined($file) or next; if (m/^@@ .*\+(\d+)/) { $line = $1; next } defined($line) or next; -- 1.7.12.165.g8cb9d9c ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] git-jump: ignore (custom) prefix in diff mode 2012-09-17 1:21 [PATCH] git-jump: ignore (custom) prefix in diff mode Mischa POSLAWSKY @ 2012-09-17 3:01 ` Mischa POSLAWSKY 2012-09-17 5:22 ` Junio C Hamano 2012-09-17 6:03 ` perryh 2012-09-17 5:24 ` Junio C Hamano 1 sibling, 2 replies; 10+ messages in thread From: Mischa POSLAWSKY @ 2012-09-17 3:01 UTC (permalink / raw) To: git; +Cc: Jeff King > diff --git contrib/git-jump/git-jump contrib/git-jump/git-jump > index a33674e..dc90cd6 100755 > --- contrib/git-jump/git-jump > +++ contrib/git-jump/git-jump Apparently diff.noprefix also applies to git format-patch. Even though git am does explicitly support -p0, I would argue against diff options creating non-standard patches. -- >8 -- Subject: [PATCH/RFC] format-patch: force default file prefixes in diff Override user configuration (eg. diff.noprefix) in patches intended for external consumption to match the default prefixes expected by git-am. Signed-off-by: Mischa POSLAWSKY <git@shiar.nl> --- builtin/log.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/log.c b/builtin/log.c index dff7921..91ded25 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1131,6 +1131,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.diff = 1; rev.max_parents = 1; DIFF_OPT_SET(&rev.diffopt, RECURSIVE); + rev.diffopt.a_prefix = "a/"; + rev.diffopt.b_prefix = "b/"; rev.subject_prefix = fmt_patch_subject_prefix; memset(&s_r_opt, 0, sizeof(s_r_opt)); s_r_opt.def = "HEAD"; -- 1.7.12.166.ga54f379 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] git-jump: ignore (custom) prefix in diff mode 2012-09-17 3:01 ` Mischa POSLAWSKY @ 2012-09-17 5:22 ` Junio C Hamano 2012-09-18 2:52 ` Mischa POSLAWSKY 2012-09-17 6:03 ` perryh 1 sibling, 1 reply; 10+ messages in thread From: Junio C Hamano @ 2012-09-17 5:22 UTC (permalink / raw) To: Mischa POSLAWSKY; +Cc: git, Jeff King Mischa POSLAWSKY <git@shiar.nl> writes: >> diff --git contrib/git-jump/git-jump contrib/git-jump/git-jump >> index a33674e..dc90cd6 100755 >> --- contrib/git-jump/git-jump >> +++ contrib/git-jump/git-jump > > Apparently diff.noprefix also applies to git format-patch. Even though > git am does explicitly support -p0, I would argue against diff options > creating non-standard patches. > > -- >8 -- > Subject: [PATCH/RFC] format-patch: force default file prefixes in diff > > Override user configuration (eg. diff.noprefix) in patches intended for > external consumption to match the default prefixes expected by git-am. > > Signed-off-by: Mischa POSLAWSKY <git@shiar.nl> > --- Not all projects expect to see a/ & b/ prefix and these are configurable for a reason. Robbing the choice that has been supported for quite a long time from them is an unacceptable regression. Why did you think this may be a good idea in the first place? Perhaps you had configured your diff.noprefix in a wrong configuration file? This is primarily per-project choice, and your clone of git.git should not have diff.noprefix set, neither your $HOME/.gitconfig unless you always work on projects that want diff.noprefix. > builtin/log.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/builtin/log.c b/builtin/log.c > index dff7921..91ded25 100644 > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -1131,6 +1131,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) > rev.diff = 1; > rev.max_parents = 1; > DIFF_OPT_SET(&rev.diffopt, RECURSIVE); > + rev.diffopt.a_prefix = "a/"; > + rev.diffopt.b_prefix = "b/"; > rev.subject_prefix = fmt_patch_subject_prefix; > memset(&s_r_opt, 0, sizeof(s_r_opt)); > s_r_opt.def = "HEAD"; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] git-jump: ignore (custom) prefix in diff mode 2012-09-17 5:22 ` Junio C Hamano @ 2012-09-18 2:52 ` Mischa POSLAWSKY 2012-09-18 3:57 ` Junio C Hamano 2012-09-18 9:00 ` Bert Wesarg 0 siblings, 2 replies; 10+ messages in thread From: Mischa POSLAWSKY @ 2012-09-18 2:52 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Jeff King Junio C Hamano skribis 2012-9-16 22:22 (-0700): > Mischa POSLAWSKY <git@shiar.nl> writes: > > > Subject: [PATCH/RFC] format-patch: force default file prefixes in diff > > > > Override user configuration (eg. diff.noprefix) in patches intended for > > external consumption to match the default prefixes expected by git-am. > > > > Signed-off-by: Mischa POSLAWSKY <git@shiar.nl> > > --- > > Not all projects expect to see a/ & b/ prefix and these are > configurable for a reason. Robbing the choice that has been > supported for quite a long time from them is an unacceptable > regression. My bad, I was assuming format-patch would mostly interact with git am. > Why did you think this may be a good idea in the first place? > > Perhaps you had configured your diff.noprefix in a wrong > configuration file? This is primarily per-project choice, and your > clone of git.git should not have diff.noprefix set, neither your > $HOME/.gitconfig unless you always work on projects that want > diff.noprefix. Then I'm not using it as intended. For me it's just a personal preference of how I'd like to review commits (diff/show) so I can easily copy-paste file names (less essential since my discovery of git jump, but still). It's not something I'd like to be communicated with any upstream project (format-patch). So it seems I'm asking for a new feature: to be able to configure local and inter-project diff options differently. In this case I'd be helped by either format.noprefix=0 or a to be bikeshedded localdiff.noprefix=1. I don't know about other options though. Does anybody actually want mnemonicprefix to be sent out as well? Another solution could be a single option defining behaviour exceptions: format.diff = normal | textconv | noconfig Expanding on the existing --(no-)textconv difference in format-patch. -- Mischa ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] git-jump: ignore (custom) prefix in diff mode 2012-09-18 2:52 ` Mischa POSLAWSKY @ 2012-09-18 3:57 ` Junio C Hamano 2012-09-18 9:00 ` Bert Wesarg 1 sibling, 0 replies; 10+ messages in thread From: Junio C Hamano @ 2012-09-18 3:57 UTC (permalink / raw) To: Mischa POSLAWSKY; +Cc: git, Jeff King Mischa POSLAWSKY <git@shiar.nl> writes: >> Perhaps you had configured your diff.noprefix in a wrong >> configuration file? This is primarily per-project choice, and your >> clone of git.git should not have diff.noprefix set, neither your >> $HOME/.gitconfig unless you always work on projects that want >> diff.noprefix. > > Then I'm not using it as intended. I would imagine that you could do (in ~/.gitconfig) [diff] noprefix (in ~/git.git/.git/config [diff] noprefix = false or something. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] git-jump: ignore (custom) prefix in diff mode 2012-09-18 2:52 ` Mischa POSLAWSKY 2012-09-18 3:57 ` Junio C Hamano @ 2012-09-18 9:00 ` Bert Wesarg 1 sibling, 0 replies; 10+ messages in thread From: Bert Wesarg @ 2012-09-18 9:00 UTC (permalink / raw) To: Mischa POSLAWSKY; +Cc: Junio C Hamano, git, Jeff King On Tue, Sep 18, 2012 at 4:52 AM, Mischa POSLAWSKY <git@shiar.nl> wrote: > Junio C Hamano skribis 2012-9-16 22:22 (-0700): > >> Mischa POSLAWSKY <git@shiar.nl> writes: >> >> > Subject: [PATCH/RFC] format-patch: force default file prefixes in diff >> > >> > Override user configuration (eg. diff.noprefix) in patches intended for >> > external consumption to match the default prefixes expected by git-am. >> > >> > Signed-off-by: Mischa POSLAWSKY <git@shiar.nl> >> > --- >> >> Not all projects expect to see a/ & b/ prefix and these are >> configurable for a reason. Robbing the choice that has been >> supported for quite a long time from them is an unacceptable >> regression. > > My bad, I was assuming format-patch would mostly interact with git am. > >> Why did you think this may be a good idea in the first place? >> >> Perhaps you had configured your diff.noprefix in a wrong >> configuration file? This is primarily per-project choice, and your >> clone of git.git should not have diff.noprefix set, neither your >> $HOME/.gitconfig unless you always work on projects that want >> diff.noprefix. > > Then I'm not using it as intended. For me it's just a personal > preference of how I'd like to review commits (diff/show) so I can easily > copy-paste file names (less essential since my discovery of git jump, > but still). It's not something I'd like to be communicated with any > upstream project (format-patch). > > So it seems I'm asking for a new feature: to be able to configure local > and inter-project diff options differently. In this case I'd be helped > by either format.noprefix=0 or a to be bikeshedded localdiff.noprefix=1. > I don't know about other options though. Does anybody actually want > mnemonicprefix to be sent out as well? I once had the same idea and posted a patch for it: http://article.gmane.org/gmane.comp.version-control.git/146215 The implementation differs though, the pure path without any mnemonicprefix is in its own line 'path <path>'. But my current patch also differs to this old one, in the current one, the path is at the end 'index' line. But I do not need this feature anymore. Bert > > Another solution could be a single option defining behaviour exceptions: > format.diff = normal | textconv | noconfig > Expanding on the existing --(no-)textconv difference in format-patch. > > -- > Mischa ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] git-jump: ignore (custom) prefix in diff mode 2012-09-17 3:01 ` Mischa POSLAWSKY 2012-09-17 5:22 ` Junio C Hamano @ 2012-09-17 6:03 ` perryh 1 sibling, 0 replies; 10+ messages in thread From: perryh @ 2012-09-17 6:03 UTC (permalink / raw) To: git; +Cc: peff, git Mischa POSLAWSKY <git@shiar.nl> wrote: > ... I would argue against diff options creating non-standard patches. Seems to me it might depend on what one means by "non-standard". I can envision cases in which increasing the number of context lines would result in the patch being more robust WRT applying correctly to a recipient's version that might be a bit different than the one against which it was created. OTOH one most likely does not want to create a patch with -b unless the apply tool also supports such and there is a way to communicate to the apply tool that -b was used in creating the patch. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] git-jump: ignore (custom) prefix in diff mode 2012-09-17 1:21 [PATCH] git-jump: ignore (custom) prefix in diff mode Mischa POSLAWSKY 2012-09-17 3:01 ` Mischa POSLAWSKY @ 2012-09-17 5:24 ` Junio C Hamano 2012-09-17 17:39 ` Jeff King 1 sibling, 1 reply; 10+ messages in thread From: Junio C Hamano @ 2012-09-17 5:24 UTC (permalink / raw) To: Mischa POSLAWSKY; +Cc: git, Jeff King Mischa POSLAWSKY <git@shiar.nl> writes: > Matching the default file prefix b/ does not yield any results if config > option diff.noprefix or diff.mnemonicprefix is enabled. > > Signed-off-by: Mischa POSLAWSKY <git@shiar.nl> > --- > Very useful script otherwise; thanks. > > contrib/git-jump/git-jump | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git contrib/git-jump/git-jump contrib/git-jump/git-jump > index a33674e..dc90cd6 100755 > --- contrib/git-jump/git-jump > +++ contrib/git-jump/git-jump > @@ -21,9 +21,9 @@ open_editor() { > } > > mode_diff() { > - git diff --relative "$@" | > + git diff --no-prefix --relative "$@" | > perl -ne ' > - if (m{^\+\+\+ b/(.*)}) { $file = $1; next } > + if (m{^\+\+\+ (.*)}) { $file = $1; next } > defined($file) or next; > if (m/^@@ .*\+(\d+)/) { $line = $1; next } > defined($line) or next; Makes sense to me. Peff? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] git-jump: ignore (custom) prefix in diff mode 2012-09-17 5:24 ` Junio C Hamano @ 2012-09-17 17:39 ` Jeff King 2012-09-17 19:48 ` Junio C Hamano 0 siblings, 1 reply; 10+ messages in thread From: Jeff King @ 2012-09-17 17:39 UTC (permalink / raw) To: Junio C Hamano; +Cc: Mischa POSLAWSKY, git On Sun, Sep 16, 2012 at 10:24:04PM -0700, Junio C Hamano wrote: > Mischa POSLAWSKY <git@shiar.nl> writes: > > > Matching the default file prefix b/ does not yield any results if config > > option diff.noprefix or diff.mnemonicprefix is enabled. > > > > Signed-off-by: Mischa POSLAWSKY <git@shiar.nl> > > --- > > Very useful script otherwise; thanks. > > > > contrib/git-jump/git-jump | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git contrib/git-jump/git-jump contrib/git-jump/git-jump > > index a33674e..dc90cd6 100755 > > --- contrib/git-jump/git-jump > > +++ contrib/git-jump/git-jump > > @@ -21,9 +21,9 @@ open_editor() { > > } > > > > mode_diff() { > > - git diff --relative "$@" | > > + git diff --no-prefix --relative "$@" | > > perl -ne ' > > - if (m{^\+\+\+ b/(.*)}) { $file = $1; next } > > + if (m{^\+\+\+ (.*)}) { $file = $1; next } > > defined($file) or next; > > if (m/^@@ .*\+(\d+)/) { $line = $1; next } > > defined($line) or next; > > Makes sense to me. Peff? Yes, looks obviously correct. Thanks. Acked-by: Jeff King <peff@peff.net> -Peff ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] git-jump: ignore (custom) prefix in diff mode 2012-09-17 17:39 ` Jeff King @ 2012-09-17 19:48 ` Junio C Hamano 0 siblings, 0 replies; 10+ messages in thread From: Junio C Hamano @ 2012-09-17 19:48 UTC (permalink / raw) To: Jeff King; +Cc: Mischa POSLAWSKY, git Jeff King <peff@peff.net> writes: > On Sun, Sep 16, 2012 at 10:24:04PM -0700, Junio C Hamano wrote: > >> Mischa POSLAWSKY <git@shiar.nl> writes: >> >> > Matching the default file prefix b/ does not yield any results if config >> > option diff.noprefix or diff.mnemonicprefix is enabled. >> > >> > Signed-off-by: Mischa POSLAWSKY <git@shiar.nl> >> > --- >> > Very useful script otherwise; thanks. >> > >> > contrib/git-jump/git-jump | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git contrib/git-jump/git-jump contrib/git-jump/git-jump >> > index a33674e..dc90cd6 100755 >> > --- contrib/git-jump/git-jump >> > +++ contrib/git-jump/git-jump >> > @@ -21,9 +21,9 @@ open_editor() { >> > ... >> >> Makes sense to me. Peff? > > Yes, looks obviously correct. Thanks. > > Acked-by: Jeff King <peff@peff.net> Thanks. It may not be obvious to many people, so here is tip of the day. I knew Mischa knew that the patch was prepared with wrong src/dst prefix (notice the lack of a/ and b/), but I did not say anything special when I drove "git am". I just did the usual "git am -s3c" and the patch was applied just fine ;-) What is happening is that: - I didn't give '-p0" to "git am", so it thought that patch was based on a tree that has "git-jump" directory at the top-level of the working tree, and the file that is being patched lived at "git-jump/git-jump" in Mischa's working tree; - However, the official git.git tree has the corresponding file at "contrib/git-jump/git-jump", and there is no such file at "git-jump/git-jump". - The three-way merge logic kicks in because of the "-3" option, using a (fake) tree that is shaped like Mischa's tree with the version before the patch as a common ancestor, to merge the version "git am" thought Mischa has (i.e. no contrib/ directory) and the version in my tree. From the point of view of the three-way merge logic, I renamed the path to be in contrib/ directory while Mischa kept the path intact and fixed the contents of the script. This merges cleanly to produce the expected result. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-09-18 9:00 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-09-17 1:21 [PATCH] git-jump: ignore (custom) prefix in diff mode Mischa POSLAWSKY 2012-09-17 3:01 ` Mischa POSLAWSKY 2012-09-17 5:22 ` Junio C Hamano 2012-09-18 2:52 ` Mischa POSLAWSKY 2012-09-18 3:57 ` Junio C Hamano 2012-09-18 9:00 ` Bert Wesarg 2012-09-17 6:03 ` perryh 2012-09-17 5:24 ` Junio C Hamano 2012-09-17 17:39 ` Jeff King 2012-09-17 19:48 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).