[RFC PATCH 0/2] gitk --color-words

[RFC PATCH 0/2] gitk --color-words

[Thomas Rast]

This little pair of patches teaches gitk a color-words diff view.

To get there, I finally got rid of the assumption that --color-words
only ever needs to output ANSI(?) color sequences.  Hopefully this
will help other people to use --color-words in nice frontends.

It uses an ad-hoc output format where newlines in the output are
insignificant (so as to be able to speak about added/removed words)
and newlines from the input are output as a line with only a '~'.

RFC among other things because

* I chose ~ more or less randomly.  Do any diff implementations out
  there use it for some other purpose?

* The --color-words code was the only user of color_fwrite_lines() so
  I tweaked it according to my needs.  But that makes it highly
  diff-specific again, so it's not in color.c any more.  Did anyone
  else intend to use it?

* The Tk part of the patch is 95% voodoo of the
  cut&paste-from-elsewhere vintage and I have no clue whether it works
  only by accident

* I still need to do some things: add docs for --porcelain, add tests
  to the first patch, move the gitk patch to affect only the subtree

* I'm not even burning midnight oil any more

Thomas Rast (2):
  Add diff --porcelain option for --color-words
  gitk: add the equivalent of diff --color-words

 color.c       |   28 ---------------
 color.h       |    1 -
 diff.c        |  105 +++++++++++++++++++++++++++++++++++++++++++++++---------
 diff.h        |    1 +
 gitk-git/gitk |   29 +++++++++++++++-
 5 files changed, 116 insertions(+), 48 deletions(-)

[PATCH 1/2] Add diff --porcelain option for --color-words

[Thomas Rast]

This teaches the --color-words mode a new option --porcelain that
disables color mode again and instead uses an ad-hoc format for the
word diff designed for machine reading:

* Newlines in the output are no longer significant

* Each diff unit is prefixed by [-+ ] and terminated by newline as
  before

* Newlines in the input are output as a line consisting only of a
  tilde '~'

Note that for backwards compatibility --color-words still implies
--color, so the order matters: if you say --porcelain --color-words,
the hunk headers will be colored; if you say --color-words
--porcelain, they won't.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 color.c |   28 -----------------
 color.h |    1 -
 diff.c  |  105 ++++++++++++++++++++++++++++++++++++++++++++++++++++----------
 diff.h  |    1 +
 4 files changed, 89 insertions(+), 46 deletions(-)

diff --git a/color.c b/color.c
index bcf4e2c..1b00554 100644
--- a/color.c
+++ b/color.c
@@ -211,31 +211,3 @@ int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...)
 	va_end(args);
 	return r;
 }
-
-/*
- * This function splits the buffer by newlines and colors the lines individually.
- *
- * Returns 0 on success.
- */
-int color_fwrite_lines(FILE *fp, const char *color,
-		size_t count, const char *buf)
-{
-	if (!*color)
-		return fwrite(buf, count, 1, fp) != 1;
-	while (count) {
-		char *p = memchr(buf, '\n', count);
-		if (p != buf && (fputs(color, fp) < 0 ||
-				fwrite(buf, p ? p - buf : count, 1, fp) != 1 ||
-				fputs(GIT_COLOR_RESET, fp) < 0))
-			return -1;
-		if (!p)
-			return 0;
-		if (fputc('\n', fp) < 0)
-			return -1;
-		count -= p + 1 - buf;
-		buf = p + 1;
-	}
-	return 0;
-}
-
-
diff --git a/color.h b/color.h
index 5c264b0..03ca064 100644
--- a/color.h
+++ b/color.h
@@ -61,6 +61,5 @@
 int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
 __attribute__((format (printf, 3, 4)))
 int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);
-int color_fwrite_lines(FILE *fp, const char *color, size_t count, const char *buf);
 
 #endif /* COLOR_H */
diff --git a/diff.c b/diff.c
index f5d93e9..1b4d620 100644
--- a/diff.c
+++ b/diff.c
@@ -577,8 +577,62 @@ struct diff_words_data {
 	const char *current_plus;
 	FILE *file;
 	regex_t *word_regex;
+	int porcelain;
 };
 
+int color_fwrite_lines(FILE *fp, const char *color,
+		size_t count, const char *buf)
+{
+	if (!*color)
+		return fwrite(buf, count, 1, fp) != 1;
+	while (count) {
+		char *p = memchr(buf, '\n', count);
+		if (p != buf && (fputs(color, fp) < 0 ||
+				fwrite(buf, p ? p - buf : count, 1, fp) != 1 ||
+				fputs(GIT_COLOR_RESET, fp) < 0))
+			return -1;
+		if (!p)
+			return 0;
+		if (fputc('\n', fp) < 0)
+			return -1;
+		count -= p + 1 - buf;
+		buf = p + 1;
+	}
+	return 0;
+}
+
+static int fn_out_diff_words_write_helper(FILE *fp,
+					   const char *color, const char *prefix,
+					   size_t count, const char *buf,
+					   int porcelain)
+{
+	const char *suffix = "\n";
+	const char *newline = "~\n";
+
+	if (!porcelain) {
+		if (!color)
+			return fwrite(buf, count, 1, fp);
+		prefix = color;
+		suffix = GIT_COLOR_RESET;
+		newline = "\n";
+	}
+
+	while (count) {
+		char *p = memchr(buf, '\n', count);
+		if (p != buf && (fputs(prefix, fp) < 0 ||
+				 fwrite(buf, p ? p - buf : count, 1, fp) != 1 ||
+				 fputs(suffix, fp) < 0))
+			return -1;
+		if (!p)
+			return 0;
+		if (fputs(newline, fp) < 0)
+			return -1;
+		count -= p + 1 - buf;
+		buf = p + 1;
+	}
+	return 0;
+}
+
 static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
 {
 	struct diff_words_data *diff_words = priv;
@@ -605,17 +659,21 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
 		plus_begin = plus_end = diff_words->plus.orig[plus_first].end;
 
 	if (diff_words->current_plus != plus_begin)
-		fwrite(diff_words->current_plus,
-				plus_begin - diff_words->current_plus, 1,
-				diff_words->file);
+		fn_out_diff_words_write_helper(diff_words->file,
+				NULL, " ",
+				plus_begin - diff_words->current_plus,
+				diff_words->current_plus,
+				diff_words->porcelain);
 	if (minus_begin != minus_end)
-		color_fwrite_lines(diff_words->file,
-				diff_get_color(1, DIFF_FILE_OLD),
-				minus_end - minus_begin, minus_begin);
+		fn_out_diff_words_write_helper(diff_words->file,
+				diff_get_color(1, DIFF_FILE_OLD), "-",
+				minus_end - minus_begin, minus_begin,
+				diff_words->porcelain);
 	if (plus_begin != plus_end)
-		color_fwrite_lines(diff_words->file,
-				diff_get_color(1, DIFF_FILE_NEW),
-				plus_end - plus_begin, plus_begin);
+		fn_out_diff_words_write_helper(diff_words->file,
+				diff_get_color(1, DIFF_FILE_NEW), "+",
+				plus_end - plus_begin, plus_begin,
+				diff_words->porcelain);
 
 	diff_words->current_plus = plus_end;
 }
@@ -700,9 +758,10 @@ static void diff_words_show(struct diff_words_data *diff_words)
 
 	/* special case: only removal */
 	if (!diff_words->plus.text.size) {
-		color_fwrite_lines(diff_words->file,
-			diff_get_color(1, DIFF_FILE_OLD),
-			diff_words->minus.text.size, diff_words->minus.text.ptr);
+		fn_out_diff_words_write_helper(diff_words->file,
+			diff_get_color(1, DIFF_FILE_OLD), "-",
+			diff_words->minus.text.size, diff_words->minus.text.ptr,
+			diff_words->porcelain);
 		diff_words->minus.text.size = 0;
 		return;
 	}
@@ -722,10 +781,11 @@ static void diff_words_show(struct diff_words_data *diff_words)
 	free(plus.ptr);
 	if (diff_words->current_plus != diff_words->plus.text.ptr +
 			diff_words->plus.text.size)
-		fwrite(diff_words->current_plus,
+		fn_out_diff_words_write_helper(diff_words->file,
+			NULL, " ",
 			diff_words->plus.text.ptr + diff_words->plus.text.size
-			- diff_words->current_plus, 1,
-			diff_words->file);
+			- diff_words->current_plus, diff_words->current_plus,
+			diff_words->porcelain);
 	diff_words->minus.text.size = diff_words->plus.text.size = 0;
 }
 
@@ -837,6 +897,8 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 
 	if (len < 1) {
 		emit_line(ecbdata->file, reset, reset, line, len);
+		if (ecbdata->diff_words && ecbdata->diff_words->porcelain)
+			fputs("~\n", ecbdata->file);
 		return;
 	}
 
@@ -851,9 +913,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 			return;
 		}
 		diff_words_flush(ecbdata);
-		line++;
-		len--;
+		if (!ecbdata->diff_words->porcelain) {
+			line++;
+			len--;
+		}
 		emit_line(ecbdata->file, plain, reset, line, len);
+		if (ecbdata->diff_words->porcelain)
+			fputs("~\n", ecbdata->file);
 		return;
 	}
 
@@ -1765,6 +1831,7 @@ static void builtin_diff(const char *name_a,
 				o->word_regex = userdiff_word_regex(two);
 			if (!o->word_regex)
 				o->word_regex = diff_word_regex_cfg;
+			ecbdata.diff_words->porcelain = DIFF_OPT_TST(o, PORCELAIN);
 			if (o->word_regex) {
 				ecbdata.diff_words->word_regex = (regex_t *)
 					xmalloc(sizeof(regex_t));
@@ -2852,6 +2919,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		DIFF_OPT_SET(options, COLOR_DIFF_WORDS);
 		options->word_regex = arg + 14;
 	}
+	else if (!strcmp(arg, "--porcelain")) {
+		DIFF_OPT_SET(options, PORCELAIN);
+		DIFF_OPT_CLR(options, COLOR_DIFF);
+	}
 	else if (!strcmp(arg, "--exit-code"))
 		DIFF_OPT_SET(options, EXIT_WITH_STATUS);
 	else if (!strcmp(arg, "--quiet"))
diff --git a/diff.h b/diff.h
index 6a71013..7fdd251 100644
--- a/diff.h
+++ b/diff.h
@@ -71,6 +71,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 #define DIFF_OPT_SUBMODULE_LOG       (1 << 23)
 #define DIFF_OPT_DIRTY_SUBMODULES    (1 << 24)
 #define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25)
+#define DIFF_OPT_PORCELAIN           (1 <<  26)
 
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
-- 
1.7.0.3.522.ga3f6f.dirty

[PATCH 2/2] gitk: add the equivalent of diff --color-words

[Thomas Rast]

Use the newly added 'diff --color-words --porcelain' to teach gitk a
color-words mode, triggered by a checkbox.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 gitk-git/gitk |   29 +++++++++++++++++++++++++++--
 1 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 1f36a3e..ee6e39c 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2240,6 +2240,9 @@ proc makewindow {} {
     ${NS}::checkbutton .bleft.mid.ignspace -text [mc "Ignore space change"] \
 	-command changeignorespace -variable ignorespace
     pack .bleft.mid.ignspace -side left -padx 5
+    ${NS}::checkbutton .bleft.mid.worddiff -text [mc "Word diff"] \
+	-command changeworddiff -variable worddiff
+    pack .bleft.mid.worddiff -side left -padx 5
     set ctext .bleft.bottom.ctext
     text $ctext -background $bgcolor -foreground $fgcolor \
 	-state disabled -font textfont \
@@ -7494,11 +7497,16 @@ proc changeignorespace {} {
     reselectline
 }
 
+proc changeworddiff {} {
+    reselectline
+}
+
 proc getblobdiffs {ids} {
     global blobdifffd diffids env
     global diffinhdr treediffs
     global diffcontext
     global ignorespace
+    global worddiff
     global limitdiffs vfilelimit curview
     global diffencoding targetline diffnparents
     global git_version
@@ -7515,6 +7523,9 @@ proc getblobdiffs {ids} {
     if {$ignorespace} {
 	append cmd " -w"
     }
+    if {$worddiff} {
+	append cmd " --color-words --porcelain"
+    }
     if {$limitdiffs && $vfilelimit($curview) ne {}} {
 	set cmd [concat $cmd -- $vfilelimit($curview)]
     }
@@ -7599,6 +7610,7 @@ proc getblobdiffline {bdf ids} {
     global ctext_file_names ctext_file_lines
     global diffinhdr treediffs mergemax diffnparents
     global diffencoding jump_to_here targetline diffline
+    global worddiff
 
     set nr 0
     $ctext conf -state normal
@@ -7727,15 +7739,22 @@ proc getblobdiffline {bdf ids} {
 	    # parse the prefix - one ' ', '-' or '+' for each parent
 	    set prefix [string range $line 0 [expr {$diffnparents - 1}]]
 	    set tag [expr {$diffnparents > 1? "m": "d"}]
+	    puts "prefix is '$prefix'"
 	    if {[string trim $prefix " -+"] eq {}} {
 		# prefix only has " ", "-" and "+" in it: normal diff line
 		set num [string first "-" $prefix]
+		if {$worddiff} {
+		    set line [string range $line 1 end]
+		}
 		if {$num >= 0} {
 		    # removed line, first parent with line is $num
 		    if {$num >= $mergemax} {
 			set num "max"
 		    }
-		    $ctext insert end "$line\n" $tag$num
+		    $ctext insert end "$line" $tag$num
+		    if {!$worddiff} {
+			$ctext insert end "\n" $tag$num
+		    }
 		} else {
 		    set tags {}
 		    if {[string first "+" $prefix] >= 0} {
@@ -7759,8 +7778,13 @@ proc getblobdiffline {bdf ids} {
 			    incr diffline
 			}
 		    }
-		    $ctext insert end "$line\n" $tags
+		    $ctext insert end "$line" $tags
+		    if {!$worddiff} {
+			$ctext insert end "\n" $tags
+		    }
 		}
+	    } elseif {$worddiff && $prefix eq "~"} {
+		$ctext insert end "\n" {}
 	    } else {
 		# "\ No newline at end of file",
 		# or something else we don't recognize
@@ -11379,6 +11403,7 @@ if {[tk windowingsystem] eq "win32"} {
 set diffcolors {red "#00a000" blue}
 set diffcontext 3
 set ignorespace 0
+set worddiff 0
 set markbgcolor "#e0e0ff"
 
 set circlecolors {white blue gray blue blue}
-- 
1.7.0.3.522.ga3f6f.dirty

Re: [PATCH 2/2] gitk: add the equivalent of diff --color-words

[Thomas Rast]

Thomas Rast wrote:
> @@ -7727,15 +7739,22 @@ proc getblobdiffline {bdf ids} {
>  	    # parse the prefix - one ' ', '-' or '+' for each parent
>  	    set prefix [string range $line 0 [expr {$diffnparents - 1}]]
>  	    set tag [expr {$diffnparents > 1? "m": "d"}]
> +	    puts "prefix is '$prefix'"

This is a leftover debugging print and should be removed if you want
to try it... sorry for the noise.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

[PATCH v2 0/2] gitk --color-words

[Thomas Rast]

So here's a version intended for application.

[1/2] gained tests and docs.

[2/2] now disables the word-diff code for merge commits, since the
underlying --color-words also just gives up and shows a combined
diff.  It also patches /gitk, i.e., it's intended for application
inside the subtree.

I wasn't really sure about the last point -- this makes things a bit
complicated since Paul will have to apply 2/2, and Junio will have to
take care to only pull from Paul once 1/2 is in.


Thomas Rast (2):
  Add diff --porcelain option for --color-words
  gitk: add the equivalent of diff --color-words

[PATCH v2 1/2] Add diff --porcelain option for --color-words

[Thomas Rast]

This teaches the --color-words mode a new option --porcelain that
disables color mode again and instead uses an ad-hoc format for the
word diff designed for machine reading:

* Newlines in the output are no longer significant

* Each diff unit is prefixed by [-+ ] and terminated by newline as
  before

* Newlines in the input are output as a line consisting only of a
  tilde '~'

Note that for backwards compatibility --color-words still implies
--color, so the order matters: if you say --porcelain --color-words,
the hunk headers will be colored; if you say --color-words
--porcelain, they won't.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Documentation/diff-options.txt |   11 ++++
 color.c                        |   28 -----------
 color.h                        |    1 -
 diff.c                         |  105 +++++++++++++++++++++++++++++++++-------
 diff.h                         |    1 +
 t/t4034-diff-words.sh          |   27 ++++++++++
 6 files changed, 127 insertions(+), 46 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 60e922e..ca9c7ad 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -142,6 +142,17 @@ The regex can also be set via a diff driver or configuration option, see
 linkgit:gitattributes[1] or linkgit:git-config[1].  Giving it explicitly
 overrides any diff driver or configuration setting.  Diff drivers
 override configuration settings.
++
+With `--porcelain`, use a variant of the unified diff format:
+Added/removed/unchanged runs of words are printed as a line starting
+with `+`/`-`/` ` (respectively) and terminated by a newline.  Newlines
+in the input are shown as a tilde `~` on a line of its own.
+
+--porcelain::
+	Switch to plumbing mode, designed for consumption by a
+	porcelain script.  Currently, this only affects
+	`--color-words`, which see.
+
 endif::git-format-patch[]
 
 --no-renames::
diff --git a/color.c b/color.c
index bcf4e2c..1b00554 100644
--- a/color.c
+++ b/color.c
@@ -211,31 +211,3 @@ int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...)
 	va_end(args);
 	return r;
 }
-
-/*
- * This function splits the buffer by newlines and colors the lines individually.
- *
- * Returns 0 on success.
- */
-int color_fwrite_lines(FILE *fp, const char *color,
-		size_t count, const char *buf)
-{
-	if (!*color)
-		return fwrite(buf, count, 1, fp) != 1;
-	while (count) {
-		char *p = memchr(buf, '\n', count);
-		if (p != buf && (fputs(color, fp) < 0 ||
-				fwrite(buf, p ? p - buf : count, 1, fp) != 1 ||
-				fputs(GIT_COLOR_RESET, fp) < 0))
-			return -1;
-		if (!p)
-			return 0;
-		if (fputc('\n', fp) < 0)
-			return -1;
-		count -= p + 1 - buf;
-		buf = p + 1;
-	}
-	return 0;
-}
-
-
diff --git a/color.h b/color.h
index 5c264b0..03ca064 100644
--- a/color.h
+++ b/color.h
@@ -61,6 +61,5 @@
 int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
 __attribute__((format (printf, 3, 4)))
 int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);
-int color_fwrite_lines(FILE *fp, const char *color, size_t count, const char *buf);
 
 #endif /* COLOR_H */
diff --git a/diff.c b/diff.c
index f5d93e9..1b4d620 100644
--- a/diff.c
+++ b/diff.c
@@ -577,8 +577,62 @@ struct diff_words_data {
 	const char *current_plus;
 	FILE *file;
 	regex_t *word_regex;
+	int porcelain;
 };
 
+int color_fwrite_lines(FILE *fp, const char *color,
+		size_t count, const char *buf)
+{
+	if (!*color)
+		return fwrite(buf, count, 1, fp) != 1;
+	while (count) {
+		char *p = memchr(buf, '\n', count);
+		if (p != buf && (fputs(color, fp) < 0 ||
+				fwrite(buf, p ? p - buf : count, 1, fp) != 1 ||
+				fputs(GIT_COLOR_RESET, fp) < 0))
+			return -1;
+		if (!p)
+			return 0;
+		if (fputc('\n', fp) < 0)
+			return -1;
+		count -= p + 1 - buf;
+		buf = p + 1;
+	}
+	return 0;
+}
+
+static int fn_out_diff_words_write_helper(FILE *fp,
+					   const char *color, const char *prefix,
+					   size_t count, const char *buf,
+					   int porcelain)
+{
+	const char *suffix = "\n";
+	const char *newline = "~\n";
+
+	if (!porcelain) {
+		if (!color)
+			return fwrite(buf, count, 1, fp);
+		prefix = color;
+		suffix = GIT_COLOR_RESET;
+		newline = "\n";
+	}
+
+	while (count) {
+		char *p = memchr(buf, '\n', count);
+		if (p != buf && (fputs(prefix, fp) < 0 ||
+				 fwrite(buf, p ? p - buf : count, 1, fp) != 1 ||
+				 fputs(suffix, fp) < 0))
+			return -1;
+		if (!p)
+			return 0;
+		if (fputs(newline, fp) < 0)
+			return -1;
+		count -= p + 1 - buf;
+		buf = p + 1;
+	}
+	return 0;
+}
+
 static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
 {
 	struct diff_words_data *diff_words = priv;
@@ -605,17 +659,21 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
 		plus_begin = plus_end = diff_words->plus.orig[plus_first].end;
 
 	if (diff_words->current_plus != plus_begin)
-		fwrite(diff_words->current_plus,
-				plus_begin - diff_words->current_plus, 1,
-				diff_words->file);
+		fn_out_diff_words_write_helper(diff_words->file,
+				NULL, " ",
+				plus_begin - diff_words->current_plus,
+				diff_words->current_plus,
+				diff_words->porcelain);
 	if (minus_begin != minus_end)
-		color_fwrite_lines(diff_words->file,
-				diff_get_color(1, DIFF_FILE_OLD),
-				minus_end - minus_begin, minus_begin);
+		fn_out_diff_words_write_helper(diff_words->file,
+				diff_get_color(1, DIFF_FILE_OLD), "-",
+				minus_end - minus_begin, minus_begin,
+				diff_words->porcelain);
 	if (plus_begin != plus_end)
-		color_fwrite_lines(diff_words->file,
-				diff_get_color(1, DIFF_FILE_NEW),
-				plus_end - plus_begin, plus_begin);
+		fn_out_diff_words_write_helper(diff_words->file,
+				diff_get_color(1, DIFF_FILE_NEW), "+",
+				plus_end - plus_begin, plus_begin,
+				diff_words->porcelain);
 
 	diff_words->current_plus = plus_end;
 }
@@ -700,9 +758,10 @@ static void diff_words_show(struct diff_words_data *diff_words)
 
 	/* special case: only removal */
 	if (!diff_words->plus.text.size) {
-		color_fwrite_lines(diff_words->file,
-			diff_get_color(1, DIFF_FILE_OLD),
-			diff_words->minus.text.size, diff_words->minus.text.ptr);
+		fn_out_diff_words_write_helper(diff_words->file,
+			diff_get_color(1, DIFF_FILE_OLD), "-",
+			diff_words->minus.text.size, diff_words->minus.text.ptr,
+			diff_words->porcelain);
 		diff_words->minus.text.size = 0;
 		return;
 	}
@@ -722,10 +781,11 @@ static void diff_words_show(struct diff_words_data *diff_words)
 	free(plus.ptr);
 	if (diff_words->current_plus != diff_words->plus.text.ptr +
 			diff_words->plus.text.size)
-		fwrite(diff_words->current_plus,
+		fn_out_diff_words_write_helper(diff_words->file,
+			NULL, " ",
 			diff_words->plus.text.ptr + diff_words->plus.text.size
-			- diff_words->current_plus, 1,
-			diff_words->file);
+			- diff_words->current_plus, diff_words->current_plus,
+			diff_words->porcelain);
 	diff_words->minus.text.size = diff_words->plus.text.size = 0;
 }
 
@@ -837,6 +897,8 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 
 	if (len < 1) {
 		emit_line(ecbdata->file, reset, reset, line, len);
+		if (ecbdata->diff_words && ecbdata->diff_words->porcelain)
+			fputs("~\n", ecbdata->file);
 		return;
 	}
 
@@ -851,9 +913,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 			return;
 		}
 		diff_words_flush(ecbdata);
-		line++;
-		len--;
+		if (!ecbdata->diff_words->porcelain) {
+			line++;
+			len--;
+		}
 		emit_line(ecbdata->file, plain, reset, line, len);
+		if (ecbdata->diff_words->porcelain)
+			fputs("~\n", ecbdata->file);
 		return;
 	}
 
@@ -1765,6 +1831,7 @@ static void builtin_diff(const char *name_a,
 				o->word_regex = userdiff_word_regex(two);
 			if (!o->word_regex)
 				o->word_regex = diff_word_regex_cfg;
+			ecbdata.diff_words->porcelain = DIFF_OPT_TST(o, PORCELAIN);
 			if (o->word_regex) {
 				ecbdata.diff_words->word_regex = (regex_t *)
 					xmalloc(sizeof(regex_t));
@@ -2852,6 +2919,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		DIFF_OPT_SET(options, COLOR_DIFF_WORDS);
 		options->word_regex = arg + 14;
 	}
+	else if (!strcmp(arg, "--porcelain")) {
+		DIFF_OPT_SET(options, PORCELAIN);
+		DIFF_OPT_CLR(options, COLOR_DIFF);
+	}
 	else if (!strcmp(arg, "--exit-code"))
 		DIFF_OPT_SET(options, EXIT_WITH_STATUS);
 	else if (!strcmp(arg, "--quiet"))
diff --git a/diff.h b/diff.h
index 6a71013..7fdd251 100644
--- a/diff.h
+++ b/diff.h
@@ -71,6 +71,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 #define DIFF_OPT_SUBMODULE_LOG       (1 << 23)
 #define DIFF_OPT_DIRTY_SUBMODULES    (1 << 24)
 #define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25)
+#define DIFF_OPT_PORCELAIN           (1 <<  26)
 
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 2e2e103..26637d3 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -55,6 +55,33 @@ test_expect_success 'word diff with runs of whitespace' '
 
 '
 
+sed 's/#.*$//' > expect <<EOF
+diff --git a/pre b/post
+index 330b04f..5ed8eff 100644
+--- a/pre
++++ b/post
+@@ -1,3 +1,7 @@
+-h(4)
++h(4),hh[44]
+~
+ # significant space
+~
+ a = b + c
+~
+~
++aa = a
+~
+~
++aeff = aeff * ( aaa )
+~
+EOF
+
+test_expect_success '--porcelain' '
+
+	word_diff --color-words --porcelain
+
+'
+
 cat > expect <<\EOF
 <WHITE>diff --git a/pre b/post<RESET>
 <WHITE>index 330b04f..5ed8eff 100644<RESET>
-- 
1.7.0.4.504.g4d2c

Re: [PATCH v2 1/2] Add diff --porcelain option for --color-words

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> This teaches the --color-words mode a new option --porcelain that
> disables color mode again and instead uses an ad-hoc format for the
> word diff designed for machine reading.

How readable can you make this for human consumption while still keeping
it machine readable?  The answer could be it already is human readble.

Two reasons I ask the above question are that I find the feature quite
interesting, and would want to see if it can be also fed to humans, and
that the combination of this new option and the existing --color-words is
misnamed.

What you are giving "git diff" is "word-level diff" as opposed to the
usual "line-level diff", isn't it?  The machinery may have already been
there, but it had a hardwired representation of the result to color pre-
and post- image words, and you are giving the result from the machinery
another representation with this patch.  If you call this --word-diff,
then it would become more clear that --color-words perhaps should have
been called --word-diff=color or something.

Besides, --porcelain invites "what does it do without --color-words?", to
which you wouldn't have a good answer, as non word-diff output is already
machine readable.

Re: [PATCH v2 1/2] Add diff --porcelain option for --color-words

[Miles Bader]

Junio C Hamano <gitster@pobox.com> writes:
> How readable can you make this for human consumption while still keeping
> it machine readable?  The answer could be it already is human readble.
>
> Two reasons I ask the above question are that I find the feature quite
> interesting, and would want to see if it can be also fed to humans, and
> that the combination of this new option and the existing --color-words is
> misnamed.

There's the format used by the "wdiff" program, which is more like
traditional diff output in that it doesn't use color, but is human
friendly, and also seems to be somewhat machine-parseable:

   $ echo 'This is a test' > /tmp/a
   $ echo 'This is funky test' > /tmp/b
   $ wdiff /tmp/a /tmp/b
   This is [-a-] {+funky+} test

[I say "somewhat" because wdiff itself doesn't appear to escape potentially
ambiguous content, e.g., if there's actually a "{+" in the file....]

-Miles

-- 
Marriage, n. The state or condition of a community consisting of a master, a
mistress and two slaves, making in all, two.

[PATCH v2 2/2] gitk: add the equivalent of diff --color-words

[Thomas Rast]

Use the newly added 'diff --color-words --porcelain' to teach gitk a
color-words mode, triggered by a checkbox.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 gitk |   29 +++++++++++++++++++++++++++--
 1 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/gitk b/gitk
index 1f36a3e..d269226 100755
--- a/gitk
+++ b/gitk
@@ -2240,6 +2240,9 @@ proc makewindow {} {
     ${NS}::checkbutton .bleft.mid.ignspace -text [mc "Ignore space change"] \
 	-command changeignorespace -variable ignorespace
     pack .bleft.mid.ignspace -side left -padx 5
+    ${NS}::checkbutton .bleft.mid.worddiff -text [mc "Word diff"] \
+	-command changeworddiff -variable worddiff
+    pack .bleft.mid.worddiff -side left -padx 5
     set ctext .bleft.bottom.ctext
     text $ctext -background $bgcolor -foreground $fgcolor \
 	-state disabled -font textfont \
@@ -7494,11 +7497,16 @@ proc changeignorespace {} {
     reselectline
 }
 
+proc changeworddiff {} {
+    reselectline
+}
+
 proc getblobdiffs {ids} {
     global blobdifffd diffids env
     global diffinhdr treediffs
     global diffcontext
     global ignorespace
+    global worddiff
     global limitdiffs vfilelimit curview
     global diffencoding targetline diffnparents
     global git_version
@@ -7515,6 +7523,9 @@ proc getblobdiffs {ids} {
     if {$ignorespace} {
 	append cmd " -w"
     }
+    if {$worddiff} {
+	append cmd " --color-words --porcelain"
+    }
     if {$limitdiffs && $vfilelimit($curview) ne {}} {
 	set cmd [concat $cmd -- $vfilelimit($curview)]
     }
@@ -7599,6 +7610,7 @@ proc getblobdiffline {bdf ids} {
     global ctext_file_names ctext_file_lines
     global diffinhdr treediffs mergemax diffnparents
     global diffencoding jump_to_here targetline diffline
+    global worddiff
 
     set nr 0
     $ctext conf -state normal
@@ -7727,15 +7739,22 @@ proc getblobdiffline {bdf ids} {
 	    # parse the prefix - one ' ', '-' or '+' for each parent
 	    set prefix [string range $line 0 [expr {$diffnparents - 1}]]
 	    set tag [expr {$diffnparents > 1? "m": "d"}]
+	    set dowords [expr {$worddiff && $diffnparents == 1}]
 	    if {[string trim $prefix " -+"] eq {}} {
 		# prefix only has " ", "-" and "+" in it: normal diff line
 		set num [string first "-" $prefix]
+		if {$dowords} {
+		    set line [string range $line 1 end]
+		}
 		if {$num >= 0} {
 		    # removed line, first parent with line is $num
 		    if {$num >= $mergemax} {
 			set num "max"
 		    }
-		    $ctext insert end "$line\n" $tag$num
+		    $ctext insert end "$line" $tag$num
+		    if {!$dowords} {
+			$ctext insert end "\n" $tag$num
+		    }
 		} else {
 		    set tags {}
 		    if {[string first "+" $prefix] >= 0} {
@@ -7759,8 +7778,13 @@ proc getblobdiffline {bdf ids} {
 			    incr diffline
 			}
 		    }
-		    $ctext insert end "$line\n" $tags
+		    $ctext insert end "$line" $tags
+		    if {!$dowords} {
+			$ctext insert end "\n" $tags
+		    }
 		}
+	    } elseif {$dowords && $prefix eq "~"} {
+		$ctext insert end "\n" {}
 	    } else {
 		# "\ No newline at end of file",
 		# or something else we don't recognize
@@ -11379,6 +11403,7 @@ if {[tk windowingsystem] eq "win32"} {
 set diffcolors {red "#00a000" blue}
 set diffcontext 3
 set ignorespace 0
+set worddiff 0
 set markbgcolor "#e0e0ff"
 
 set circlecolors {white blue gray blue blue}
-- 
1.7.0.4.504.g4d2c

Re: [PATCH v2 0/2] gitk --color-words

[Paul Mackerras]

On Sat, Apr 03, 2010 at 08:06:18PM +0200, Thomas Rast wrote:

> I wasn't really sure about the last point -- this makes things a bit
> complicated since Paul will have to apply 2/2, and Junio will have to
> take care to only pull from Paul once 1/2 is in.

You mean that the patch will mean that gitk will break if the
underlying git isn't new enough?  I have been careful to avoid that
sort of dependency as far as possible.  Have a look at how this is
handled for the --textconv and --submodule options in [getblobdiffs]
and do something similar for --color-words, please.

Paul.

Re: [PATCH v2 0/2] gitk --color-words

[Thomas Rast]

Paul Mackerras wrote:
> On Sat, Apr 03, 2010 at 08:06:18PM +0200, Thomas Rast wrote:
> 
> > I wasn't really sure about the last point -- this makes things a bit
> > complicated since Paul will have to apply 2/2, and Junio will have to
> > take care to only pull from Paul once 1/2 is in.
> 
> You mean that the patch will mean that gitk will break if the
> underlying git isn't new enough?  I have been careful to avoid that
> sort of dependency as far as possible.  Have a look at how this is
> handled for the --textconv and --submodule options in [getblobdiffs]
> and do something similar for --color-words, please.

Ok, I'll look into that.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

[PATCH 0/2] gitk --color-words

[Thomas Rast]

Miles Bader wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> > How readable can you make this for human consumption while still keeping
> > it machine readable?  The answer could be it already is human readble.
> >
> > Two reasons I ask the above question are that I find the feature quite
> > interesting, and would want to see if it can be also fed to humans, and
> > that the combination of this new option and the existing --color-words is
> > misnamed.
> 
> There's the format used by the "wdiff" program, which is more like
> traditional diff output in that it doesn't use color, but is human
> friendly, and also seems to be somewhat machine-parseable:
> 
>    $ echo 'This is a test' > /tmp/a
>    $ echo 'This is funky test' > /tmp/b
>    $ wdiff /tmp/a /tmp/b
>    This is [-a-] {+funky+} test
> 
> [I say "somewhat" because wdiff itself doesn't appear to escape potentially
> ambiguous content, e.g., if there's actually a "{+" in the file....]

Junio C Hamano wrote:
> If you call this --word-diff, then it would become more clear that
> --color-words perhaps should have been called --word-diff=color or
> something.

Excellent ideas!  I don't have anything to add ;-)

This makes [1/2] a rather new patch though, I moved the whole
prefix/suffix handing further out to accommodate different styles.

There's a little change in [2/2] apart from the obvious option
renaming, too: it interprets --color-words and --word-diff as an
initial setting for the checkbox.


Thomas Rast (2):
  diff: add --word-diff option that generalizes --color-words
  gitk: add the equivalent of diff --color-words

[PATCH 2/2] gitk: add the equivalent of diff --color-words

[Thomas Rast]

Use the newly added 'diff --word-diff=porcelain' to teach gitk a
color-words mode, triggered by a checkbox.

As an extra twist, automatically enable this word-diff support when
the user mentions a word-diff related option on the command line.
These options were previously ignored because they would break diff
parsing.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 gitk |   37 ++++++++++++++++++++++++++++++++++---
 1 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/gitk b/gitk
index 1f36a3e..05ed053 100755
--- a/gitk
+++ b/gitk
@@ -131,6 +131,7 @@ proc unmerged_files {files} {
 
 proc parseviewargs {n arglist} {
     global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env
+    global worddiff
 
     set vdatemode($n) 0
     set vmergeonly($n) 0
@@ -168,7 +169,7 @@ proc parseviewargs {n arglist} {
 		lappend diffargs $arg
 	    }
 	    "--raw" - "--patch-with-raw" - "--patch-with-stat" -
-	    "--name-only" - "--name-status" - "--color" - "--color-words" -
+	    "--name-only" - "--name-status" - "--color" -
 	    "--log-size" - "--pretty=*" - "--decorate" - "--abbrev-commit" -
 	    "--cc" - "-z" - "--header" - "--parents" - "--boundary" -
 	    "--no-color" - "-g" - "--walk-reflogs" - "--no-walk" -
@@ -177,6 +178,11 @@ proc parseviewargs {n arglist} {
 		# These cause our parsing of git log's output to fail, or else
 		# they're options we want to set ourselves, so ignore them.
 	    }
+	    "--color-words*" - "--word-diff*" {
+		# These trigger a word diff in the console interface,
+		# so help the user by enabling our own support
+		set worddiff 1
+	    }
 	    "--stat=*" - "--numstat" - "--shortstat" - "--summary" -
 	    "--check" - "--exit-code" - "--quiet" - "--topo-order" -
 	    "--full-history" - "--dense" - "--sparse" -
@@ -2240,6 +2246,9 @@ proc makewindow {} {
     ${NS}::checkbutton .bleft.mid.ignspace -text [mc "Ignore space change"] \
 	-command changeignorespace -variable ignorespace
     pack .bleft.mid.ignspace -side left -padx 5
+    ${NS}::checkbutton .bleft.mid.worddiff -text [mc "Word diff"] \
+	-command changeworddiff -variable worddiff
+    pack .bleft.mid.worddiff -side left -padx 5
     set ctext .bleft.bottom.ctext
     text $ctext -background $bgcolor -foreground $fgcolor \
 	-state disabled -font textfont \
@@ -7494,11 +7503,16 @@ proc changeignorespace {} {
     reselectline
 }
 
+proc changeworddiff {} {
+    reselectline
+}
+
 proc getblobdiffs {ids} {
     global blobdifffd diffids env
     global diffinhdr treediffs
     global diffcontext
     global ignorespace
+    global worddiff
     global limitdiffs vfilelimit curview
     global diffencoding targetline diffnparents
     global git_version
@@ -7515,6 +7529,9 @@ proc getblobdiffs {ids} {
     if {$ignorespace} {
 	append cmd " -w"
     }
+    if {$worddiff} {
+	append cmd " --word-diff=porcelain"
+    }
     if {$limitdiffs && $vfilelimit($curview) ne {}} {
 	set cmd [concat $cmd -- $vfilelimit($curview)]
     }
@@ -7599,6 +7616,7 @@ proc getblobdiffline {bdf ids} {
     global ctext_file_names ctext_file_lines
     global diffinhdr treediffs mergemax diffnparents
     global diffencoding jump_to_here targetline diffline
+    global worddiff
 
     set nr 0
     $ctext conf -state normal
@@ -7727,15 +7745,22 @@ proc getblobdiffline {bdf ids} {
 	    # parse the prefix - one ' ', '-' or '+' for each parent
 	    set prefix [string range $line 0 [expr {$diffnparents - 1}]]
 	    set tag [expr {$diffnparents > 1? "m": "d"}]
+	    set dowords [expr {$worddiff && $diffnparents == 1}]
 	    if {[string trim $prefix " -+"] eq {}} {
 		# prefix only has " ", "-" and "+" in it: normal diff line
 		set num [string first "-" $prefix]
+		if {$dowords} {
+		    set line [string range $line 1 end]
+		}
 		if {$num >= 0} {
 		    # removed line, first parent with line is $num
 		    if {$num >= $mergemax} {
 			set num "max"
 		    }
-		    $ctext insert end "$line\n" $tag$num
+		    $ctext insert end "$line" $tag$num
+		    if {!$dowords} {
+			$ctext insert end "\n" $tag$num
+		    }
 		} else {
 		    set tags {}
 		    if {[string first "+" $prefix] >= 0} {
@@ -7759,8 +7784,13 @@ proc getblobdiffline {bdf ids} {
 			    incr diffline
 			}
 		    }
-		    $ctext insert end "$line\n" $tags
+		    $ctext insert end "$line" $tags
+		    if {!$dowords} {
+			$ctext insert end "\n" $tags
+		    }
 		}
+	    } elseif {$dowords && $prefix eq "~"} {
+		$ctext insert end "\n" {}
 	    } else {
 		# "\ No newline at end of file",
 		# or something else we don't recognize
@@ -11379,6 +11409,7 @@ if {[tk windowingsystem] eq "win32"} {
 set diffcolors {red "#00a000" blue}
 set diffcontext 3
 set ignorespace 0
+set worddiff 0
 set markbgcolor "#e0e0ff"
 
 set circlecolors {white blue gray blue blue}
-- 
1.7.0.3.521.ga1e9e