git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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: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

* [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] 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] 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

* 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 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

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).