| Message ID | 20240315055448.GA2253326@quokka (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5] diff: add diff.srcPrefix and diff.dstPrefix configuration variables | expand |
On 2024-03-15 06:54, Peter Hutterer wrote: > Allow the default prefixes "a/" and "b/" to be tweaked by the > diff.srcPrefix and diff.dstPrefix configuration variables. > > Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> Looking good to me! Reviewed-by: Dragan Simic <dsimic@manjaro.org> > --- > Changes to v4 (as pointed out by Dragan): > - copy/paste-o fixed in the dstprefix test > - reworded the description for the tests as suggested > > Dragan: good catch on the test, thanks for that. Thanks for applying the suggestions in v5. > I think for the > rewording of --default-prefix it might be faster if you reword it? > Otherwise we might keep spinning this one for a quite a bit longer :) Agreed, I had exactly the same in mind. :) > Documentation/config/diff.txt | 6 ++++++ > Documentation/diff-options.txt | 5 +++-- > diff.c | 14 ++++++++++++-- > t/t4013-diff-various.sh | 35 ++++++++++++++++++++++++++++++++++ > 4 files changed, 56 insertions(+), 4 deletions(-) > > diff --git a/Documentation/config/diff.txt > b/Documentation/config/diff.txt > index 6c7e09a1ef5e..fea89291c675 100644 > --- a/Documentation/config/diff.txt > +++ b/Documentation/config/diff.txt > @@ -111,6 +111,12 @@ diff.mnemonicPrefix:: > diff.noprefix:: > If set, 'git diff' does not show any source or destination prefix. > > +diff.srcPrefix:: > + If set, 'git diff' uses this source prefix. Defaults to "a/". > + > +diff.dstPrefix:: > + If set, 'git diff' uses this destination prefix. Defaults to "b/". > + > diff.relative:: > If set to 'true', 'git diff' does not show changes outside of the > directory > and show pathnames relative to the current directory. > diff --git a/Documentation/diff-options.txt > b/Documentation/diff-options.txt > index aaaff0d46f0c..0e9456957e37 100644 > --- a/Documentation/diff-options.txt > +++ b/Documentation/diff-options.txt > @@ -865,8 +865,9 @@ endif::git-format-patch[] > > --default-prefix:: > Use the default source and destination prefixes ("a/" and "b/"). > - This is usually the default already, but may be used to override > - config such as `diff.noprefix`. > + This overrides configuration variables such as `diff.noprefix`, > + `diff.srcPrefix`, `diff.dstPrefix`, and `diff.mnemonicPrefix` > + (see `git-config`(1)). > > --line-prefix=<prefix>:: > Prepend an additional prefix to every line of output. > diff --git a/diff.c b/diff.c > index e50def45383e..108c1875775d 100644 > --- a/diff.c > +++ b/diff.c > @@ -62,6 +62,8 @@ static const char *diff_order_file_cfg; > int diff_auto_refresh_index = 1; > static int diff_mnemonic_prefix; > static int diff_no_prefix; > +static const char *diff_src_prefix = "a/"; > +static const char *diff_dst_prefix = "b/"; > static int diff_relative; > static int diff_stat_name_width; > static int diff_stat_graph_width; > @@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const char > *value, > diff_no_prefix = git_config_bool(var, value); > return 0; > } > + if (!strcmp(var, "diff.srcprefix")) { > + return git_config_string(&diff_src_prefix, var, value); > + } > + if (!strcmp(var, "diff.dstprefix")) { > + return git_config_string(&diff_dst_prefix, var, value); > + } > if (!strcmp(var, "diff.relative")) { > diff_relative = git_config_bool(var, value); > return 0; > @@ -3425,8 +3433,8 @@ void diff_set_noprefix(struct diff_options > *options) > > void diff_set_default_prefix(struct diff_options *options) > { > - options->a_prefix = "a/"; > - options->b_prefix = "b/"; > + options->a_prefix = diff_src_prefix; > + options->b_prefix = diff_dst_prefix; > } > > struct userdiff_driver *get_textconv(struct repository *r, > @@ -5362,6 +5370,8 @@ static int diff_opt_default_prefix(const struct > option *opt, > > BUG_ON_OPT_NEG(unset); > BUG_ON_OPT_ARG(optarg); > + diff_src_prefix = "a/"; > + diff_dst_prefix = "b/"; > diff_set_default_prefix(options); > return 0; > } > diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh > index 1e3b2dbea484..6bf69888f258 100755 > --- a/t/t4013-diff-various.sh > +++ b/t/t4013-diff-various.sh > @@ -663,6 +663,41 @@ test_expect_success 'diff --default-prefix > overrides diff.mnemonicprefix' ' > check_prefix actual a/file0 b/file0 > ' > > +test_expect_success 'diff respects diff.srcprefix' ' > + git -c diff.srcprefix=x/ diff >actual && > + check_prefix actual x/file0 b/file0 > +' > + > +test_expect_success 'diff respects diff.dstprefix' ' > + git -c diff.dstprefix=y/ diff >actual && > + check_prefix actual a/file0 y/file0 > +' > + > +test_expect_success 'diff --src-prefix overrides diff.srcprefix' ' > + git -c diff.srcprefix=y/ diff --src-prefix=z/ >actual && > + check_prefix actual z/file0 b/file0 > +' > + > +test_expect_success 'diff --dst-prefix overrides diff.dstprefix' ' > + git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual && > + check_prefix actual a/file0 z/file0 > +' > + > +test_expect_success 'diff.*prefix ignored with diff.noprefix' ' > + git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff > >actual && > + check_prefix actual file0 file0 > +' > + > +test_expect_success 'diff.*prefix ignored with diff.mnemonicprefix' ' > + git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix > diff >actual && > + check_prefix actual i/file0 w/file0 > +' > + > +test_expect_success 'diff.*prefix ignored with --default-prefix' ' > + git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix > >actual && > + check_prefix actual a/file0 b/file0 > +' > + > test_expect_success 'diff --no-renames cannot be abbreviated' ' > test_expect_code 129 git diff --no-rename >actual 2>error && > test_must_be_empty actual &&
Peter Hutterer <peter.hutterer@who-t.net> writes: > Allow the default prefixes "a/" and "b/" to be tweaked by the > diff.srcPrefix and diff.dstPrefix configuration variables. > > Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> > --- > Changes to v4 (as pointed out by Dragan): > - copy/paste-o fixed in the dstprefix test This one I understand is an improvement > - reworded the description for the tests as suggested Moving from "diff src/dstprefix" to "diff.*prefix" feels more like a regresison to me, when it is about interaction between {src,dst}prefix and other kind of prefix variables like {no,mnemonic}prefix. I would have understood if the updated title were more like: test_expect_success 'diff.{src,dst}Prefix are ignored with diff.noPrefix' I am tempted to queue v4 with the z/ -> y/ fix from this round, without any other changes from v4 to v5. Thanks, both.
On 2024-03-15 18:00, Junio C Hamano wrote: > Peter Hutterer <peter.hutterer@who-t.net> writes: > >> Allow the default prefixes "a/" and "b/" to be tweaked by the >> diff.srcPrefix and diff.dstPrefix configuration variables. >> >> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> >> --- >> Changes to v4 (as pointed out by Dragan): >> - copy/paste-o fixed in the dstprefix test > > This one I understand is an improvement > >> - reworded the description for the tests as suggested > > Moving from "diff src/dstprefix" to "diff.*prefix" feels more like a > regresison to me, when it is about interaction between {src,dst}prefix > and other kind of prefix variables like {no,mnemonic}prefix. I > would have understood if the updated title were more like: > > test_expect_success 'diff.{src,dst}Prefix are ignored with > diff.noPrefix' That would be even better, thank you for pointing it out. If possible, I'd suggest that your wording becomes merged. > I am tempted to queue v4 with the z/ -> y/ fix from this round, > without any other changes from v4 to v5. > > Thanks, both.
Junio C Hamano <gitster@pobox.com> writes: > I am tempted to queue v4 with the z/ -> y/ fix from this round, > without any other changes from v4 to v5. So, that is what I did before I pushed out today's integration result. I however have an "after the dust settles" clean-up patch on top (not committed yet), which I am sending out for review. ------- >8 -------------- >8 -------------- >8 -------- Subject: diff.*Prefix: use camelCase in the doc and test titles We added documentation for diff.srcPrefix and diff.dstPrefix with their names properly camelCased, but the diff.noPrefix is listed there in all lowercase. Also these configuration variables, both existing ones and the {src,dst}Prefix we recently added, were spelled in all lowercase in the tests in t4013. Now we are done with the main change, clean these up. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- * If we were early in the review cycle, we would strongly prefer to do a "preliminary clean-up" followed by the main change, as the clean-up step would be much less controversial and can be queued earlier before the main change solidifies. But at v5 the main change is more or less perfect, so it is not worth rerolling to split the clean-up changes into preliminary ones and change to the main patch. So this is written as an "on top, after the dust settles" clean-up patch. Documentation/config/diff.txt | 2 +- t/t4013-diff-various.sh | 48 +++++++++++++++++++++---------------------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git c/Documentation/config/diff.txt w/Documentation/config/diff.txt index fea89291c6..5ce7b91f1d 100644 --- c/Documentation/config/diff.txt +++ w/Documentation/config/diff.txt @@ -108,7 +108,7 @@ diff.mnemonicPrefix:: `git diff --no-index a b`;; compares two non-git things (1) and (2). -diff.noprefix:: +diff.noPrefix:: If set, 'git diff' does not show any source or destination prefix. diff.srcPrefix:: diff --git c/t/t4013-diff-various.sh w/t/t4013-diff-various.sh index cfb5ad3d8d..3855d68dbc 100755 --- c/t/t4013-diff-various.sh +++ w/t/t4013-diff-various.sh @@ -633,8 +633,8 @@ check_prefix () { test_cmp expect actual.paths } -test_expect_success 'diff-files does not respect diff.noprefix' ' - git -c diff.noprefix diff-files -p >actual && +test_expect_success 'diff-files does not respect diff.noPrefix' ' + git -c diff.noPrefix diff-files -p >actual && check_prefix actual a/file0 b/file0 ' @@ -643,58 +643,58 @@ test_expect_success 'diff-files respects --no-prefix' ' check_prefix actual file0 file0 ' -test_expect_success 'diff respects diff.noprefix' ' - git -c diff.noprefix diff >actual && +test_expect_success 'diff respects diff.noPrefix' ' + git -c diff.noPrefix diff >actual && check_prefix actual file0 file0 ' -test_expect_success 'diff --default-prefix overrides diff.noprefix' ' - git -c diff.noprefix diff --default-prefix >actual && +test_expect_success 'diff --default-prefix overrides diff.noPrefix' ' + git -c diff.noPrefix diff --default-prefix >actual && check_prefix actual a/file0 b/file0 ' -test_expect_success 'diff respects diff.mnemonicprefix' ' - git -c diff.mnemonicprefix diff >actual && +test_expect_success 'diff respects diff.mnemonicPrefix' ' + git -c diff.mnemonicPrefix diff >actual && check_prefix actual i/file0 w/file0 ' -test_expect_success 'diff --default-prefix overrides diff.mnemonicprefix' ' - git -c diff.mnemonicprefix diff --default-prefix >actual && +test_expect_success 'diff --default-prefix overrides diff.mnemonicPrefix' ' + git -c diff.mnemonicPrefix diff --default-prefix >actual && check_prefix actual a/file0 b/file0 ' -test_expect_success 'diff respects diff.srcprefix' ' - git -c diff.srcprefix=x/ diff >actual && +test_expect_success 'diff respects diff.srcPrefix' ' + git -c diff.srcPrefix=x/ diff >actual && check_prefix actual x/file0 b/file0 ' -test_expect_success 'diff respects diff.dstprefix' ' - git -c diff.dstprefix=y/ diff >actual && +test_expect_success 'diff respects diff.dstPrefix' ' + git -c diff.dstPrefix=y/ diff >actual && check_prefix actual a/file0 y/file0 ' -test_expect_success 'diff --src-prefix overrides diff.srcprefix' ' - git -c diff.srcprefix=y/ diff --src-prefix=z/ >actual && +test_expect_success 'diff --src-prefix overrides diff.srcPrefix' ' + git -c diff.srcPrefix=y/ diff --src-prefix=z/ >actual && check_prefix actual z/file0 b/file0 ' -test_expect_success 'diff --dst-prefix overrides diff.dstprefix' ' - git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual && +test_expect_success 'diff --dst-prefix overrides diff.dstPrefix' ' + git -c diff.dstPrefix=y/ diff --dst-prefix=z/ >actual && check_prefix actual a/file0 z/file0 ' -test_expect_success 'diff.{src,dst}prefix ignored with diff.noprefix' ' - git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff >actual && +test_expect_success 'diff.{src,dst}Prefix ignored with diff.noPrefix' ' + git -c diff.dstPrefix=y/ -c diff.srcPrefix=x/ -c diff.noPrefix diff >actual && check_prefix actual file0 file0 ' -test_expect_success 'diff.{src,dst}prefix ignored with diff.mnemonicprefix' ' - git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix diff >actual && +test_expect_success 'diff.{src,dst}Prefix ignored with diff.mnemonicPrefix' ' + git -c diff.dstPrefix=x/ -c diff.srcPrefix=y/ -c diff.mnemonicPrefix diff >actual && check_prefix actual i/file0 w/file0 ' -test_expect_success 'diff.{src,dst}prefix ignored with --default-prefix' ' - git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix >actual && +test_expect_success 'diff.{src,dst}Prefix ignored with --default-prefix' ' + git -c diff.dstPrefix=x/ -c diff.srcPrefix=y/ diff --default-prefix >actual && check_prefix actual a/file0 b/file0 '
On 2024-03-16 06:57, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > >> I am tempted to queue v4 with the z/ -> y/ fix from this round, >> without any other changes from v4 to v5. > > So, that is what I did before I pushed out today's integration > result. I however have an "after the dust settles" clean-up patch > on top (not committed yet), which I am sending out for review. Looking great to me! Thanks a lot for the final touches. Reviewed-by: Dragan Simic <dsimic@manjaro.org> > ------- >8 -------------- >8 -------------- >8 -------- > Subject: diff.*Prefix: use camelCase in the doc and test titles > > We added documentation for diff.srcPrefix and diff.dstPrefix with > their names properly camelCased, but the diff.noPrefix is listed > there in all lowercase. Also these configuration variables, both > existing ones and the {src,dst}Prefix we recently added, were > spelled in all lowercase in the tests in t4013. > > Now we are done with the main change, clean these up. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > > * If we were early in the review cycle, we would strongly prefer to > do a "preliminary clean-up" followed by the main change, as the > clean-up step would be much less controversial and can be queued > earlier before the main change solidifies. But at v5 the main > change is more or less perfect, so it is not worth rerolling to > split the clean-up changes into preliminary ones and change to > the main patch. So this is written as an "on top, after the dust > settles" clean-up patch. > > Documentation/config/diff.txt | 2 +- > t/t4013-diff-various.sh | 48 > +++++++++++++++++++++---------------------- > 2 files changed, 25 insertions(+), 25 deletions(-) > > diff --git c/Documentation/config/diff.txt > w/Documentation/config/diff.txt > index fea89291c6..5ce7b91f1d 100644 > --- c/Documentation/config/diff.txt > +++ w/Documentation/config/diff.txt > @@ -108,7 +108,7 @@ diff.mnemonicPrefix:: > `git diff --no-index a b`;; > compares two non-git things (1) and (2). > > -diff.noprefix:: > +diff.noPrefix:: > If set, 'git diff' does not show any source or destination prefix. > > diff.srcPrefix:: > diff --git c/t/t4013-diff-various.sh w/t/t4013-diff-various.sh > index cfb5ad3d8d..3855d68dbc 100755 > --- c/t/t4013-diff-various.sh > +++ w/t/t4013-diff-various.sh > @@ -633,8 +633,8 @@ check_prefix () { > test_cmp expect actual.paths > } > > -test_expect_success 'diff-files does not respect diff.noprefix' ' > - git -c diff.noprefix diff-files -p >actual && > +test_expect_success 'diff-files does not respect diff.noPrefix' ' > + git -c diff.noPrefix diff-files -p >actual && > check_prefix actual a/file0 b/file0 > ' > > @@ -643,58 +643,58 @@ test_expect_success 'diff-files respects > --no-prefix' ' > check_prefix actual file0 file0 > ' > > -test_expect_success 'diff respects diff.noprefix' ' > - git -c diff.noprefix diff >actual && > +test_expect_success 'diff respects diff.noPrefix' ' > + git -c diff.noPrefix diff >actual && > check_prefix actual file0 file0 > ' > > -test_expect_success 'diff --default-prefix overrides diff.noprefix' ' > - git -c diff.noprefix diff --default-prefix >actual && > +test_expect_success 'diff --default-prefix overrides diff.noPrefix' ' > + git -c diff.noPrefix diff --default-prefix >actual && > check_prefix actual a/file0 b/file0 > ' > > -test_expect_success 'diff respects diff.mnemonicprefix' ' > - git -c diff.mnemonicprefix diff >actual && > +test_expect_success 'diff respects diff.mnemonicPrefix' ' > + git -c diff.mnemonicPrefix diff >actual && > check_prefix actual i/file0 w/file0 > ' > > -test_expect_success 'diff --default-prefix overrides > diff.mnemonicprefix' ' > - git -c diff.mnemonicprefix diff --default-prefix >actual && > +test_expect_success 'diff --default-prefix overrides > diff.mnemonicPrefix' ' > + git -c diff.mnemonicPrefix diff --default-prefix >actual && > check_prefix actual a/file0 b/file0 > ' > > -test_expect_success 'diff respects diff.srcprefix' ' > - git -c diff.srcprefix=x/ diff >actual && > +test_expect_success 'diff respects diff.srcPrefix' ' > + git -c diff.srcPrefix=x/ diff >actual && > check_prefix actual x/file0 b/file0 > ' > > -test_expect_success 'diff respects diff.dstprefix' ' > - git -c diff.dstprefix=y/ diff >actual && > +test_expect_success 'diff respects diff.dstPrefix' ' > + git -c diff.dstPrefix=y/ diff >actual && > check_prefix actual a/file0 y/file0 > ' > > -test_expect_success 'diff --src-prefix overrides diff.srcprefix' ' > - git -c diff.srcprefix=y/ diff --src-prefix=z/ >actual && > +test_expect_success 'diff --src-prefix overrides diff.srcPrefix' ' > + git -c diff.srcPrefix=y/ diff --src-prefix=z/ >actual && > check_prefix actual z/file0 b/file0 > ' > > -test_expect_success 'diff --dst-prefix overrides diff.dstprefix' ' > - git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual && > +test_expect_success 'diff --dst-prefix overrides diff.dstPrefix' ' > + git -c diff.dstPrefix=y/ diff --dst-prefix=z/ >actual && > check_prefix actual a/file0 z/file0 > ' > > -test_expect_success 'diff.{src,dst}prefix ignored with diff.noprefix' > ' > - git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff > >actual && > +test_expect_success 'diff.{src,dst}Prefix ignored with diff.noPrefix' > ' > + git -c diff.dstPrefix=y/ -c diff.srcPrefix=x/ -c diff.noPrefix diff > >actual && > check_prefix actual file0 file0 > ' > > -test_expect_success 'diff.{src,dst}prefix ignored with > diff.mnemonicprefix' ' > - git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix > diff >actual && > +test_expect_success 'diff.{src,dst}Prefix ignored with > diff.mnemonicPrefix' ' > + git -c diff.dstPrefix=x/ -c diff.srcPrefix=y/ -c diff.mnemonicPrefix > diff >actual && > check_prefix actual i/file0 w/file0 > ' > > -test_expect_success 'diff.{src,dst}prefix ignored with > --default-prefix' ' > - git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix > >actual && > +test_expect_success 'diff.{src,dst}Prefix ignored with > --default-prefix' ' > + git -c diff.dstPrefix=x/ -c diff.srcPrefix=y/ diff --default-prefix > >actual && > check_prefix actual a/file0 b/file0 > '
On Fri, Mar 15, 2024 at 10:57:22PM -0700, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > > > I am tempted to queue v4 with the z/ -> y/ fix from this round, > > without any other changes from v4 to v5. > > So, that is what I did before I pushed out today's integration > result. I however have an "after the dust settles" clean-up patch > on top (not committed yet), which I am sending out for review. > > ------- >8 -------------- >8 -------------- >8 -------- > Subject: diff.*Prefix: use camelCase in the doc and test titles > > We added documentation for diff.srcPrefix and diff.dstPrefix with > their names properly camelCased, but the diff.noPrefix is listed > there in all lowercase. Also these configuration variables, both > existing ones and the {src,dst}Prefix we recently added, were > spelled in all lowercase in the tests in t4013. > > Now we are done with the main change, clean these up. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> And thanks for merging the patch! Cheers, Peter > --- > > * If we were early in the review cycle, we would strongly prefer to > do a "preliminary clean-up" followed by the main change, as the > clean-up step would be much less controversial and can be queued > earlier before the main change solidifies. But at v5 the main > change is more or less perfect, so it is not worth rerolling to > split the clean-up changes into preliminary ones and change to > the main patch. So this is written as an "on top, after the dust > settles" clean-up patch. > > Documentation/config/diff.txt | 2 +- > t/t4013-diff-various.sh | 48 +++++++++++++++++++++---------------------- > 2 files changed, 25 insertions(+), 25 deletions(-) > > diff --git c/Documentation/config/diff.txt w/Documentation/config/diff.txt > index fea89291c6..5ce7b91f1d 100644 > --- c/Documentation/config/diff.txt > +++ w/Documentation/config/diff.txt > @@ -108,7 +108,7 @@ diff.mnemonicPrefix:: > `git diff --no-index a b`;; > compares two non-git things (1) and (2). > > -diff.noprefix:: > +diff.noPrefix:: > If set, 'git diff' does not show any source or destination prefix. > > diff.srcPrefix:: > diff --git c/t/t4013-diff-various.sh w/t/t4013-diff-various.sh > index cfb5ad3d8d..3855d68dbc 100755 > --- c/t/t4013-diff-various.sh > +++ w/t/t4013-diff-various.sh > @@ -633,8 +633,8 @@ check_prefix () { > test_cmp expect actual.paths > } > > -test_expect_success 'diff-files does not respect diff.noprefix' ' > - git -c diff.noprefix diff-files -p >actual && > +test_expect_success 'diff-files does not respect diff.noPrefix' ' > + git -c diff.noPrefix diff-files -p >actual && > check_prefix actual a/file0 b/file0 > ' > > @@ -643,58 +643,58 @@ test_expect_success 'diff-files respects --no-prefix' ' > check_prefix actual file0 file0 > ' > > -test_expect_success 'diff respects diff.noprefix' ' > - git -c diff.noprefix diff >actual && > +test_expect_success 'diff respects diff.noPrefix' ' > + git -c diff.noPrefix diff >actual && > check_prefix actual file0 file0 > ' > > -test_expect_success 'diff --default-prefix overrides diff.noprefix' ' > - git -c diff.noprefix diff --default-prefix >actual && > +test_expect_success 'diff --default-prefix overrides diff.noPrefix' ' > + git -c diff.noPrefix diff --default-prefix >actual && > check_prefix actual a/file0 b/file0 > ' > > -test_expect_success 'diff respects diff.mnemonicprefix' ' > - git -c diff.mnemonicprefix diff >actual && > +test_expect_success 'diff respects diff.mnemonicPrefix' ' > + git -c diff.mnemonicPrefix diff >actual && > check_prefix actual i/file0 w/file0 > ' > > -test_expect_success 'diff --default-prefix overrides diff.mnemonicprefix' ' > - git -c diff.mnemonicprefix diff --default-prefix >actual && > +test_expect_success 'diff --default-prefix overrides diff.mnemonicPrefix' ' > + git -c diff.mnemonicPrefix diff --default-prefix >actual && > check_prefix actual a/file0 b/file0 > ' > > -test_expect_success 'diff respects diff.srcprefix' ' > - git -c diff.srcprefix=x/ diff >actual && > +test_expect_success 'diff respects diff.srcPrefix' ' > + git -c diff.srcPrefix=x/ diff >actual && > check_prefix actual x/file0 b/file0 > ' > > -test_expect_success 'diff respects diff.dstprefix' ' > - git -c diff.dstprefix=y/ diff >actual && > +test_expect_success 'diff respects diff.dstPrefix' ' > + git -c diff.dstPrefix=y/ diff >actual && > check_prefix actual a/file0 y/file0 > ' > > -test_expect_success 'diff --src-prefix overrides diff.srcprefix' ' > - git -c diff.srcprefix=y/ diff --src-prefix=z/ >actual && > +test_expect_success 'diff --src-prefix overrides diff.srcPrefix' ' > + git -c diff.srcPrefix=y/ diff --src-prefix=z/ >actual && > check_prefix actual z/file0 b/file0 > ' > > -test_expect_success 'diff --dst-prefix overrides diff.dstprefix' ' > - git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual && > +test_expect_success 'diff --dst-prefix overrides diff.dstPrefix' ' > + git -c diff.dstPrefix=y/ diff --dst-prefix=z/ >actual && > check_prefix actual a/file0 z/file0 > ' > > -test_expect_success 'diff.{src,dst}prefix ignored with diff.noprefix' ' > - git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff >actual && > +test_expect_success 'diff.{src,dst}Prefix ignored with diff.noPrefix' ' > + git -c diff.dstPrefix=y/ -c diff.srcPrefix=x/ -c diff.noPrefix diff >actual && > check_prefix actual file0 file0 > ' > > -test_expect_success 'diff.{src,dst}prefix ignored with diff.mnemonicprefix' ' > - git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix diff >actual && > +test_expect_success 'diff.{src,dst}Prefix ignored with diff.mnemonicPrefix' ' > + git -c diff.dstPrefix=x/ -c diff.srcPrefix=y/ -c diff.mnemonicPrefix diff >actual && > check_prefix actual i/file0 w/file0 > ' > > -test_expect_success 'diff.{src,dst}prefix ignored with --default-prefix' ' > - git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix >actual && > +test_expect_success 'diff.{src,dst}Prefix ignored with --default-prefix' ' > + git -c diff.dstPrefix=x/ -c diff.srcPrefix=y/ diff --default-prefix >actual && > check_prefix actual a/file0 b/file0 > ' >
Peter Hutterer <peter.hutterer@who-t.net> writes: > On Fri, Mar 15, 2024 at 10:57:22PM -0700, Junio C Hamano wrote: >> Junio C Hamano <gitster@pobox.com> writes: >> >> > I am tempted to queue v4 with the z/ -> y/ fix from this round, >> > without any other changes from v4 to v5. >> >> So, that is what I did before I pushed out today's integration >> result. I however have an "after the dust settles" clean-up patch >> on top (not committed yet), which I am sending out for review. >> >> ------- >8 -------------- >8 -------------- >8 -------- >> Subject: diff.*Prefix: use camelCase in the doc and test titles >> >> We added documentation for diff.srcPrefix and diff.dstPrefix with >> their names properly camelCased, but the diff.noPrefix is listed >> there in all lowercase. Also these configuration variables, both >> existing ones and the {src,dst}Prefix we recently added, were >> spelled in all lowercase in the tests in t4013. >> >> Now we are done with the main change, clean these up. >> >> Signed-off-by: Junio C Hamano <gitster@pobox.com> > > Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> > > And thanks for merging the patch! > > Cheers, > Peter Thanks.
diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt index 6c7e09a1ef5e..fea89291c675 100644 --- a/Documentation/config/diff.txt +++ b/Documentation/config/diff.txt @@ -111,6 +111,12 @@ diff.mnemonicPrefix:: diff.noprefix:: If set, 'git diff' does not show any source or destination prefix. +diff.srcPrefix:: + If set, 'git diff' uses this source prefix. Defaults to "a/". + +diff.dstPrefix:: + If set, 'git diff' uses this destination prefix. Defaults to "b/". + diff.relative:: If set to 'true', 'git diff' does not show changes outside of the directory and show pathnames relative to the current directory. diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index aaaff0d46f0c..0e9456957e37 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -865,8 +865,9 @@ endif::git-format-patch[] --default-prefix:: Use the default source and destination prefixes ("a/" and "b/"). - This is usually the default already, but may be used to override - config such as `diff.noprefix`. + This overrides configuration variables such as `diff.noprefix`, + `diff.srcPrefix`, `diff.dstPrefix`, and `diff.mnemonicPrefix` + (see `git-config`(1)). --line-prefix=<prefix>:: Prepend an additional prefix to every line of output. diff --git a/diff.c b/diff.c index e50def45383e..108c1875775d 100644 --- a/diff.c +++ b/diff.c @@ -62,6 +62,8 @@ static const char *diff_order_file_cfg; int diff_auto_refresh_index = 1; static int diff_mnemonic_prefix; static int diff_no_prefix; +static const char *diff_src_prefix = "a/"; +static const char *diff_dst_prefix = "b/"; static int diff_relative; static int diff_stat_name_width; static int diff_stat_graph_width; @@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const char *value, diff_no_prefix = git_config_bool(var, value); return 0; } + if (!strcmp(var, "diff.srcprefix")) { + return git_config_string(&diff_src_prefix, var, value); + } + if (!strcmp(var, "diff.dstprefix")) { + return git_config_string(&diff_dst_prefix, var, value); + } if (!strcmp(var, "diff.relative")) { diff_relative = git_config_bool(var, value); return 0; @@ -3425,8 +3433,8 @@ void diff_set_noprefix(struct diff_options *options) void diff_set_default_prefix(struct diff_options *options) { - options->a_prefix = "a/"; - options->b_prefix = "b/"; + options->a_prefix = diff_src_prefix; + options->b_prefix = diff_dst_prefix; } struct userdiff_driver *get_textconv(struct repository *r, @@ -5362,6 +5370,8 @@ static int diff_opt_default_prefix(const struct option *opt, BUG_ON_OPT_NEG(unset); BUG_ON_OPT_ARG(optarg); + diff_src_prefix = "a/"; + diff_dst_prefix = "b/"; diff_set_default_prefix(options); return 0; } diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 1e3b2dbea484..6bf69888f258 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -663,6 +663,41 @@ test_expect_success 'diff --default-prefix overrides diff.mnemonicprefix' ' check_prefix actual a/file0 b/file0 ' +test_expect_success 'diff respects diff.srcprefix' ' + git -c diff.srcprefix=x/ diff >actual && + check_prefix actual x/file0 b/file0 +' + +test_expect_success 'diff respects diff.dstprefix' ' + git -c diff.dstprefix=y/ diff >actual && + check_prefix actual a/file0 y/file0 +' + +test_expect_success 'diff --src-prefix overrides diff.srcprefix' ' + git -c diff.srcprefix=y/ diff --src-prefix=z/ >actual && + check_prefix actual z/file0 b/file0 +' + +test_expect_success 'diff --dst-prefix overrides diff.dstprefix' ' + git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual && + check_prefix actual a/file0 z/file0 +' + +test_expect_success 'diff.*prefix ignored with diff.noprefix' ' + git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff >actual && + check_prefix actual file0 file0 +' + +test_expect_success 'diff.*prefix ignored with diff.mnemonicprefix' ' + git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix diff >actual && + check_prefix actual i/file0 w/file0 +' + +test_expect_success 'diff.*prefix ignored with --default-prefix' ' + git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix >actual && + check_prefix actual a/file0 b/file0 +' + test_expect_success 'diff --no-renames cannot be abbreviated' ' test_expect_code 129 git diff --no-rename >actual 2>error && test_must_be_empty actual &&
Allow the default prefixes "a/" and "b/" to be tweaked by the diff.srcPrefix and diff.dstPrefix configuration variables. Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> --- Changes to v4 (as pointed out by Dragan): - copy/paste-o fixed in the dstprefix test - reworded the description for the tests as suggested Dragan: good catch on the test, thanks for that. I think for the rewording of --default-prefix it might be faster if you reword it? Otherwise we might keep spinning this one for a quite a bit longer :) Documentation/config/diff.txt | 6 ++++++ Documentation/diff-options.txt | 5 +++-- diff.c | 14 ++++++++++++-- t/t4013-diff-various.sh | 35 ++++++++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 4 deletions(-)