Re: [PATCH] git-jump: ignore (custom) prefix in diff mode
On Tue, Sep 18, 2012 at 4:52 AM, Mischa POSLAWSKY wrote: > Junio C Hamano skribis 2012-9-16 22:22 (-0700): > >> Mischa POSLAWSKY 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 >> > --- >> >> 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 '. 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-jump: ignore (custom) prefix in diff mode
Mischa POSLAWSKY 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-jump: ignore (custom) prefix in diff mode
Junio C Hamano skribis 2012-9-16 22:22 (-0700): > Mischa POSLAWSKY 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 > > --- > > 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-jump: ignore (custom) prefix in diff mode
Jeff King writes:
> On Sun, Sep 16, 2012 at 10:24:04PM -0700, Junio C Hamano wrote:
>
>> Mischa POSLAWSKY 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
>> > ---
>> > 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
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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-jump: ignore (custom) prefix in diff mode
On Sun, Sep 16, 2012 at 10:24:04PM -0700, Junio C Hamano wrote:
> Mischa POSLAWSKY 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
> > ---
> > 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-jump: ignore (custom) prefix in diff mode
Mischa POSLAWSKY 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-jump: ignore (custom) prefix in diff mode
Mischa POSLAWSKY 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
> ---
> 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-jump: ignore (custom) prefix in diff mode
Mischa POSLAWSKY 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 > --- 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"; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-jump: ignore (custom) prefix in diff mode
> 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 --- 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-jump: ignore (custom) prefix in diff mode
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
---
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
