* [PATCH] Add format-patch option --no-name-prefix. @ 2007-12-18 15:42 Pascal Obry 2007-12-18 15:47 ` Pascal Obry ` (3 more replies) 0 siblings, 4 replies; 22+ messages in thread From: Pascal Obry @ 2007-12-18 15:42 UTC (permalink / raw) To: git; +Cc: pascal This option can be used to generate a patch file where file names are relative to the Git root directory. Such a patch can then be applied with the standard patch tool using option -p0. Signed-off-by: Pascal Obry <pascal@obry.net> --- Documentation/git-format-patch.txt | 6 +++++- builtin-log.c | 7 ++++++- diff.c | 10 ++++++++-- diff.h | 1 + 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 6fb9429..5a642ad 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -15,7 +15,7 @@ SYNOPSIS [-n | --numbered | -N | --no-numbered] [--start-number <n>] [--numbered-files] [--in-reply-to=Message-Id] [--suffix=.<sfx>] - [--ignore-if-in-upstream] + [--ignore-if-in-upstream] [--no-name-prefix] [--subject-prefix=Subject-Prefix] [ <since> | <revision range> ] @@ -90,6 +90,10 @@ include::diff-options.txt[] without the default first line of the commit appended. Mutually exclusive with the --stdout option. +--no-name-prefix:: + Generate a patch file that can be applied with a patch(1) -p0 + from the Git root directory. + -k|--keep-subject:: Do not strip/add '[PATCH]' from the first line of the commit log message. diff --git a/builtin-log.c b/builtin-log.c index cc3cc90..36582bd 100644 --- a/builtin-log.c +++ b/builtin-log.c @@ -599,6 +599,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) int subject_prefix = 0; int ignore_if_in_upstream = 0; int thread = 0; + int no_name_prefix = 0; const char *in_reply_to = NULL; struct patch_ids ids; char *add_signoff = NULL; @@ -636,6 +637,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) } else if (!prefixcmp(argv[i], "--start-number=")) start_number = strtol(argv[i] + 15, NULL, 10); + else if (!prefixcmp(argv[i], "--no-name-prefix")) + no_name_prefix = 1; else if (!strcmp(argv[i], "--numbered-files")) numbered_files = 1; else if (!strcmp(argv[i], "--start-number")) { @@ -719,8 +722,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) die ("unrecognized argument: %s", argv[1]); if (!rev.diffopt.output_format) - rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY | DIFF_FORMAT_PATCH; + rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY | DIFF_FORMAT_PATCH | DIFF_FORMAT_NAME_PREFIX; + if (no_name_prefix) + rev.diffopt.output_format &= ~DIFF_FORMAT_NAME_PREFIX; if (!DIFF_OPT_TST(&rev.diffopt, TEXT)) DIFF_OPT_SET(&rev.diffopt, BINARY); diff --git a/diff.c b/diff.c index e26584c..f07d9c0 100644 --- a/diff.c +++ b/diff.c @@ -1212,8 +1212,14 @@ static void builtin_diff(const char *name_a, const char *set = diff_get_color_opt(o, DIFF_METAINFO); const char *reset = diff_get_color_opt(o, DIFF_RESET); - a_one = quote_two("a/", name_a + (*name_a == '/')); - b_two = quote_two("b/", name_b + (*name_b == '/')); + if (o->output_format & DIFF_FORMAT_NAME_PREFIX) { + a_one = quote_two("a/", name_a + (*name_a == '/')); + b_two = quote_two("b/", name_b + (*name_b == '/')); + } + else { + a_one = quote_two("", name_a + (*name_a == '/')); + b_two = quote_two("", name_b + (*name_b == '/')); + } lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null"; lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null"; printf("%sdiff --git %s %s%s\n", set, a_one, b_two, reset); diff --git a/diff.h b/diff.h index 7e8000a..86458a3 100644 --- a/diff.h +++ b/diff.h @@ -30,6 +30,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q, #define DIFF_FORMAT_SUMMARY 0x0008 #define DIFF_FORMAT_PATCH 0x0010 #define DIFF_FORMAT_SHORTSTAT 0x0020 +#define DIFF_FORMAT_NAME_PREFIX 0x0040 /* These override all above */ #define DIFF_FORMAT_NAME 0x0100 -- 1.5.4.rc0.56.g6fbe ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] Add format-patch option --no-name-prefix. 2007-12-18 15:42 [PATCH] Add format-patch option --no-name-prefix Pascal Obry @ 2007-12-18 15:47 ` Pascal Obry 2007-12-18 15:54 ` Johannes Sixt ` (2 subsequent siblings) 3 siblings, 0 replies; 22+ messages in thread From: Pascal Obry @ 2007-12-18 15:47 UTC (permalink / raw) To: git To give a bit of context about this patch. I need to send changeset to a server by e-mail for testing purpose before committing. The server is assuming that the patch can be applied with "patch -p0 < file" from the repository root. The option --no-name-prefix does just that, removing the leading 'a/' and 'b/'. Pascal. -- --|------------------------------------------------------ --| Pascal Obry Team-Ada Member --| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE --|------------------------------------------------------ --| http://www.obry.net --| "The best way to travel is by means of imagination" --| --| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add format-patch option --no-name-prefix. 2007-12-18 15:42 [PATCH] Add format-patch option --no-name-prefix Pascal Obry 2007-12-18 15:47 ` Pascal Obry @ 2007-12-18 15:54 ` Johannes Sixt 2007-12-18 15:59 ` Pascal Obry 2007-12-18 16:03 ` Andreas Ericsson 2007-12-18 16:21 ` [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" Johannes Schindelin 3 siblings, 1 reply; 22+ messages in thread From: Johannes Sixt @ 2007-12-18 15:54 UTC (permalink / raw) To: Pascal Obry; +Cc: git, pascal Pascal Obry schrieb: > This option can be used to generate a patch file > where file names are relative to the Git root > directory. Such a patch can then be applied with > the standard patch tool using option -p0. While I've always wondered what the a/ and b/ prefixes were good for (and I still do), I also wonder what's so different between typing patch -p0 and patch -p1 that we need another diff option for it. Ok, on my keyboard 0 is typed with the right hand, and 1 with the left hand, but... ?? -- Hannes ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add format-patch option --no-name-prefix. 2007-12-18 15:54 ` Johannes Sixt @ 2007-12-18 15:59 ` Pascal Obry 2007-12-18 16:50 ` Linus Torvalds 0 siblings, 1 reply; 22+ messages in thread From: Pascal Obry @ 2007-12-18 15:59 UTC (permalink / raw) To: Johannes Sixt; +Cc: Pascal Obry, git Johannes Sixt a écrit : > that we need another diff option for it. Ok, on my keyboard 0 is typed > with the right hand, and 1 with the left hand, but... ?? Because you just did not read my follow-up message :) I need this has I do not have the way to change the server applying the patch. So nothing wrong with my hands or fingers :) Pascal. -- --|------------------------------------------------------ --| Pascal Obry Team-Ada Member --| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE --|------------------------------------------------------ --| http://www.obry.net --| "The best way to travel is by means of imagination" --| --| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add format-patch option --no-name-prefix. 2007-12-18 15:59 ` Pascal Obry @ 2007-12-18 16:50 ` Linus Torvalds 2007-12-18 16:54 ` Pascal Obry ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: Linus Torvalds @ 2007-12-18 16:50 UTC (permalink / raw) To: Pascal Obry; +Cc: Johannes Sixt, Pascal Obry, git On Tue, 18 Dec 2007, Pascal Obry wrote: > Johannes Sixt a écrit : > > that we need another diff option for it. Ok, on my keyboard 0 is typed > > with the right hand, and 1 with the left hand, but... ?? > > Because you just did not read my follow-up message :) > > I need this has I do not have the way to change the server applying the > patch. So nothing wrong with my hands or fingers :) Well, we'd also need something like that for doing recursive diffs of submodules (since then we'd want to do the internal diff with the submodule name appended to the prefix), so I do think this whole "--prefix" makes sense. But I obviously think the version by Dscho is better (exactly because it's *not* enough to just clear the name prefix entirely), although I think that one is broken too - using ':' to separate the prefixes is *not* acceptable, since ':' is very possibly part of the prefix. So I think you'd need separate arguments for the from/to prefixes, and not try to shoehorn it into one argument. With possibly some simple form to say "no prefix". So maybe something like --src-prefix=<string> // default "a/" --dst-prefix=<string> // default "b/" --no-prefix // shorthand for --src-prefix="" --dst-prefix="" would work for everybody? Linus ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add format-patch option --no-name-prefix. 2007-12-18 16:50 ` Linus Torvalds @ 2007-12-18 16:54 ` Pascal Obry 2007-12-18 17:06 ` Johannes Schindelin 2007-12-18 20:07 ` [PATCH] Add format-patch option --no-name-prefix Junio C Hamano 2 siblings, 0 replies; 22+ messages in thread From: Pascal Obry @ 2007-12-18 16:54 UTC (permalink / raw) To: Linus Torvalds; +Cc: Johannes Sixt, Pascal Obry, git Linus Torvalds a écrit : > --src-prefix=<string> // default "a/" > --dst-prefix=<string> // default "b/" > --no-prefix // shorthand for --src-prefix="" --dst-prefix="" > > would work for everybody? Fine with me. Pascal. -- --|------------------------------------------------------ --| Pascal Obry Team-Ada Member --| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE --|------------------------------------------------------ --| http://www.obry.net --| "The best way to travel is by means of imagination" --| --| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add format-patch option --no-name-prefix. 2007-12-18 16:50 ` Linus Torvalds 2007-12-18 16:54 ` Pascal Obry @ 2007-12-18 17:06 ` Johannes Schindelin 2007-12-18 17:56 ` Pascal Obry 2007-12-18 20:07 ` [PATCH] Add format-patch option --no-name-prefix Junio C Hamano 2 siblings, 1 reply; 22+ messages in thread From: Johannes Schindelin @ 2007-12-18 17:06 UTC (permalink / raw) To: Linus Torvalds; +Cc: Pascal Obry, Johannes Sixt, Pascal Obry, git Hi, On Tue, 18 Dec 2007, Linus Torvalds wrote: > But I obviously think the version by Dscho is better (exactly because it's > *not* enough to just clear the name prefix entirely), although I think > that one is broken too - using ':' to separate the prefixes is *not* > acceptable, since ':' is very possibly part of the prefix. > > So I think you'd need separate arguments for the from/to prefixes, and not > try to shoehorn it into one argument. With possibly some simple form to > say "no prefix". So maybe something like > > --src-prefix=<string> // default "a/" > --dst-prefix=<string> // default "b/" > --no-prefix // shorthand for --src-prefix="" --dst-prefix="" > > would work for everybody? If this is preferred, please squash this: -- snipsnap -- Documentation/diff-options.txt | 11 ++++++++--- diff.c | 24 ++++++++++++++---------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 672a2d0..0d3dccc 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -211,9 +211,14 @@ endif::git-format-patch[] --no-ext-diff:: Disallow external diff drivers. ---prefix=<prefix1>[:<prefix2>]:: - Show the given path prefixes instead of "a/" and "b/". Leave - it empty to show no prefix at all. +--src-prefix <prefix>:: + Show the given source prefix instead of "a/". + +--dst-prefix <prefix>:: + Show the given destination prefix instead of "b/". + +--no-prefix:: + Do not show any source or destination prefix. For more detailed explanation on these common options, see also link:diffcore.html[diffcore documentation]. diff --git a/diff.c b/diff.c index 095bbb5..9bc5fea 100644 --- a/diff.c +++ b/diff.c @@ -2317,16 +2317,20 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) else if (40 < options->abbrev) options->abbrev = 40; } - else if (!strcmp(arg, "--prefix=")) { - char *colon = strchr(arg + 9, ':'); - options->a_prefix = arg + 9; - if (colon) { - *colon = '\0'; - options->b_prefix = colon + 1; - } - else - options->b_prefix = options->a_prefix; - } + else if (!strcmp(arg, "--src-prefix")) { + if (ac < 2) + return error("--src-prefix needs a parameter"); + options->a_prefix = arg + 1; + return 2; + } + else if (!strcmp(arg, "--dst-prefix")) { + if (ac < 2) + return error("--dst-prefix needs a parameter"); + options->b_prefix = arg + 1; + return 2; + } + else if (!strcmp(arg, "--no-prefix")) + options->a_prefix = options->b_prefix = ""; else return 0; return 1; ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] Add format-patch option --no-name-prefix. 2007-12-18 17:06 ` Johannes Schindelin @ 2007-12-18 17:56 ` Pascal Obry 2007-12-18 18:56 ` [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" Johannes Schindelin 0 siblings, 1 reply; 22+ messages in thread From: Pascal Obry @ 2007-12-18 17:56 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Linus Torvalds, Johannes Sixt, Pascal Obry, git Johannes Schindelin a écrit : > If this is preferred, please squash this: Work fine for me. We just need a consolidated patch with proper change log. Thanks, Pascal. -- --|------------------------------------------------------ --| Pascal Obry Team-Ada Member --| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE --|------------------------------------------------------ --| http://www.obry.net --| "The best way to travel is by means of imagination" --| --| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" 2007-12-18 17:56 ` Pascal Obry @ 2007-12-18 18:56 ` Johannes Schindelin 2007-12-18 19:03 ` Pascal Obry 2007-12-18 19:05 ` Linus Torvalds 0 siblings, 2 replies; 22+ messages in thread From: Johannes Schindelin @ 2007-12-18 18:56 UTC (permalink / raw) To: gitster; +Cc: Pascal Obry, Linus Torvalds, Johannes Sixt, Pascal Obry, git With the new options "--src-prefix <prefix>", "--dst-prefix <prefix>" and "--no-prefix", you can now control the path prefixes of the diff machinery. These used to by hardwired to "a/" for the source prefix and "b/" for the destination prefix. Initial patch by Pascal Obry. Sane option names suggested by Linus. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- On Tue, 18 Dec 2007, Pascal Obry wrote: > Johannes Schindelin a ?crit : > > If this is preferred, please squash this: > > Work fine for me. > > We just need a consolidated patch with proper change log. How does this grab you? Documentation/diff-options.txt | 9 +++++++++ diff.c | 33 +++++++++++++++++++++++++-------- diff.h | 1 + 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 9ecc1d7..0d3dccc 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -211,5 +211,14 @@ endif::git-format-patch[] --no-ext-diff:: Disallow external diff drivers. +--src-prefix <prefix>:: + Show the given source prefix instead of "a/". + +--dst-prefix <prefix>:: + Show the given destination prefix instead of "b/". + +--no-prefix:: + Do not show any source or destination prefix. + For more detailed explanation on these common options, see also link:diffcore.html[diffcore documentation]. diff --git a/diff.c b/diff.c index e26584c..43f62d8 100644 --- a/diff.c +++ b/diff.c @@ -290,9 +290,10 @@ static void emit_rewrite_diff(const char *name_a, const char *name_b, struct diff_filespec *one, struct diff_filespec *two, - int color_diff) + struct diff_options *o) { int lc_a, lc_b; + int color_diff = DIFF_OPT_TST(o, COLOR_DIFF); const char *name_a_tab, *name_b_tab; const char *metainfo = diff_get_color(color_diff, DIFF_METAINFO); const char *fraginfo = diff_get_color(color_diff, DIFF_FRAGINFO); @@ -309,9 +310,9 @@ static void emit_rewrite_diff(const char *name_a, diff_populate_filespec(two, 0); lc_a = count_lines(one->data, one->size); lc_b = count_lines(two->data, two->size); - printf("%s--- a/%s%s%s\n%s+++ b/%s%s%s\n%s@@ -", - metainfo, name_a, name_a_tab, reset, - metainfo, name_b, name_b_tab, reset, fraginfo); + printf("%s--- %s%s%s%s\n%s+++ %s%s%s%s\n%s@@ -", + metainfo, o->a_prefix, name_a, name_a_tab, reset, + metainfo, o->b_prefix, name_b, name_b_tab, reset, fraginfo); print_line_count(lc_a); printf(" +"); print_line_count(lc_b); @@ -1212,8 +1213,8 @@ static void builtin_diff(const char *name_a, const char *set = diff_get_color_opt(o, DIFF_METAINFO); const char *reset = diff_get_color_opt(o, DIFF_RESET); - a_one = quote_two("a/", name_a + (*name_a == '/')); - b_two = quote_two("b/", name_b + (*name_b == '/')); + a_one = quote_two(o->a_prefix, name_a + (*name_a == '/')); + b_two = quote_two(o->b_prefix, name_b + (*name_b == '/')); lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null"; lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null"; printf("%sdiff --git %s %s%s\n", set, a_one, b_two, reset); @@ -1242,8 +1243,7 @@ static void builtin_diff(const char *name_a, if ((one->mode ^ two->mode) & S_IFMT) goto free_ab_and_return; if (complete_rewrite) { - emit_rewrite_diff(name_a, name_b, one, two, - DIFF_OPT_TST(o, COLOR_DIFF)); + emit_rewrite_diff(name_a, name_b, one, two, o); o->found_changes = 1; goto free_ab_and_return; } @@ -2020,6 +2020,9 @@ void diff_setup(struct diff_options *options) else DIFF_OPT_CLR(options, COLOR_DIFF); options->detect_rename = diff_detect_rename_default; + + options->a_prefix = "a/"; + options->b_prefix = "b/"; } int diff_setup_done(struct diff_options *options) @@ -2291,6 +2294,20 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) else if (40 < options->abbrev) options->abbrev = 40; } + else if (!strcmp(arg, "--src-prefix")) { + if (ac < 2) + return error("--src-prefix needs a parameter"); + options->a_prefix = arg + 1; + return 2; + } + else if (!strcmp(arg, "--dst-prefix")) { + if (ac < 2) + return error("--dst-prefix needs a parameter"); + options->b_prefix = arg + 1; + return 2; + } + else if (!strcmp(arg, "--no-prefix")) + options->a_prefix = options->b_prefix = ""; else return 0; return 1; diff --git a/diff.h b/diff.h index 7e8000a..beccf85 100644 --- a/diff.h +++ b/diff.h @@ -69,6 +69,7 @@ struct diff_options { const char *orderfile; const char *pickaxe; const char *single_follow; + const char *a_prefix, *b_prefix; unsigned flags; int context; int break_opt; -- 1.5.4.rc0.70.g30f7 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" 2007-12-18 18:56 ` [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" Johannes Schindelin @ 2007-12-18 19:03 ` Pascal Obry 2007-12-18 19:05 ` Linus Torvalds 1 sibling, 0 replies; 22+ messages in thread From: Pascal Obry @ 2007-12-18 19:03 UTC (permalink / raw) To: Johannes Schindelin Cc: gitster, Linus Torvalds, Johannes Sixt, Pascal Obry, git Johannes Schindelin a écrit : > With the new options "--src-prefix <prefix>", "--dst-prefix <prefix>" > and "--no-prefix", you can now control the path prefixes of the diff > machinery. These used to by hardwired to "a/" for the source prefix > and "b/" for the destination prefix. > > Initial patch by Pascal Obry. Sane option names suggested by Linus. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Thanks this looks good to me. Pascal. -- --|------------------------------------------------------ --| Pascal Obry Team-Ada Member --| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE --|------------------------------------------------------ --| http://www.obry.net --| "The best way to travel is by means of imagination" --| --| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" 2007-12-18 18:56 ` [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" Johannes Schindelin 2007-12-18 19:03 ` Pascal Obry @ 2007-12-18 19:05 ` Linus Torvalds 2007-12-18 19:32 ` [PATCH v4] " Johannes Schindelin 1 sibling, 1 reply; 22+ messages in thread From: Linus Torvalds @ 2007-12-18 19:05 UTC (permalink / raw) To: Johannes Schindelin; +Cc: gitster, Pascal Obry, Johannes Sixt, Pascal Obry, git On Tue, 18 Dec 2007, Johannes Schindelin wrote: > > With the new options "--src-prefix <prefix>", "--dst-prefix <prefix>" I really would prefer "--[src|dst]-prefix=<prefix>" as a single argument. I think that's the more common form for long arguments, isn't it (with any short-format arguments usually using the "-L <prefix>" kind of form)? Linus ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4] Teach diff machinery to display other prefixes than "a/" and "b/" 2007-12-18 19:05 ` Linus Torvalds @ 2007-12-18 19:32 ` Johannes Schindelin 2007-12-18 19:55 ` Linus Torvalds 0 siblings, 1 reply; 22+ messages in thread From: Johannes Schindelin @ 2007-12-18 19:32 UTC (permalink / raw) To: Linus Torvalds; +Cc: gitster, Pascal Obry, Johannes Sixt, Pascal Obry, git With the new options "--src-prefix=<prefix>", "--dst-prefix=<prefix>" and "--no-prefix", you can now control the path prefixes of the diff machinery. These used to by hardwired to "a/" for the source prefix and "b/" for the destination prefix. Initial patch by Pascal Obry. Sane option names suggested by Linus. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- On Tue, 18 Dec 2007, Linus Torvalds wrote: > On Tue, 18 Dec 2007, Johannes Schindelin wrote: > > > > With the new options "--src-prefix <prefix>", "--dst-prefix > > <prefix>" > > I really would prefer "--[src|dst]-prefix=<prefix>" as a single > argument. > > I think that's the more common form for long arguments, isn't it > (with any short-format arguments usually using the "-L <prefix>" > kind of form)? Incidentally, this fixes src-prefix and dst-prefix (I wrote av[1] instead of arg + 1, but forgot to commit). So now, it is tested at least once per option. Documentation/diff-options.txt | 9 +++++++++ diff.c | 25 +++++++++++++++++-------- diff.h | 1 + 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 9ecc1d7..1a78635 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -211,5 +211,14 @@ endif::git-format-patch[] --no-ext-diff:: Disallow external diff drivers. +--src-prefix=<prefix>:: + Show the given source prefix instead of "a/". + +--dst-prefix=<prefix>:: + Show the given destination prefix instead of "b/". + +--no-prefix:: + Do not show any source or destination prefix. + For more detailed explanation on these common options, see also link:diffcore.html[diffcore documentation]. diff --git a/diff.c b/diff.c index e26584c..61fd492 100644 --- a/diff.c +++ b/diff.c @@ -290,9 +290,10 @@ static void emit_rewrite_diff(const char *name_a, const char *name_b, struct diff_filespec *one, struct diff_filespec *two, - int color_diff) + struct diff_options *o) { int lc_a, lc_b; + int color_diff = DIFF_OPT_TST(o, COLOR_DIFF); const char *name_a_tab, *name_b_tab; const char *metainfo = diff_get_color(color_diff, DIFF_METAINFO); const char *fraginfo = diff_get_color(color_diff, DIFF_FRAGINFO); @@ -309,9 +310,9 @@ static void emit_rewrite_diff(const char *name_a, diff_populate_filespec(two, 0); lc_a = count_lines(one->data, one->size); lc_b = count_lines(two->data, two->size); - printf("%s--- a/%s%s%s\n%s+++ b/%s%s%s\n%s@@ -", - metainfo, name_a, name_a_tab, reset, - metainfo, name_b, name_b_tab, reset, fraginfo); + printf("%s--- %s%s%s%s\n%s+++ %s%s%s%s\n%s@@ -", + metainfo, o->a_prefix, name_a, name_a_tab, reset, + metainfo, o->b_prefix, name_b, name_b_tab, reset, fraginfo); print_line_count(lc_a); printf(" +"); print_line_count(lc_b); @@ -1212,8 +1213,8 @@ static void builtin_diff(const char *name_a, const char *set = diff_get_color_opt(o, DIFF_METAINFO); const char *reset = diff_get_color_opt(o, DIFF_RESET); - a_one = quote_two("a/", name_a + (*name_a == '/')); - b_two = quote_two("b/", name_b + (*name_b == '/')); + a_one = quote_two(o->a_prefix, name_a + (*name_a == '/')); + b_two = quote_two(o->b_prefix, name_b + (*name_b == '/')); lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null"; lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null"; printf("%sdiff --git %s %s%s\n", set, a_one, b_two, reset); @@ -1242,8 +1243,7 @@ static void builtin_diff(const char *name_a, if ((one->mode ^ two->mode) & S_IFMT) goto free_ab_and_return; if (complete_rewrite) { - emit_rewrite_diff(name_a, name_b, one, two, - DIFF_OPT_TST(o, COLOR_DIFF)); + emit_rewrite_diff(name_a, name_b, one, two, o); o->found_changes = 1; goto free_ab_and_return; } @@ -2020,6 +2020,9 @@ void diff_setup(struct diff_options *options) else DIFF_OPT_CLR(options, COLOR_DIFF); options->detect_rename = diff_detect_rename_default; + + options->a_prefix = "a/"; + options->b_prefix = "b/"; } int diff_setup_done(struct diff_options *options) @@ -2291,6 +2294,12 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) else if (40 < options->abbrev) options->abbrev = 40; } + else if (!prefixcmp(arg, "--src-prefix=")) + options->a_prefix = arg + 13; + else if (!prefixcmp(arg, "--dst-prefix=")) + options->b_prefix = arg + 13; + else if (!strcmp(arg, "--no-prefix")) + options->a_prefix = options->b_prefix = ""; else return 0; return 1; diff --git a/diff.h b/diff.h index 7e8000a..beccf85 100644 --- a/diff.h +++ b/diff.h @@ -69,6 +69,7 @@ struct diff_options { const char *orderfile; const char *pickaxe; const char *single_follow; + const char *a_prefix, *b_prefix; unsigned flags; int context; int break_opt; -- 1.5.4.rc0.70.g30f7 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v4] Teach diff machinery to display other prefixes than "a/" and "b/" 2007-12-18 19:32 ` [PATCH v4] " Johannes Schindelin @ 2007-12-18 19:55 ` Linus Torvalds 0 siblings, 0 replies; 22+ messages in thread From: Linus Torvalds @ 2007-12-18 19:55 UTC (permalink / raw) To: Johannes Schindelin; +Cc: gitster, Pascal Obry, Johannes Sixt, Pascal Obry, git On Tue, 18 Dec 2007, Johannes Schindelin wrote: > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Thanks, Linus ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add format-patch option --no-name-prefix. 2007-12-18 16:50 ` Linus Torvalds 2007-12-18 16:54 ` Pascal Obry 2007-12-18 17:06 ` Johannes Schindelin @ 2007-12-18 20:07 ` Junio C Hamano 2 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2007-12-18 20:07 UTC (permalink / raw) To: Linus Torvalds; +Cc: Pascal Obry, Johannes Sixt, Pascal Obry, git Linus Torvalds <torvalds@linux-foundation.org> writes: > On Tue, 18 Dec 2007, Pascal Obry wrote: > >> Johannes Sixt a écrit : >> > that we need another diff option for it. Ok, on my keyboard 0 is typed >> > with the right hand, and 1 with the left hand, but... ?? >> >> Because you just did not read my follow-up message :) >> >> I need this has I do not have the way to change the server applying the >> patch. So nothing wrong with my hands or fingers :) > .... > So I think you'd need separate arguments for the from/to prefixes, and not > try to shoehorn it into one argument. With possibly some simple form to > say "no prefix". So maybe something like > > --src-prefix=<string> // default "a/" > --dst-prefix=<string> // default "b/" > --no-prefix // shorthand for --src-prefix="" --dst-prefix="" > > would work for everybody? One worry I have is that "diff --git" is validated more strictly than other diffs by "git-apply", and patches generated with arbitrary prefix would break it. It might make sense to drop " --git" from the patch header if we allow a/ and b/ to be changed inconsistently. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add format-patch option --no-name-prefix. 2007-12-18 15:42 [PATCH] Add format-patch option --no-name-prefix Pascal Obry 2007-12-18 15:47 ` Pascal Obry 2007-12-18 15:54 ` Johannes Sixt @ 2007-12-18 16:03 ` Andreas Ericsson 2007-12-18 16:11 ` Pascal Obry 2007-12-18 16:21 ` [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" Johannes Schindelin 3 siblings, 1 reply; 22+ messages in thread From: Andreas Ericsson @ 2007-12-18 16:03 UTC (permalink / raw) To: Pascal Obry; +Cc: git, pascal Pascal Obry wrote: > int thread = 0; > + int no_name_prefix = 0; Do we not need no double negations, yes? -- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add format-patch option --no-name-prefix. 2007-12-18 16:03 ` Andreas Ericsson @ 2007-12-18 16:11 ` Pascal Obry 2007-12-19 8:55 ` Andreas Ericsson 0 siblings, 1 reply; 22+ messages in thread From: Pascal Obry @ 2007-12-18 16:11 UTC (permalink / raw) To: Andreas Ericsson; +Cc: Pascal Obry, git Andreas Ericsson a écrit : > Pascal Obry wrote: >> int thread = 0; >> + int no_name_prefix = 0; > > Do we not need no double negations, yes? Not sure, looks clearer to use variable name corresponding to the option name to me... Pascal. -- --|------------------------------------------------------ --| Pascal Obry Team-Ada Member --| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE --|------------------------------------------------------ --| http://www.obry.net --| "The best way to travel is by means of imagination" --| --| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add format-patch option --no-name-prefix. 2007-12-18 16:11 ` Pascal Obry @ 2007-12-19 8:55 ` Andreas Ericsson 2007-12-19 9:21 ` Johannes Sixt 0 siblings, 1 reply; 22+ messages in thread From: Andreas Ericsson @ 2007-12-19 8:55 UTC (permalink / raw) To: Pascal Obry; +Cc: Pascal Obry, git Pascal Obry wrote: > Andreas Ericsson a écrit : >> Pascal Obry wrote: >>> int thread = 0; >>> + int no_name_prefix = 0; >> Do we not need no double negations, yes? > > Not sure, looks clearer to use variable name corresponding to the option > name to me... > Perhaps. We just had this discussion on the list where multiple people had extended a negative-sounding option. Personally I find it hard to parse and bug-prone to write (and edit) something like if (!no_prefix) add_the_prefix();", but perhaps that's just me. -- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Add format-patch option --no-name-prefix. 2007-12-19 8:55 ` Andreas Ericsson @ 2007-12-19 9:21 ` Johannes Sixt 0 siblings, 0 replies; 22+ messages in thread From: Johannes Sixt @ 2007-12-19 9:21 UTC (permalink / raw) To: Andreas Ericsson, Pascal Obry; +Cc: Pascal Obry, git Andreas Ericsson schrieb: > Pascal Obry wrote: >> Andreas Ericsson a écrit : >>> Pascal Obry wrote: >>>> int thread = 0; >>>> + int no_name_prefix = 0; >>> Do we not need no double negations, yes? >> >> Not sure, looks clearer to use variable name corresponding to the option >> name to me... Sure. Only that the option name is --name-prefix, and the no- part of it is just the negation (that many other long option names also offer). > Perhaps. We just had this discussion on the list where multiple people had > extended a negative-sounding option. Personally I find it hard to parse > and bug-prone to write (and edit) something like > > if (!no_prefix) > add_the_prefix();", > > but perhaps that's just me. Oh, no, you are not alone! Johannes "We-don't-need-no-steenkin'-duuble-negations" Sixt ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" 2007-12-18 15:42 [PATCH] Add format-patch option --no-name-prefix Pascal Obry ` (2 preceding siblings ...) 2007-12-18 16:03 ` Andreas Ericsson @ 2007-12-18 16:21 ` Johannes Schindelin 2007-12-18 16:45 ` Pascal Obry 3 siblings, 1 reply; 22+ messages in thread From: Johannes Schindelin @ 2007-12-18 16:21 UTC (permalink / raw) To: Pascal Obry; +Cc: git, pascal With the new option "--prefix=<prefix1>[:<prefix2>]" you can change the shown prefix, or suppress it (by specifying the empty string). Initial patch by Pascal Obry. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- On Tue, 18 Dec 2007, Pascal Obry wrote: > This option can be used to generate a patch file > where file names are relative to the Git root > directory. Such a patch can then be applied with > the standard patch tool using option -p0. How about this instead? It is not much longer, but more versatile, as you can actually change the prefix, and not only in format-patch. Oh, and if somebody has a better idea for the name of the option, I would appreciate your input. Documentation/diff-options.txt | 4 ++++ diff.c | 29 +++++++++++++++++++++-------- diff.h | 1 + 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 9ecc1d7..672a2d0 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -211,5 +211,9 @@ endif::git-format-patch[] --no-ext-diff:: Disallow external diff drivers. +--prefix=<prefix1>[:<prefix2>]:: + Show the given path prefixes instead of "a/" and "b/". Leave + it empty to show no prefix at all. + For more detailed explanation on these common options, see also link:diffcore.html[diffcore documentation]. diff --git a/diff.c b/diff.c index e26584c..404ba91 100644 --- a/diff.c +++ b/diff.c @@ -290,9 +290,10 @@ static void emit_rewrite_diff(const char *name_a, const char *name_b, struct diff_filespec *one, struct diff_filespec *two, - int color_diff) + struct diff_options *o) { int lc_a, lc_b; + int color_diff = DIFF_OPT_TST(o, COLOR_DIFF); const char *name_a_tab, *name_b_tab; const char *metainfo = diff_get_color(color_diff, DIFF_METAINFO); const char *fraginfo = diff_get_color(color_diff, DIFF_FRAGINFO); @@ -309,9 +310,9 @@ static void emit_rewrite_diff(const char *name_a, diff_populate_filespec(two, 0); lc_a = count_lines(one->data, one->size); lc_b = count_lines(two->data, two->size); - printf("%s--- a/%s%s%s\n%s+++ b/%s%s%s\n%s@@ -", - metainfo, name_a, name_a_tab, reset, - metainfo, name_b, name_b_tab, reset, fraginfo); + printf("%s--- %s%s%s%s\n%s+++ %s%s%s%s\n%s@@ -", + metainfo, o->a_prefix, name_a, name_a_tab, reset, + metainfo, o->b_prefix, name_b, name_b_tab, reset, fraginfo); print_line_count(lc_a); printf(" +"); print_line_count(lc_b); @@ -1212,8 +1213,8 @@ static void builtin_diff(const char *name_a, const char *set = diff_get_color_opt(o, DIFF_METAINFO); const char *reset = diff_get_color_opt(o, DIFF_RESET); - a_one = quote_two("a/", name_a + (*name_a == '/')); - b_two = quote_two("b/", name_b + (*name_b == '/')); + a_one = quote_two(o->a_prefix, name_a + (*name_a == '/')); + b_two = quote_two(o->b_prefix, name_b + (*name_b == '/')); lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null"; lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null"; printf("%sdiff --git %s %s%s\n", set, a_one, b_two, reset); @@ -1242,8 +1243,7 @@ static void builtin_diff(const char *name_a, if ((one->mode ^ two->mode) & S_IFMT) goto free_ab_and_return; if (complete_rewrite) { - emit_rewrite_diff(name_a, name_b, one, two, - DIFF_OPT_TST(o, COLOR_DIFF)); + emit_rewrite_diff(name_a, name_b, one, two, o); o->found_changes = 1; goto free_ab_and_return; } @@ -2020,6 +2020,9 @@ void diff_setup(struct diff_options *options) else DIFF_OPT_CLR(options, COLOR_DIFF); options->detect_rename = diff_detect_rename_default; + + options->a_prefix = "a/"; + options->b_prefix = "b/"; } int diff_setup_done(struct diff_options *options) @@ -2291,6 +2294,16 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) else if (40 < options->abbrev) options->abbrev = 40; } + else if (!strcmp(arg, "--prefix=")) { + char *colon = strchr(arg + 9, ':'); + options->a_prefix = arg + 9; + if (colon) { + *colon = '\0'; + options->b_prefix = colon + 1; + } + else + options->b_prefix = options->a_prefix; + } else return 0; return 1; diff --git a/diff.h b/diff.h index 7e8000a..beccf85 100644 --- a/diff.h +++ b/diff.h @@ -69,6 +69,7 @@ struct diff_options { const char *orderfile; const char *pickaxe; const char *single_follow; + const char *a_prefix, *b_prefix; unsigned flags; int context; int break_opt; -- 1.5.4.rc0.70.g30f7 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" 2007-12-18 16:21 ` [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" Johannes Schindelin @ 2007-12-18 16:45 ` Pascal Obry 2007-12-18 16:58 ` Johannes Schindelin 2007-12-18 17:02 ` Matthieu Moy 0 siblings, 2 replies; 22+ messages in thread From: Pascal Obry @ 2007-12-18 16:45 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Pascal Obry, git Johannes Schindelin a écrit : > With the new option "--prefix=<prefix1>[:<prefix2>]" you can change > the shown prefix, or suppress it (by specifying the empty string). Why not ? But do you have a motivation for this change ? I mean why would you want to use a completely different prefix ? My change was only for format-patch as this is used to build patch that some other tools can process. If we find a sensible usage for your prefix option I'm all for it as this solves also my problem. It is just that I think all options must have at least one usage :) Pascal. -- --|------------------------------------------------------ --| Pascal Obry Team-Ada Member --| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE --|------------------------------------------------------ --| http://www.obry.net --| "The best way to travel is by means of imagination" --| --| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" 2007-12-18 16:45 ` Pascal Obry @ 2007-12-18 16:58 ` Johannes Schindelin 2007-12-18 17:02 ` Matthieu Moy 1 sibling, 0 replies; 22+ messages in thread From: Johannes Schindelin @ 2007-12-18 16:58 UTC (permalink / raw) To: Pascal Obry; +Cc: Pascal Obry, git Hi, On Tue, 18 Dec 2007, Pascal Obry wrote: > Johannes Schindelin a ?crit : > > With the new option "--prefix=<prefix1>[:<prefix2>]" you can change > > the shown prefix, or suppress it (by specifying the empty string). > > Why not ? But do you have a motivation for this change ? I mean why > would you want to use a completely different prefix ? I vaguely remember that somebody once asked for something a la GNU patch's -B option. Besides, why restrict ourselves? I mean, really, my patch only adds 6/4 added/removed lines relative to your patch (part of which stems from the fact that I did not forget the "diff --git" line). Why not take the added value virtually for free? Ciao, Dscho ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" 2007-12-18 16:45 ` Pascal Obry 2007-12-18 16:58 ` Johannes Schindelin @ 2007-12-18 17:02 ` Matthieu Moy 1 sibling, 0 replies; 22+ messages in thread From: Matthieu Moy @ 2007-12-18 17:02 UTC (permalink / raw) To: Pascal Obry; +Cc: Johannes Schindelin, Pascal Obry, git Pascal Obry <pascal@obry.net> writes: > Johannes Schindelin a écrit : >> With the new option "--prefix=<prefix1>[:<prefix2>]" you can change >> the shown prefix, or suppress it (by specifying the empty string). > > Why not ? But do you have a motivation for this change ? I mean why > would you want to use a completely different prefix ? It can make sense when you send the patch to someone who might not know the context in which you wrote the patch, and who's not using git. Then --- your-version/foo.c +++ my-version/foo.c can be more expressive than a/ and b/. Some people like to have orig/ and mod/ also. I can live without --prefix=... option, but doing it general at once is a good idea, since adding this backward-compatibly on top of your patch would mean having several redundant options. -- Matthieu ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2007-12-19 9:22 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-12-18 15:42 [PATCH] Add format-patch option --no-name-prefix Pascal Obry 2007-12-18 15:47 ` Pascal Obry 2007-12-18 15:54 ` Johannes Sixt 2007-12-18 15:59 ` Pascal Obry 2007-12-18 16:50 ` Linus Torvalds 2007-12-18 16:54 ` Pascal Obry 2007-12-18 17:06 ` Johannes Schindelin 2007-12-18 17:56 ` Pascal Obry 2007-12-18 18:56 ` [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" Johannes Schindelin 2007-12-18 19:03 ` Pascal Obry 2007-12-18 19:05 ` Linus Torvalds 2007-12-18 19:32 ` [PATCH v4] " Johannes Schindelin 2007-12-18 19:55 ` Linus Torvalds 2007-12-18 20:07 ` [PATCH] Add format-patch option --no-name-prefix Junio C Hamano 2007-12-18 16:03 ` Andreas Ericsson 2007-12-18 16:11 ` Pascal Obry 2007-12-19 8:55 ` Andreas Ericsson 2007-12-19 9:21 ` Johannes Sixt 2007-12-18 16:21 ` [PATCH] Teach diff machinery to display other prefixes than "a/" and "b/" Johannes Schindelin 2007-12-18 16:45 ` Pascal Obry 2007-12-18 16:58 ` Johannes Schindelin 2007-12-18 17:02 ` Matthieu Moy
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).