threads / patch / 23677

patchdiff: add configuration option for disabling diff prefixes.

Subject: [PATCH] diff: add configuration option for disabling diff prefixes.

## tl;dr

9 messages between May 3, 2010 and May 5, 2010. Diffs are folded; open one to read it.

replies: 8people: 3as markdown or json

Eli Collins· May 3, 2010, 02:03 UTC · lore
From: Eli Collins <eli@cloudera.com>
With new configuration "diff.noprefix", "git diff" does not show a source or destination prefix ala "git diff --no-prefix".
Signed-off-by: Eli Collins <eli@cloudera.com>
---
 Documentation/config.txt |    2 ++
 diff.c                   |    9 ++++++++-
 2 files changed, 10 insertions(+), 1 deletions(-)
Show changes to 2 files +10 −1

Documentation/config.txt, diff.c

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 92f851e..e883952 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -796,6 +796,8 @@ diff.mnemonicprefix::
 	standard "a/" and "b/" depending on what is being compared.  When
 	this configuration is in effect, reverse diff output also swaps
 	the order of the prefixes:
+diff.noprefix::
+	If set, 'git diff' does not show any source or destination prefix.
 `git diff`;;
 	compares the (i)ndex and the (w)ork tree;
 `git diff HEAD`;;
diff --git a/diff.c b/diff.c
index d0ecbc3..07269c2 100644
--- a/diff.c
+++ b/diff.c
@@ -30,6 +30,7 @@ static const char *diff_word_regex_cfg;
 static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
+static int diff_no_prefix;
 
 static char diff_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_RESET,
@@ -100,6 +101,10 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 		diff_mnemonic_prefix = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.noprefix")) {
+		diff_no_prefix = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "diff.external"))
 		return git_config_string(&external_diff_cmd_cfg, var, value);
 	if (!strcmp(var, "diff.wordregex"))
@@ -2559,7 +2564,9 @@ void diff_setup(struct diff_options *options)
 		DIFF_OPT_SET(options, COLOR_DIFF);
 	options->detect_rename = diff_detect_rename_default;
 
-	if (!diff_mnemonic_prefix) {
+	if (diff_no_prefix) {
+		options->a_prefix = options->b_prefix = "";
+	} else if (!diff_mnemonic_prefix) {
 		options->a_prefix = "a/";
 		options->b_prefix = "b/";
 	}
-- 
1.7.1.dirty
Bert Wesarg· May 3, 2010, 06:27 UTC · re: Eli Collins · lore

[PATCH] diff: add a 'path' meta header for non-renames and non-copies

Hi Eli, 

I like to have a path without any prefix in the diff header, too, but also like to see the diff mnemonic prefix (see diff.mnemonicprefix config). For some diffs there is such path, which is in the extended header of the diff for copies and renames. So I wrote the appended patch wich adds also an extended header for non-copies and non-renames which shows the path without any prefix.

Regards, Bert

--- 8< ---
From: Bert Wesarg <bert.wesarg@googlemail.com>
Subject: [PATCH] diff: add a 'path' meta header for non-renames and non-copies
This way you have always a path without any diff mnemonic prefix.
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 Documentation/config.txt              |    4 ++++
 Documentation/diff-generate-patch.txt |    4 ++++
 diff.c                                |   12 ++++++++++--
 3 files changed, 18 insertions(+), 2 deletions(-)
Show changes to 3 files +18 −2

Documentation/config.txt, Documentation/diff-generate-patch.txt, diff.c

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 92f851e..652365e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -807,6 +807,10 @@ diff.mnemonicprefix::
 `git diff --no-index a b`;;
 	compares two non-git things (1) and (2).
 
+diff.path::
+	Always add a 'path <path>' extended header into the diff output,
+	for non-copies and non-renames.
+
 diff.renameLimit::
 	The number of files to consider when performing the copy/rename
 	detection; equivalent to the 'git diff' option '-l'.
diff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt
index 8f9a241..67ba78f 100644
--- a/Documentation/diff-generate-patch.txt
+++ b/Documentation/diff-generate-patch.txt
@@ -30,6 +30,7 @@ the file that rename/copy produces, respectively.
        new mode <mode>
        deleted file mode <mode>
        new file mode <mode>
+       path <path>
        copy from <path>
        copy to <path>
        rename from <path>
@@ -38,6 +39,9 @@ the file that rename/copy produces, respectively.
        dissimilarity index <number>
        index <hash>..<hash> <mode>
 
+    The 'path' header will only show up, if the diff.path configure option
+    is set.
+
 3.  TAB, LF, double quote and backslash characters in pathnames
     are represented as `\t`, `\n`, `\"` and `\\`, respectively.
     If there is need for such substitution then the whole
diff --git a/diff.c b/diff.c
index d0ecbc3..fa33b9c 100644
--- a/diff.c
+++ b/diff.c
@@ -30,6 +30,7 @@ static const char *diff_word_regex_cfg;
 static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
+static int diff_path;
 
 static char diff_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_RESET,
@@ -104,6 +105,10 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 		return git_config_string(&external_diff_cmd_cfg, var, value);
 	if (!strcmp(var, "diff.wordregex"))
 		return git_config_string(&diff_word_regex_cfg, var, value);
+	if (!strcmp(var, "diff.path")) {
+		diff_path = git_config_bool(var, value);
+		return 0;
+	}
 
 	return git_diff_basic_config(var, value, cb);
 }
@@ -2351,8 +2356,11 @@ static void fill_metainfo(struct strbuf *msg,
 		}
 		/* fallthru */
 	default:
-		/* nothing */
-		;
+		if (diff_path) {
+			strbuf_addstr(msg, "path ");
+			quote_c_style(name, msg, NULL, 0);
+			strbuf_addch(msg, '\n');
+		}
 	}
 	if (one && two && hashcmp(one->sha1, two->sha1)) {
 		int abbrev = DIFF_OPT_TST(o, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
-- 
tg: (ddd02b7..) bw/always-print-a-path-meta-header (depends on: master)
Junio C Hamano· May 3, 2010, 07:30 UTC · re: Bert Wesarg · lore

Re: [PATCH] diff: add a 'path' meta header for non-renames and non-copies

Sorry, but none of these configuration variables are acceptable as-is.

Many scripts (the ones shipped with core-git, in contrib, or written by users) depend on their invocation of "git diff-*" family without any funny command line arguments like --no-prefix nor --src-prefix to produce a patch that can be applied without giving a custom -p value to "git apply".

Letting the user break that promise is already bad, and not giving the scripts any way to protect themselves from these configuration variables by overriding from the command line makes these doubly bad ideas.

Bert Wesarg· May 3, 2010, 07:37 UTC · re: Junio C Hamano · lore

Re: [PATCH] diff: add a 'path' meta header for non-renames and non-copies

On Mon, May 3, 2010 at 09:30, Junio C Hamano <gitster@pobox.com> wrote:
Show 10 quoted lines
> Sorry, but none of these configuration variables are acceptable as-is.
>
> Many scripts (the ones shipped with core-git, in contrib, or written by
> users) depend on their invocation of "git diff-*" family without any funny
> command line arguments like --no-prefix nor --src-prefix to produce a
> patch that can be applied without giving a custom -p value to "git apply".
>
> Letting the user break that promise is already bad, and not giving the
> scripts any way to protect themselves from these configuration variables
> by overriding from the command line makes these doubly bad ideas.

As far as I understand git_diff_ui_config() applies only for "git diff" not for any of the plumping "git diff-*" tools. Also I thought that these extended headers are exactly there to support arbitrary meta information. Therefore I think your arguments don't hold for my 'path' extended header, do they?

Thanks, Bert

>
Junio C Hamano· May 3, 2010, 16:29 UTC · re: Bert Wesarg · lore

Re: [PATCH] diff: add a 'path' meta header for non-renames and non-copies

Bert Wesarg <bert.wesarg@googlemail.com> writes:
> As far as I understand git_diff_ui_config() applies only for "git
> diff" not for any of the plumping "git diff-*" tools.

Ah, I missed that the patch was to the diff_ui parser. Sorry for confusion and thanks for straightening me up.

Bert Wesarg· May 3, 2010, 22:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] diff: add a 'path' meta header for non-renames and non-copies

On Mon, May 3, 2010 at 18:29, Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>> As far as I understand git_diff_ui_config() applies only for "git
>> diff" not for any of the plumping "git diff-*" tools.
>
> Ah, I missed that the patch was to the diff_ui parser.  Sorry for
> confusion and thanks for straightening me up.
>

So, now that I render your arguments voidless, do you have any other thoughts on adding a <path> extended header for non-renames and non-copies?

Regards, Bert

Junio C Hamano· May 4, 2010, 01:34 UTC · re: Bert Wesarg · lore

Re: [PATCH] diff: add a 'path' meta header for non-renames and non-copies

Bert Wesarg <bert.wesarg@googlemail.com> writes:
> ... do you have any other
> thoughts on adding a <path> extended header for non-renames and
> non-copies?

Other than "Why do we even need it?", no. We actually have been careful when designing the output format to make sure that necessary information appears.

See also this thread:
    http://thread.gmane.org/gmane.comp.version-control.git/3990/focus=4002

I wasn't too opposed to having a separate "Index: " line (or Anton's "name") like CVS does, but the conclusion of the thread was that it is not necessary.

Bert Wesarg· May 4, 2010, 19:30 UTC · re: Junio C Hamano · lore

Re: [PATCH] diff: add a 'path' meta header for non-renames and non-copies

On Tue, May 4, 2010 at 03:34, Junio C Hamano <gitster@pobox.com> wrote:
Show 18 quoted lines
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>> ... do you have any other
>> thoughts on adding a <path> extended header for non-renames and
>> non-copies?
>
> Other than "Why do we even need it?", no.  We actually have been careful
> when designing the output format to make sure that necessary information
> appears.
>
> See also this thread:
>
>    http://thread.gmane.org/gmane.comp.version-control.git/3990/focus=4002
>
> I wasn't too opposed to having a separate "Index: " line (or Anton's
> "name") like CVS does, but the conclusion of the thread was that it is not
> necessary.
>

Thanks for the pointer. The discussion revolves around technical arguments and the conclusion that this extra information is not necessary is reasonable. My intention for this information comes only from the usability and convenience side: I want a path without any prefix which I can select in the terminal and use this to open the file. Thats why it is only an option for the UI interface.

Bert
Eli Collins· May 5, 2010, 21:14 UTC · re: Bert Wesarg · lore

Re: [PATCH] diff: add a 'path' meta header for non-renames and non-copies

Ditto. I frequently generate patches for other systems (primarily svn) and they require diffs w/o perfixes. Seemed like there should be a way to enable this option in gitconfig so users don't need to type --no-prefix w every diff invocation or use an alias (aliasing diff to diff --no-prefix in gitconfig doesn't work and wouldn't be a good idea anyway).

Thanks, Eli

On Tue, May 4, 2010 at 12:30 PM, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
Show 29 quoted lines
> On Tue, May 4, 2010 at 03:34, Junio C Hamano <gitster@pobox.com> wrote:
>> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>>
>>> ... do you have any other
>>> thoughts on adding a <path> extended header for non-renames and
>>> non-copies?
>>
>> Other than "Why do we even need it?", no.  We actually have been careful
>> when designing the output format to make sure that necessary information
>> appears.
>>
>> See also this thread:
>>
>>    http://thread.gmane.org/gmane.comp.version-control.git/3990/focus=4002
>>
>> I wasn't too opposed to having a separate "Index: " line (or Anton's
>> "name") like CVS does, but the conclusion of the thread was that it is not
>> necessary.
>>
>
> Thanks for the pointer. The discussion revolves around technical
> arguments and the conclusion that this extra information is not
> necessary is reasonable. My intention for this information comes only
> from the usability and convenience side: I want a path without any
> prefix which I can select in the terminal and use this to open the
> file. Thats why it is only an option for the UI interface.
>
> Bert
>

← back to recent threads