{"thread":{"id":"31560","subject":"[PATCH] git-jump: ignore (custom) prefix in diff mode","startedAt":"2012-09-17T01:21:55Z","lastAt":"2012-09-18T09:00:34Z","messageCount":10,"participants":["Mischa POSLAWSKY","Junio C Hamano","perryh@pluto.rain.com","Jeff King","Bert Wesarg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"199178","messageId":"1347844915-4130-1-git-send-email-git@shiar.nl","threadId":"31560","inReplyTo":null,"subject":"[PATCH] git-jump: ignore (custom) prefix in diff mode","fromName":"Mischa POSLAWSKY","fromEmail":"git@shiar.nl","sentAt":"2012-09-17T01:21:55Z","receivedAt":"2012-09-17T01:21:55Z","isPatch":true,"sender":{"key":"git@shiar.nl","avatar":null},"body":"Matching the default file prefix b/ does not yield any results if config\noption diff.noprefix or diff.mnemonicprefix is enabled.\n\nSigned-off-by: Mischa POSLAWSKY \n---\nVery useful script otherwise; thanks.\n\n contrib/git-jump/git-jump | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git contrib/git-jump/git-jump contrib/git-jump/git-jump\nindex a33674e..dc90cd6 100755\n--- contrib/git-jump/git-jump\n+++ contrib/git-jump/git-jump\n@@ -21,9 +21,9 @@ open_editor() {\n }\n \n mode_diff() {\n-\tgit diff --relative \"$@\" |\n+\tgit diff --no-prefix --relative \"$@\" |\n \tperl -ne '\n-\tif (m{^\\+\\+\\+ b/(.*)}) { $file = $1; next }\n+\tif (m{^\\+\\+\\+ (.*)}) { $file = $1; next }\n \tdefined($file) or next;\n \tif (m/^@@ .*\\+(\\d+)/) { $line = $1; next }\n \tdefined($line) or next;\n-- \n1.7.12.165.g8cb9d9c\n"},{"id":"199179","messageId":"20120917030143.GA19986@pearl.mediadesign.nl","threadId":"31560","inReplyTo":"1347844915-4130-1-git-send-email-git@shiar.nl","subject":"Re: [PATCH] git-jump: ignore (custom) prefix in diff mode","fromName":"Mischa POSLAWSKY","fromEmail":"git@shiar.nl","sentAt":"2012-09-17T03:01:43Z","receivedAt":"2012-09-17T03:01:43Z","isPatch":true,"sender":{"key":"git@shiar.nl","avatar":null},"body":"> diff --git contrib/git-jump/git-jump contrib/git-jump/git-jump\n> index a33674e..dc90cd6 100755\n> --- contrib/git-jump/git-jump\n> +++ contrib/git-jump/git-jump\n\nApparently diff.noprefix also applies to git format-patch. Even though\ngit am does explicitly support -p0, I would argue against diff options\ncreating non-standard patches.\n\n-- >8 --\nSubject: [PATCH/RFC] format-patch: force default file prefixes in diff\n\nOverride user configuration (eg. diff.noprefix) in patches intended for\nexternal consumption to match the default prefixes expected by git-am.\n\nSigned-off-by: Mischa POSLAWSKY \n---\n builtin/log.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex dff7921..91ded25 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1131,6 +1131,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \trev.diff = 1;\n \trev.max_parents = 1;\n \tDIFF_OPT_SET(&rev.diffopt, RECURSIVE);\n+\trev.diffopt.a_prefix = \"a/\";\n+\trev.diffopt.b_prefix = \"b/\";\n \trev.subject_prefix = fmt_patch_subject_prefix;\n \tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n \ts_r_opt.def = \"HEAD\";\n-- \n1.7.12.166.ga54f379\n"},{"id":"199186","messageId":"7vy5k9w7eh.fsf@alter.siamese.dyndns.org","threadId":"31560","inReplyTo":"20120917030143.GA19986@pearl.mediadesign.nl","subject":"Re: [PATCH] git-jump: ignore (custom) prefix in diff mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-17T05:22:14Z","receivedAt":"2012-09-17T05:22:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mischa POSLAWSKY writes:\n\n>> diff --git contrib/git-jump/git-jump contrib/git-jump/git-jump\n>> index a33674e..dc90cd6 100755\n>> --- contrib/git-jump/git-jump\n>> +++ contrib/git-jump/git-jump\n>\n> Apparently diff.noprefix also applies to git format-patch. Even though\n> git am does explicitly support -p0, I would argue against diff options\n> creating non-standard patches.\n>\n> -- >8 --\n> Subject: [PATCH/RFC] format-patch: force default file prefixes in diff\n>\n> Override user configuration (eg. diff.noprefix) in patches intended for\n> external consumption to match the default prefixes expected by git-am.\n>\n> Signed-off-by: Mischa POSLAWSKY \n> ---\n\nNot all projects expect to see a/ & b/ prefix and these are\nconfigurable for a reason. Robbing the choice that has been\nsupported for quite a long time from them is an unacceptable\nregression.\n\nWhy did you think this may be a good idea in the first place?\n\nPerhaps you had configured your diff.noprefix in a wrong\nconfiguration file? This is primarily per-project choice, and your\nclone of git.git should not have diff.noprefix set, neither your\n$HOME/.gitconfig unless you always work on projects that want\ndiff.noprefix.\n\n> builtin/log.c | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/builtin/log.c b/builtin/log.c\n> index dff7921..91ded25 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1131,6 +1131,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \trev.diff = 1;\n> \trev.max_parents = 1;\n> \tDIFF_OPT_SET(&rev.diffopt, RECURSIVE);\n> +\trev.diffopt.a_prefix = \"a/\";\n> +\trev.diffopt.b_prefix = \"b/\";\n> \trev.subject_prefix = fmt_patch_subject_prefix;\n> \tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n> \ts_r_opt.def = \"HEAD\";\n"},{"id":"199187","messageId":"7vtxuxw7bf.fsf@alter.siamese.dyndns.org","threadId":"31560","inReplyTo":"1347844915-4130-1-git-send-email-git@shiar.nl","subject":"Re: [PATCH] git-jump: ignore (custom) prefix in diff mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-17T05:24:04Z","receivedAt":"2012-09-17T05:24:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mischa POSLAWSKY writes:\n\n> Matching the default file prefix b/ does not yield any results if config\n> option diff.noprefix or diff.mnemonicprefix is enabled.\n>\n> Signed-off-by: Mischa POSLAWSKY \n> ---\n> Very useful script otherwise; thanks.\n>\n> contrib/git-jump/git-jump | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git contrib/git-jump/git-jump contrib/git-jump/git-jump\n> index a33674e..dc90cd6 100755\n> --- contrib/git-jump/git-jump\n> +++ contrib/git-jump/git-jump\n> @@ -21,9 +21,9 @@ open_editor() {\n> }\n> \n> mode_diff() {\n> -\tgit diff --relative \"$@\" |\n> +\tgit diff --no-prefix --relative \"$@\" |\n> \tperl -ne '\n> -\tif (m{^\\+\\+\\+ b/(.*)}) { $file = $1; next }\n> +\tif (m{^\\+\\+\\+ (.*)}) { $file = $1; next }\n> \tdefined($file) or next;\n> \tif (m/^@@ .*\\+(\\d+)/) { $line = $1; next }\n> \tdefined($line) or next;\n\nMakes sense to me. Peff?\n"},{"id":"199197","messageId":"50571f9b.2XSlVe/YBL63thko%perryh@pluto.rain.com","threadId":"31560","inReplyTo":"20120917030143.GA19986@pearl.mediadesign.nl","subject":"Re: [PATCH] git-jump: ignore (custom) prefix in diff mode","fromName":"","fromEmail":"perryh@pluto.rain.com","sentAt":"2012-09-17T06:03:23Z","receivedAt":"2012-09-17T06:03:23Z","isPatch":true,"sender":{"key":"perryh@pluto.rain.com","avatar":null},"body":"Mischa POSLAWSKY wrote:\n\n> ... I would argue against diff options creating non-standard patches.\n\nSeems to me it might depend on what one means by \"non-standard\".\n\nI can envision cases in which increasing the number of context lines\nwould result in the patch being more robust WRT applying correctly\nto a recipient's version that might be a bit different than the one\nagainst which it was created.\n\nOTOH one most likely does not want to create a patch with -b unless\nthe apply tool also supports such and there is a way to communicate\nto the apply tool that -b was used in creating the patch.\n"},{"id":"199261","messageId":"20120917173945.GA22000@sigill.intra.peff.net","threadId":"31560","inReplyTo":"7vtxuxw7bf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-jump: ignore (custom) prefix in diff mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-09-17T17:39:45Z","receivedAt":"2012-09-17T17:39:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Sep 16, 2012 at 10:24:04PM -0700, Junio C Hamano wrote:\n\n> Mischa POSLAWSKY writes:\n> \n> > Matching the default file prefix b/ does not yield any results if config\n> > option diff.noprefix or diff.mnemonicprefix is enabled.\n> >\n> > Signed-off-by: Mischa POSLAWSKY \n> > ---\n> > Very useful script otherwise; thanks.\n> >\n> > contrib/git-jump/git-jump | 4 ++--\n> > 1 file changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git contrib/git-jump/git-jump contrib/git-jump/git-jump\n> > index a33674e..dc90cd6 100755\n> > --- contrib/git-jump/git-jump\n> > +++ contrib/git-jump/git-jump\n> > @@ -21,9 +21,9 @@ open_editor() {\n> > }\n> > \n> > mode_diff() {\n> > -\tgit diff --relative \"$@\" |\n> > +\tgit diff --no-prefix --relative \"$@\" |\n> > \tperl -ne '\n> > -\tif (m{^\\+\\+\\+ b/(.*)}) { $file = $1; next }\n> > +\tif (m{^\\+\\+\\+ (.*)}) { $file = $1; next }\n> > \tdefined($file) or next;\n> > \tif (m/^@@ .*\\+(\\d+)/) { $line = $1; next }\n> > \tdefined($line) or next;\n> \n> Makes sense to me. Peff?\n\nYes, looks obviously correct. Thanks.\n\nAcked-by: Jeff King \n\n-Peff\n"},{"id":"199272","messageId":"7vlig8v3b5.fsf@alter.siamese.dyndns.org","threadId":"31560","inReplyTo":"20120917173945.GA22000@sigill.intra.peff.net","subject":"Re: [PATCH] git-jump: ignore (custom) prefix in diff mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-17T19:48:14Z","receivedAt":"2012-09-17T19:48:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sun, Sep 16, 2012 at 10:24:04PM -0700, Junio C Hamano wrote:\n>\n>> Mischa POSLAWSKY writes:\n>> \n>> > Matching the default file prefix b/ does not yield any results if config\n>> > option diff.noprefix or diff.mnemonicprefix is enabled.\n>> >\n>> > Signed-off-by: Mischa POSLAWSKY \n>> > ---\n>> > Very useful script otherwise; thanks.\n>> >\n>> > contrib/git-jump/git-jump | 4 ++--\n>> > 1 file changed, 2 insertions(+), 2 deletions(-)\n>> >\n>> > diff --git contrib/git-jump/git-jump contrib/git-jump/git-jump\n>> > index a33674e..dc90cd6 100755\n>> > --- contrib/git-jump/git-jump\n>> > +++ contrib/git-jump/git-jump\n>> > @@ -21,9 +21,9 @@ open_editor() {\n>> > ...\n>> \n>> Makes sense to me. Peff?\n>\n> Yes, looks obviously correct. Thanks.\n>\n> Acked-by: Jeff King \n\nThanks.\n\nIt may not be obvious to many people, so here is tip of the day.\n\nI knew Mischa knew that the patch was prepared with wrong src/dst\nprefix (notice the lack of a/ and b/), but I did not say anything\nspecial when I drove \"git am\". I just did the usual \"git am -s3c\"\nand the patch was applied just fine ;-)\n\nWhat is happening is that:\n\n - I didn't give '-p0\" to \"git am\", so it thought that patch was\n based on a tree that has \"git-jump\" directory at the top-level\n of the working tree, and the file that is being patched lived at\n \"git-jump/git-jump\" in Mischa's working tree;\n\n - However, the official git.git tree has the corresponding file at\n \"contrib/git-jump/git-jump\", and there is no such file at\n \"git-jump/git-jump\".\n\n - The three-way merge logic kicks in because of the \"-3\" option,\n using a (fake) tree that is shaped like Mischa's tree with the\n version before the patch as a common ancestor, to merge the\n version \"git am\" thought Mischa has (i.e. no contrib/ directory)\n and the version in my tree. From the point of view of the\n three-way merge logic, I renamed the path to be in contrib/\n directory while Mischa kept the path intact and fixed the\n contents of the script. This merges cleanly to produce the\n expected result.\n"},{"id":"199323","messageId":"20120918025229.GB19986@pearl.mediadesign.nl","threadId":"31560","inReplyTo":"7vy5k9w7eh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-jump: ignore (custom) prefix in diff mode","fromName":"Mischa POSLAWSKY","fromEmail":"git@shiar.nl","sentAt":"2012-09-18T02:52:29Z","receivedAt":"2012-09-18T02:52:29Z","isPatch":true,"sender":{"key":"git@shiar.nl","avatar":null},"body":"Junio C Hamano skribis 2012-9-16 22:22 (-0700):\n\n> Mischa POSLAWSKY writes:\n> \n> > Subject: [PATCH/RFC] format-patch: force default file prefixes in diff\n> >\n> > Override user configuration (eg. diff.noprefix) in patches intended for\n> > external consumption to match the default prefixes expected by git-am.\n> >\n> > Signed-off-by: Mischa POSLAWSKY \n> > ---\n> \n> Not all projects expect to see a/ & b/ prefix and these are\n> configurable for a reason. Robbing the choice that has been\n> supported for quite a long time from them is an unacceptable\n> regression.\n\nMy bad, I was assuming format-patch would mostly interact with git am.\n\n> Why did you think this may be a good idea in the first place?\n> \n> Perhaps you had configured your diff.noprefix in a wrong\n> configuration file? This is primarily per-project choice, and your\n> clone of git.git should not have diff.noprefix set, neither your\n> $HOME/.gitconfig unless you always work on projects that want\n> diff.noprefix.\n\nThen I'm not using it as intended. For me it's just a personal\npreference of how I'd like to review commits (diff/show) so I can easily\ncopy-paste file names (less essential since my discovery of git jump,\nbut still). It's not something I'd like to be communicated with any\nupstream project (format-patch).\n\nSo it seems I'm asking for a new feature: to be able to configure local\nand inter-project diff options differently. In this case I'd be helped\nby either format.noprefix=0 or a to be bikeshedded localdiff.noprefix=1.\nI don't know about other options though. Does anybody actually want\nmnemonicprefix to be sent out as well?\n\nAnother solution could be a single option defining behaviour exceptions:\nformat.diff = normal | textconv | noconfig\nExpanding on the existing --(no-)textconv difference in format-patch.\n\n-- \nMischa\n"},{"id":"199326","messageId":"7vsjagoudl.fsf@alter.siamese.dyndns.org","threadId":"31560","inReplyTo":"20120918025229.GB19986@pearl.mediadesign.nl","subject":"Re: [PATCH] git-jump: ignore (custom) prefix in diff mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-18T03:57:42Z","receivedAt":"2012-09-18T03:57:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mischa POSLAWSKY writes:\n\n>> Perhaps you had configured your diff.noprefix in a wrong\n>> configuration file? This is primarily per-project choice, and your\n>> clone of git.git should not have diff.noprefix set, neither your\n>> $HOME/.gitconfig unless you always work on projects that want\n>> diff.noprefix.\n>\n> Then I'm not using it as intended.\n\nI would imagine that you could do\n\n\t(in ~/.gitconfig)\n\t[diff] noprefix\n\n (in ~/git.git/.git/config\n [diff] noprefix = false\n\nor something.\n"},{"id":"199355","messageId":"CAKPyHN2_RShv8Emr+OfZF_m1oP7Ann5tNuio4+yuSjfHXc043Q@mail.gmail.com","threadId":"31560","inReplyTo":"20120918025229.GB19986@pearl.mediadesign.nl","subject":"Re: [PATCH] git-jump: ignore (custom) prefix in diff mode","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2012-09-18T09:00:34Z","receivedAt":"2012-09-18T09:00:34Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Tue, Sep 18, 2012 at 4:52 AM, Mischa POSLAWSKY wrote:\n> Junio C Hamano skribis 2012-9-16 22:22 (-0700):\n>\n>> Mischa POSLAWSKY writes:\n>>\n>> > Subject: [PATCH/RFC] format-patch: force default file prefixes in diff\n>> >\n>> > Override user configuration (eg. diff.noprefix) in patches intended for\n>> > external consumption to match the default prefixes expected by git-am.\n>> >\n>> > Signed-off-by: Mischa POSLAWSKY \n>> > ---\n>>\n>> Not all projects expect to see a/ & b/ prefix and these are\n>> configurable for a reason. Robbing the choice that has been\n>> supported for quite a long time from them is an unacceptable\n>> regression.\n>\n> My bad, I was assuming format-patch would mostly interact with git am.\n>\n>> Why did you think this may be a good idea in the first place?\n>>\n>> Perhaps you had configured your diff.noprefix in a wrong\n>> configuration file? This is primarily per-project choice, and your\n>> clone of git.git should not have diff.noprefix set, neither your\n>> $HOME/.gitconfig unless you always work on projects that want\n>> diff.noprefix.\n>\n> Then I'm not using it as intended. For me it's just a personal\n> preference of how I'd like to review commits (diff/show) so I can easily\n> copy-paste file names (less essential since my discovery of git jump,\n> but still). It's not something I'd like to be communicated with any\n> upstream project (format-patch).\n>\n> So it seems I'm asking for a new feature: to be able to configure local\n> and inter-project diff options differently. In this case I'd be helped\n> by either format.noprefix=0 or a to be bikeshedded localdiff.noprefix=1.\n> I don't know about other options though. Does anybody actually want\n> mnemonicprefix to be sent out as well?\n\nI once had the same idea and posted a patch for it:\n\nhttp://article.gmane.org/gmane.comp.version-control.git/146215\n\nThe implementation differs though, the pure path without any\nmnemonicprefix is in its own line 'path '. But my current patch\nalso differs to this old one, in the current one, the path is at the\nend 'index' line. But I do not need this feature anymore.\n\nBert\n\n>\n> Another solution could be a single option defining behaviour exceptions:\n> format.diff = normal | textconv | noconfig\n> Expanding on the existing --(no-)textconv difference in format-patch.\n>\n> --\n> Mischa\n"}]}