[PATCH] Teach --text option to diff

[PATCH] Teach --text option to diff

[Stephan Feder]

Add new item text to struct diff_options.
If set then do not try to detect binary files.

Signed-off-by: Stephan Feder <sf@b-i-t.de>
---

I have to send patches of binary data to a customer but the builtin diff
was no help in this case.

Notes:

1. The shorthand -a for --text is not implemented. Is there a conflicting
shorthand?

2. For diffstat --text is ignored. It seems pointless because binary
patch data is not for human consumption anyway.

3. No documentation yet. If the patch is accepted I will add a short
description. To Documentation/diff-options.txt?

Regards
    Stephan


 diff.c |    5 ++++-
 diff.h |    1 +
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/diff.c b/diff.c
index f0450a8..1f0219d 100644
--- a/diff.c
+++ b/diff.c
@@ -723,7 +723,7 @@ static void builtin_diff(const char *nam
 	if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
 		die("unable to read files to diff");
 
-	if (mmfile_is_binary(&mf1) || mmfile_is_binary(&mf2)) {
+	if (!o->text && (mmfile_is_binary(&mf1) || mmfile_is_binary(&mf2))) {
 		/* Quite common confusing case */
 		if (mf1.size == mf2.size &&
 		    !memcmp(mf1.ptr, mf2.ptr, mf1.size))
@@ -1561,6 +1561,9 @@ int diff_opt_parse(struct diff_options *
 		options->output_format |= DIFF_FORMAT_PATCH;
 		options->full_index = options->binary = 1;
 	}
+	else if (!strcmp(arg, "--text")) {
+		options->text = 1;
+	}
 	else if (!strcmp(arg, "--name-only"))
 		options->output_format |= DIFF_FORMAT_NAME;
 	else if (!strcmp(arg, "--name-status"))
diff --git a/diff.h b/diff.h
index d557394..f80f646 100644
--- a/diff.h
+++ b/diff.h
@@ -42,6 +42,7 @@ struct diff_options {
 	unsigned recursive:1,
 		 tree_in_recursive:1,
 		 binary:1,
+		 text:1,
 		 full_index:1,
 		 silent_on_remove:1,
 		 find_copies_harder:1,
-- 
1.4.1.gbc483

Re: [PATCH] Teach --text option to diff

[Junio C Hamano]

Stephan Feder <sf@b-i-t.de> writes:

> I have to send patches of binary data to a customer but the builtin diff
> was no help in this case.

Given the previous patch, and also your point #2 below, I would
have expected you to introduce an option to force files to be
treated as binary even when they are otherwise misidentified as
text, but this patch is going the other way.

Interesting.

> 1. The shorthand -a for --text is not implemented. Is there a conflicting
> shorthand?

I do not think of one offhand, but it's the responsibility for
the party to propose such an enhancement to do the study ;-)

> 2. For diffstat --text is ignored. It seems pointless because binary
> patch data is not for human consumption anyway.

> 3. No documentation yet. If the patch is accepted I will add a short
> description. To Documentation/diff-options.txt?

Most likely that would be the place.

Re: [PATCH] Teach --text option to diff

[sf]

Junio C Hamano wrote:
> Stephan Feder <sf@b-i-t.de> writes:
> 
>> I have to send patches of binary data to a customer but the builtin diff
>> was no help in this case.
> 
> Given the previous patch, and also your point #2 below, I would
> have expected you to introduce an option to force files to be
> treated as binary even when they are otherwise misidentified as
> text, but this patch is going the other way.
> 
> Interesting.

Not really. I was surprised that the GNU diff option --text is
unsupported in the builtin diff.

> 
>> 1. The shorthand -a for --text is not implemented. Is there a conflicting
>> shorthand?
> 
> I do not think of one offhand, but it's the responsibility for
> the party to propose such an enhancement to do the study ;-)

Of course. I did not find any conflict but as the builtin diff and its
options are used by quite a lot of git commands I wanted to make sure.

If no objections arise I am going to add the shorthand.

>> 2. For diffstat --text is ignored. It seems pointless because binary
>> patch data is not for human consumption anyway.
> 
>> 3. No documentation yet. If the patch is accepted I will add a short
>> description. To Documentation/diff-options.txt?
> 
> Most likely that would be the place.

Good.

[PATCH 1/3] Teach diff -a as shorthand for --text

[Stephan Feder]

Signed-off-by: Stephan Feder <sf@b-i-t.de>
---
 diff.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/diff.c b/diff.c
index 1f0219d..b423491 100644
--- a/diff.c
+++ b/diff.c
@@ -1561,7 +1561,7 @@ int diff_opt_parse(struct diff_options *
 		options->output_format |= DIFF_FORMAT_PATCH;
 		options->full_index = options->binary = 1;
 	}
-	else if (!strcmp(arg, "--text")) {
+	else if (!strcmp(arg, "-a") || !strcmp(arg, "--text")) {
 		options->text = 1;
 	}
 	else if (!strcmp(arg, "--name-only"))
-- 
1.4.1.gbc483

[PATCH 2/3] Add -a and --text to common diff options help

[Stephan Feder]

Signed-off-by: Stephan Feder <sf@b-i-t.de>
---
 diff.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/diff.h b/diff.h
index f80f646..8ab0448 100644
--- a/diff.h
+++ b/diff.h
@@ -162,7 +162,8 @@ #define COMMON_DIFF_OPTIONS_HELP \
 "  -O<file>      reorder diffs according to the <file>.\n" \
 "  -S<string>    find filepair whose only one side contains the string.\n" \
 "  --pickaxe-all\n" \
-"                show all files diff when -S is used and hit is found.\n"
+"                show all files diff when -S is used and hit is found.\n" \
+"  -a  --text    treat all files as text.\n"
 
 extern int diff_queue_is_empty(void);
 extern void diff_flush(struct diff_options*);
-- 
1.4.1.gbc483

[PATCH 3/3] diff-options: Explain --text and -a

[Stephan Feder]

Signed-off-by: Stephan Feder <sf@b-i-t.de>
---
 Documentation/diff-options.txt |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index f523ec2..1a93629 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -94,5 +94,11 @@
 	Swap two inputs; that is, show differences from index or
 	on-disk file to tree contents.
 
+--text::
+	Treat all files as text.
+
+-a::
+	Shorthand for "--text".
+
 For more detailed explanation on these common options, see also
 link:diffcore.html[diffcore documentation].
-- 
1.4.1.gbc483