Git development
 help / color / mirror / Atom feed
* [PATCH take 3 0/4] color-words improvements
From: Johannes Schindelin @ 2009-01-14 17:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, git, Santi Béjar
In-Reply-To: <7vprisj26i.fsf@gitster.siamese.dyndns.org>


This series is getting bigger and bigger, unfortunately, just what I tried
to avoid.

But at least I am pretty comfortable with the readability of the result,
and it adds tests -- finally.

Changes relative to the last round: color_fwrite_lines() had problems with
empty lines, and find_word_boundary() was replaced by find_word_boundaries(),
which finds not only the end of the next word, but the start, too.

The only "funny" thing I realized is that the lines which are output
by emit_line() add a RESET at the end of the line, and I do not do that
in color_fwrite_lines().

Can anybody think of undesired behavior as a consequence?

Johannes Schindelin (4):
  Add color_fwrite_lines(), a function coloring each line individually
  color-words: refactor word splitting and use ALLOC_GROW()
  color-words: change algorithm to allow for 0-character word
    boundaries
  color-words: take an optional regular expression describing words

 Documentation/diff-options.txt |    6 +-
 color.c                        |   28 ++++++
 color.h                        |    1 +
 diff.c                         |  203 ++++++++++++++++++++++++++--------------
 diff.h                         |    1 +
 t/t4034-diff-words.sh          |   86 +++++++++++++++++
 6 files changed, 253 insertions(+), 72 deletions(-)
 create mode 100755 t/t4034-diff-words.sh

^ permalink raw reply

* [PATCH 1/4] Add color_fwrite_lines(), a function coloring each line individually
From: Johannes Schindelin @ 2009-01-14 17:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, git, Santi Béjar
In-Reply-To: <alpine.DEB.1.00.0901141840100.3586@pacific.mpi-cbg.de>


We have to set the color before every line and reset it before every
newline.  Add a function color_fwrite_lines() which does that for us.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 color.c |   28 ++++++++++++++++++++++++++++
 color.h |    1 +
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/color.c b/color.c
index fc0b72a..d4ae83f 100644
--- a/color.c
+++ b/color.c
@@ -191,3 +191,31 @@ 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(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 6cf5c88..cd5c985 100644
--- a/color.h
+++ b/color.h
@@ -19,5 +19,6 @@ int git_config_colorbool(const char *var, const char *value, int stdout_is_tty);
 void color_parse(const char *var, const char *value, char *dst);
 int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
 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 */
-- 
1.6.1.243.g4c9c5a

^ permalink raw reply related

* [PATCH 2/4] color-words: refactor word splitting and use ALLOC_GROW()
From: Johannes Schindelin @ 2009-01-14 17:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, git, Santi Béjar
In-Reply-To: <alpine.DEB.1.00.0901141840100.3586@pacific.mpi-cbg.de>


Word splitting is now performed by the function diff_words_fill(),
avoiding having the same code twice.

In the same spirit, avoid duplicating the code of ALLOC_GROW().

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	This has not changed, actually.  Just for your convenience.

 diff.c |   40 +++++++++++++++++++---------------------
 1 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/diff.c b/diff.c
index f67e0b2..6d87ea5 100644
--- a/diff.c
+++ b/diff.c
@@ -326,10 +326,7 @@ struct diff_words_buffer {
 static void diff_words_append(char *line, unsigned long len,
 		struct diff_words_buffer *buffer)
 {
-	if (buffer->text.size + len > buffer->alloc) {
-		buffer->alloc = (buffer->text.size + len) * 3 / 2;
-		buffer->text.ptr = xrealloc(buffer->text.ptr, buffer->alloc);
-	}
+	ALLOC_GROW(buffer->text.ptr, buffer->text.size + len, buffer->alloc);
 	line++;
 	len--;
 	memcpy(buffer->text.ptr + buffer->text.size, line, len);
@@ -398,6 +395,22 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
 	}
 }
 
+/*
+ * This function splits the words in buffer->text, and stores the list with
+ * newline separator into out.
+ */
+static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out)
+{
+	int i;
+	out->size = buffer->text.size;
+	out->ptr = xmalloc(out->size);
+	memcpy(out->ptr, buffer->text.ptr, out->size);
+	for (i = 0; i < out->size; i++)
+		if (isspace(out->ptr[i]))
+			out->ptr[i] = '\n';
+	buffer->current = 0;
+}
+
 /* this executes the word diff on the accumulated buffers */
 static void diff_words_show(struct diff_words_data *diff_words)
 {
@@ -405,26 +418,11 @@ static void diff_words_show(struct diff_words_data *diff_words)
 	xdemitconf_t xecfg;
 	xdemitcb_t ecb;
 	mmfile_t minus, plus;
-	int i;
 
 	memset(&xpp, 0, sizeof(xpp));
 	memset(&xecfg, 0, sizeof(xecfg));
-	minus.size = diff_words->minus.text.size;
-	minus.ptr = xmalloc(minus.size);
-	memcpy(minus.ptr, diff_words->minus.text.ptr, minus.size);
-	for (i = 0; i < minus.size; i++)
-		if (isspace(minus.ptr[i]))
-			minus.ptr[i] = '\n';
-	diff_words->minus.current = 0;
-
-	plus.size = diff_words->plus.text.size;
-	plus.ptr = xmalloc(plus.size);
-	memcpy(plus.ptr, diff_words->plus.text.ptr, plus.size);
-	for (i = 0; i < plus.size; i++)
-		if (isspace(plus.ptr[i]))
-			plus.ptr[i] = '\n';
-	diff_words->plus.current = 0;
-
+	diff_words_fill(&diff_words->minus, &minus);
+	diff_words_fill(&diff_words->plus, &plus);
 	xpp.flags = XDF_NEED_MINIMAL;
 	xecfg.ctxlen = diff_words->minus.alloc + diff_words->plus.alloc;
 	xdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words,
-- 
1.6.1.243.g4c9c5a

^ permalink raw reply related

* [PATCH 3/4] color-words: change algorithm to allow for 0-character word boundaries
From: Johannes Schindelin @ 2009-01-14 17:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, git, Santi Béjar
In-Reply-To: <alpine.DEB.1.00.0901141840100.3586@pacific.mpi-cbg.de>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 8793 bytes --]


Up until now, the color-words code assumed that word boundaries are
identical to white space characters.

Therefore, it could get away with a very simple scheme: it copied the
hunks, substituted newlines for each white space character, called
libxdiff with the processed text, and then identified the text to
output by the offsets (which agreed since the original text had the
same length).

This code was ugly, for a number of reasons:

- it was impossible to introduce 0-character word boundaries,

- we had to print everything word by word, and

- the code needed extra special handling of newlines in the removed part.

Fix all of these issues by processing the text such that

- we build word lists, separated by newlines,

- we remember the original offsets for every word, and

- after calling libxdiff on the wordlists, we parse the hunk headers, and
  find the corresponding offsets, and then

- we print the removed/added parts in one go.

The pre and post samples in the test were provided by Santi Béjar.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 diff.c                |  153 +++++++++++++++++++++++++++----------------------
 t/t4034-diff-words.sh |   62 ++++++++++++++++++++
 2 files changed, 147 insertions(+), 68 deletions(-)
 create mode 100755 t/t4034-diff-words.sh

diff --git a/diff.c b/diff.c
index 6d87ea5..fe8b1f0 100644
--- a/diff.c
+++ b/diff.c
@@ -319,8 +319,10 @@ static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
 struct diff_words_buffer {
 	mmfile_t text;
 	long alloc;
-	long current; /* output pointer */
-	int suppressed_newline;
+	struct diff_words_orig {
+		const char *begin, *end;
+	} *orig;
+	int orig_nr, orig_alloc;
 };
 
 static void diff_words_append(char *line, unsigned long len,
@@ -335,80 +337,81 @@ static void diff_words_append(char *line, unsigned long len,
 
 struct diff_words_data {
 	struct diff_words_buffer minus, plus;
+	const char *current_plus;
 	FILE *file;
 };
 
-static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,
-		int suppress_newline)
-{
-	const char *ptr;
-	int eol = 0;
-
-	if (len == 0)
-		return;
-
-	ptr  = buffer->text.ptr + buffer->current;
-	buffer->current += len;
-
-	if (ptr[len - 1] == '\n') {
-		eol = 1;
-		len--;
-	}
-
-	fputs(diff_get_color(1, color), file);
-	fwrite(ptr, len, 1, file);
-	fputs(diff_get_color(1, DIFF_RESET), file);
-
-	if (eol) {
-		if (suppress_newline)
-			buffer->suppressed_newline = 1;
-		else
-			putc('\n', file);
-	}
-}
-
 static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
 {
 	struct diff_words_data *diff_words = priv;
+	int minus_first, minus_len, plus_first, plus_len;
+	const char *minus_begin, *minus_end, *plus_begin, *plus_end;
 
-	if (diff_words->minus.suppressed_newline) {
-		if (line[0] != '+')
-			putc('\n', diff_words->file);
-		diff_words->minus.suppressed_newline = 0;
-	}
+	if (line[0] != '@' || parse_hunk_header(line, len,
+			&minus_first, &minus_len, &plus_first, &plus_len))
+		return;
 
-	len--;
-	switch (line[0]) {
-		case '-':
-			print_word(diff_words->file,
-				   &diff_words->minus, len, DIFF_FILE_OLD, 1);
-			break;
-		case '+':
-			print_word(diff_words->file,
-				   &diff_words->plus, len, DIFF_FILE_NEW, 0);
-			break;
-		case ' ':
-			print_word(diff_words->file,
-				   &diff_words->plus, len, DIFF_PLAIN, 0);
-			diff_words->minus.current += len;
-			break;
-	}
+	minus_begin = diff_words->minus.orig[minus_first].begin;
+	minus_end = minus_len == 0 ? minus_begin :
+		diff_words->minus.orig[minus_first + minus_len - 1].end;
+	plus_begin = diff_words->plus.orig[plus_first].begin;
+	plus_end = plus_len == 0 ? plus_begin :
+		diff_words->plus.orig[plus_first + plus_len - 1].end;
+
+	if (diff_words->current_plus != plus_begin)
+		fwrite(diff_words->current_plus,
+				plus_begin - diff_words->current_plus, 1,
+				diff_words->file);
+	if (minus_begin != minus_end)
+		color_fwrite_lines(diff_words->file,
+				diff_get_color(1, DIFF_FILE_OLD),
+				minus_end - minus_begin, minus_begin);
+	if (plus_begin != plus_end)
+		color_fwrite_lines(diff_words->file,
+				diff_get_color(1, DIFF_FILE_NEW),
+				plus_end - plus_begin, plus_begin);
+
+	diff_words->current_plus = plus_end;
 }
 
 /*
- * This function splits the words in buffer->text, and stores the list with
- * newline separator into out.
+ * This function splits the words in buffer->text, stores the list with
+ * newline separator into out, and saves the offsets of the original words
+ * in buffer->orig.
  */
 static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out)
 {
-	int i;
-	out->size = buffer->text.size;
-	out->ptr = xmalloc(out->size);
-	memcpy(out->ptr, buffer->text.ptr, out->size);
-	for (i = 0; i < out->size; i++)
-		if (isspace(out->ptr[i]))
-			out->ptr[i] = '\n';
-	buffer->current = 0;
+	int i, j;
+
+	out->size = 0;
+	out->ptr = xmalloc(buffer->text.size);
+
+	/* fake an empty "0th" word */
+	ALLOC_GROW(buffer->orig, 1, buffer->orig_alloc);
+	buffer->orig[0].begin = buffer->orig[0].end = buffer->text.ptr;
+	buffer->orig_nr = 1;
+
+	for (i = 0; i < buffer->text.size; i++) {
+		if (isspace(buffer->text.ptr[i]))
+			continue;
+		for (j = i + 1; j < buffer->text.size &&
+				!isspace(buffer->text.ptr[j]); j++)
+			; /* find the end of the word */
+
+		/* store original boundaries */
+		ALLOC_GROW(buffer->orig, buffer->orig_nr + 1,
+				buffer->orig_alloc);
+		buffer->orig[buffer->orig_nr].begin = buffer->text.ptr + i;
+		buffer->orig[buffer->orig_nr].end = buffer->text.ptr + j;
+		buffer->orig_nr++;
+
+		/* store one word */
+		memcpy(out->ptr + out->size, buffer->text.ptr + i, j - i);
+		out->ptr[out->size + j - i] = '\n';
+		out->size += j - i + 1;
+
+		i = j - 1;
+	}
 }
 
 /* this executes the word diff on the accumulated buffers */
@@ -419,22 +422,34 @@ static void diff_words_show(struct diff_words_data *diff_words)
 	xdemitcb_t ecb;
 	mmfile_t minus, plus;
 
+	/* 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);
+		diff_words->minus.text.size = 0;
+		return;
+	}
+
+	diff_words->current_plus = diff_words->plus.text.ptr;
+
 	memset(&xpp, 0, sizeof(xpp));
 	memset(&xecfg, 0, sizeof(xecfg));
 	diff_words_fill(&diff_words->minus, &minus);
 	diff_words_fill(&diff_words->plus, &plus);
 	xpp.flags = XDF_NEED_MINIMAL;
-	xecfg.ctxlen = diff_words->minus.alloc + diff_words->plus.alloc;
+	xecfg.ctxlen = 0;
 	xdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words,
 		      &xpp, &xecfg, &ecb);
 	free(minus.ptr);
 	free(plus.ptr);
+	if (diff_words->current_plus != diff_words->plus.text.ptr +
+			diff_words->plus.text.size)
+		fwrite(diff_words->current_plus,
+			diff_words->plus.text.ptr + diff_words->plus.text.size
+			- diff_words->current_plus, 1,
+			diff_words->file);
 	diff_words->minus.text.size = diff_words->plus.text.size = 0;
-
-	if (diff_words->minus.suppressed_newline) {
-		putc('\n', diff_words->file);
-		diff_words->minus.suppressed_newline = 0;
-	}
 }
 
 typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);
@@ -458,7 +473,9 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
 			diff_words_show(ecbdata->diff_words);
 
 		free (ecbdata->diff_words->minus.text.ptr);
+		free (ecbdata->diff_words->minus.orig);
 		free (ecbdata->diff_words->plus.text.ptr);
+		free (ecbdata->diff_words->plus.orig);
 		free(ecbdata->diff_words);
 		ecbdata->diff_words = NULL;
 	}
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
new file mode 100755
index 0000000..b032bd3
--- /dev/null
+++ b/t/t4034-diff-words.sh
@@ -0,0 +1,62 @@
+#!/bin/sh
+
+test_description='word diff colors'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+	git config diff.color.old red
+	git config diff.color.new green
+
+'
+
+decrypt_color () {
+	sed \
+		-e 's/.\[1m/<WHITE>/g' \
+		-e 's/.\[31m/<RED>/g' \
+		-e 's/.\[32m/<GREEN>/g' \
+		-e 's/.\[36m/<BROWN>/g' \
+		-e 's/.\[m/<RESET>/g'
+}
+
+cat > pre <<\EOF
+h(4)
+
+a = b + c
+EOF
+
+cat > post <<\EOF
+h(4),hh[44]
+
+a = b + c
+
+aa = a
+
+aeff = aeff * ( aaa )
+EOF
+
+cat > expect <<\EOF
+<WHITE>diff --git a/pre b/post<RESET>
+<WHITE>index 330b04f..5ed8eff 100644<RESET>
+<WHITE>--- a/pre<RESET>
+<WHITE>+++ b/post<RESET>
+<BROWN>@@ -1,3 +1,7 @@<RESET>
+<RED>h(4)<RESET><GREEN>h(4),hh[44]<RESET>
+<RESET>
+a = b + c<RESET>
+
+<GREEN>aa = a<RESET>
+
+<GREEN>aeff = aeff * ( aaa )<RESET>
+EOF
+
+test_expect_success 'word diff with runs of whitespace' '
+
+	test_must_fail git diff --no-index --color-words pre post > output &&
+	decrypt_color < output > output.decrypted &&
+	test_cmp expect output.decrypted
+
+'
+
+test_done
-- 
1.6.1.243.g4c9c5a


^ permalink raw reply related

* [PATCH 4/4] color-words: take an optional regular expression describing words
From: Johannes Schindelin @ 2009-01-14 17:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, git, Santi Béjar
In-Reply-To: <alpine.DEB.1.00.0901141840100.3586@pacific.mpi-cbg.de>


In some applications, words are not delimited by white space.  To
allow for that, you can specify a regular expression describing
what makes a word with

	git diff --color-words='[A-Za-z0-9]+'

Note that words cannot contain newline characters.

As suggested by Thomas Rast, the words are the exact matches of the
regular expression.

Note that a regular expression beginning with a '^' will match only
a word at the beginning of the hunk, not a word at the beginning of
a line, and is probably not what you want.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/diff-options.txt |    6 +++-
 diff.c                         |   64 ++++++++++++++++++++++++++++++++++-----
 diff.h                         |    1 +
 t/t4034-diff-words.sh          |   24 +++++++++++++++
 4 files changed, 85 insertions(+), 10 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 1f8ce97..e546bfa 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -94,8 +94,12 @@ endif::git-format-patch[]
 	Turn off colored diff, even when the configuration file
 	gives the default to color output.
 
---color-words::
+--color-words[=regex]::
 	Show colored word diff, i.e. color words which have changed.
++
+Optionally, you can pass a regular expression that tells Git what the
+words are that you are looking for; The default is to interpret any
+stretch of non-whitespace as a word.
 
 --no-renames::
 	Turn off rename detection, even when the configuration
diff --git a/diff.c b/diff.c
index fe8b1f0..d5d7171 100644
--- a/diff.c
+++ b/diff.c
@@ -333,12 +333,14 @@ static void diff_words_append(char *line, unsigned long len,
 	len--;
 	memcpy(buffer->text.ptr + buffer->text.size, line, len);
 	buffer->text.size += len;
+	buffer->text.ptr[buffer->text.size] = '\0';
 }
 
 struct diff_words_data {
 	struct diff_words_buffer minus, plus;
 	const char *current_plus;
 	FILE *file;
+	regex_t *word_regex;
 };
 
 static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
@@ -374,17 +376,49 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
 	diff_words->current_plus = plus_end;
 }
 
+/* This function starts looking at *begin, and returns 0 iff a word was found. */
+static int find_word_boundaries(mmfile_t *buffer, regex_t *word_regex,
+		int *begin, int *end)
+{
+	if (word_regex && *begin < buffer->size) {
+		regmatch_t match[1];
+		if (!regexec(word_regex, buffer->ptr + *begin, 1, match, 0)) {
+			char *p = memchr(buffer->ptr + *begin + match[0].rm_so,
+					'\n', match[0].rm_eo);
+			*end = p ? p - buffer->ptr : match[0].rm_eo + *begin;
+			*begin += match[0].rm_so;
+			return *begin >= *end;
+		}
+		return -1;
+	}
+
+	/* find the next word */
+	while (*begin < buffer->size && isspace(buffer->ptr[*begin]))
+		(*begin)++;
+	if (*begin >= buffer->size)
+		return -1;
+
+	/* find the end of the word */
+	*end = *begin + 1;
+	while (*end < buffer->size && !isspace(buffer->ptr[*end]))
+		(*end)++;
+
+	return 0;
+}
+
 /*
  * This function splits the words in buffer->text, stores the list with
  * newline separator into out, and saves the offsets of the original words
  * in buffer->orig.
  */
-static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out)
+static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out,
+		regex_t *word_regex)
 {
 	int i, j;
+	long alloc = 0;
 
 	out->size = 0;
-	out->ptr = xmalloc(buffer->text.size);
+	out->ptr = NULL;
 
 	/* fake an empty "0th" word */
 	ALLOC_GROW(buffer->orig, 1, buffer->orig_alloc);
@@ -392,11 +426,8 @@ static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out)
 	buffer->orig_nr = 1;
 
 	for (i = 0; i < buffer->text.size; i++) {
-		if (isspace(buffer->text.ptr[i]))
-			continue;
-		for (j = i + 1; j < buffer->text.size &&
-				!isspace(buffer->text.ptr[j]); j++)
-			; /* find the end of the word */
+		if (find_word_boundaries(&buffer->text, word_regex, &i, &j))
+			return;
 
 		/* store original boundaries */
 		ALLOC_GROW(buffer->orig, buffer->orig_nr + 1,
@@ -406,6 +437,7 @@ static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out)
 		buffer->orig_nr++;
 
 		/* store one word */
+		ALLOC_GROW(out->ptr, out->size + j - i + 1, alloc);
 		memcpy(out->ptr + out->size, buffer->text.ptr + i, j - i);
 		out->ptr[out->size + j - i] = '\n';
 		out->size += j - i + 1;
@@ -435,9 +467,10 @@ static void diff_words_show(struct diff_words_data *diff_words)
 
 	memset(&xpp, 0, sizeof(xpp));
 	memset(&xecfg, 0, sizeof(xecfg));
-	diff_words_fill(&diff_words->minus, &minus);
-	diff_words_fill(&diff_words->plus, &plus);
+	diff_words_fill(&diff_words->minus, &minus, diff_words->word_regex);
+	diff_words_fill(&diff_words->plus, &plus, diff_words->word_regex);
 	xpp.flags = XDF_NEED_MINIMAL;
+	/* as only the hunk header will be parsed, we need a 0-context */
 	xecfg.ctxlen = 0;
 	xdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words,
 		      &xpp, &xecfg, &ecb);
@@ -476,6 +509,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
 		free (ecbdata->diff_words->minus.orig);
 		free (ecbdata->diff_words->plus.text.ptr);
 		free (ecbdata->diff_words->plus.orig);
+		free(ecbdata->diff_words->word_regex);
 		free(ecbdata->diff_words);
 		ecbdata->diff_words = NULL;
 	}
@@ -1498,6 +1532,14 @@ static void builtin_diff(const char *name_a,
 			ecbdata.diff_words =
 				xcalloc(1, sizeof(struct diff_words_data));
 			ecbdata.diff_words->file = o->file;
+			if (o->word_regex) {
+				ecbdata.diff_words->word_regex = (regex_t *)
+					xmalloc(sizeof(regex_t));
+				if (regcomp(ecbdata.diff_words->word_regex,
+						o->word_regex, REG_EXTENDED))
+					die ("Invalid regular expression: %s",
+							o->word_regex);
+			}
 		}
 		xdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata,
 			      &xpp, &xecfg, &ecb);
@@ -2513,6 +2555,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		DIFF_OPT_CLR(options, COLOR_DIFF);
 	else if (!strcmp(arg, "--color-words"))
 		options->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;
+	else if (!prefixcmp(arg, "--color-words=")) {
+		options->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;
+		options->word_regex = arg + 14;
+	}
 	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 4d5a327..23cd90c 100644
--- a/diff.h
+++ b/diff.h
@@ -98,6 +98,7 @@ struct diff_options {
 
 	int stat_width;
 	int stat_name_width;
+	const char *word_regex;
 
 	/* this is set by diffcore for DIFF_FORMAT_PATCH */
 	int found_changes;
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index b032bd3..0ed7e53 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -59,4 +59,28 @@ test_expect_success 'word diff with runs of whitespace' '
 
 '
 
+cat > expect <<\EOF
+<WHITE>diff --git a/pre b/post<RESET>
+<WHITE>index 330b04f..5ed8eff 100644<RESET>
+<WHITE>--- a/pre<RESET>
+<WHITE>+++ b/post<RESET>
+<BROWN>@@ -1,3 +1,7 @@<RESET>
+h(4),<GREEN>hh<RESET>[44]
+<RESET>
+a = b + c<RESET>
+
+<GREEN>aa = a<RESET>
+
+<GREEN>aeff = aeff * ( aaa )<RESET>
+EOF
+
+test_expect_success 'word diff with a regular expression' '
+
+	test_must_fail git diff --no-index --color-words='[a-z]+' \
+		pre post > output &&
+	decrypt_color < output > output.decrypted &&
+	test_cmp expect output.decrypted
+
+'
+
 test_done
-- 
1.6.1.243.g4c9c5a

^ permalink raw reply related

* Re: [PATCH next v2] git-notes: add test case for multi-line notes
From: Johannes Schindelin @ 2009-01-14 18:01 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Jeff King, Tor Arne Vestbø, Junio C Hamano, git
In-Reply-To: <496E1D77.6000307@viscovery.net>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 705 bytes --]

Hi,

On Wed, 14 Jan 2009, Johannes Sixt wrote:

> Jeff King schrieb:
> > On Wed, Jan 14, 2009 at 05:28:11PM +0100, Tor Arne Vestbø wrote:
> > 
> >> +MSG=${MSG//%/}
> >> +printf "$MSG" > "$1"
> >> +printf "$MSG" >& 2
> > 
> > Substitution parameter expansion is a bash-ism, IIRC. How about just
> > 
> >   printf %s "$MSG" ?
> 
> A the point was that $MSG contains \n, which should be turned int LF. IMO,
> the easiest way to achieve this is:
> 
> MSG='b3
> c3c3c3c3
> d3d3d3'
> 
> test_expect_success ' ... ' '
>    ...
>    MSG="$MSG" git notes edit
> '
> 
> and go back to using echo in the part cited above.

Heh, I almost suggested it, but I know that I get quoting wrong all the 
time.

Ciao,
Dscho

^ permalink raw reply

* Re: git-svn fails to fetch repository
From: Jay Soffian @ 2009-01-14 18:06 UTC (permalink / raw)
  To: Vladimir Pouzanov; +Cc: git, Eric Wong
In-Reply-To: <loom.20090114T083207-942@post.gmane.org>

On Wed, Jan 14, 2009 at 3:32 AM, Vladimir Pouzanov <farcaller@gmail.com> wrote:
> Jay Soffian <jaysoffian <at> gmail.com> writes:
>> So you're adding the "use Carp..." and "warn..." lines.
>>
>> Then try the import again. That should at least show why the svn_delta
>> temp file is being acquired twice.
>
> Output is pretty long so I've put in on pastebin:
> http://pastebin.com/m210be905

Okay, this is beyond me. git-svn (among other things) implements a
so-called delta editor (part of the subversion API). The driver for
that editor is apparently calling the editor's apply_textdelta()
method twice in a row w/o an intervening call to the editor's
close_file() method.

I don't understand when and/or why it would do this. This part of the
Subversion API seems not that well documented, and I got lost trying
to follow all the indirections in the Subversion source code (esp
w/the swig'ified Perl bindings). AFAICT, this should not be happening.

I could ask you to insert some more debugging statements to try to
track it to a specific file (or files), but I think at this point I'll
going to wait to see if the git-svn author has any ideas.

BTW, it doesn't help any that the order that files are checked out
seems not to be consistent. Not only is my git-svn clone working, the
order my files are checked out in is different from yours.

Oh, one other thing I don't understand is why the debugging output is
now showing that some files are being added for you (the lines
beginning with \tA). Before you weren't getting that. I had thought
these lines might be getting lost in stdout buffering, but git-svn
disables buffering on stdout, so color me confused.

Sorry I can't be more help,

j.

^ permalink raw reply

* Re: [PATCH 3/4] color-words: change algorithm to allow for 0-character word boundaries
From: Johannes Schindelin @ 2009-01-14 18:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Thomas Rast, git, Santi Béjar
In-Reply-To: <alpine.DEB.1.00.0901141851030.3586@pacific.mpi-cbg.de>

Hi,

On Wed, 14 Jan 2009, Johannes Schindelin wrote:

> +test_expect_success setup '
> +
> +	git config diff.color.old red
> +	git config diff.color.new green
> +
> +'

Oops.  This should probably go...

Ciao,
Dscho

^ permalink raw reply

* Compiler requirements for git?
From: Corey Stup @ 2009-01-14 18:32 UTC (permalink / raw)
  To: git

Newbie to the group.
I couldn't find a reference to say what the requirements are to
compile git.  Certainly it works with GCC, but GCC allows for all
sorts of non standard syntax.

When trying to compile with a C89 compliant compiler, I'm coming
across a couple issues:
- "inline" use
- trailing comma on the last member of enums

Are these oversights or does git require GCC or a C99 compiler?
Thanks!

^ permalink raw reply

* Re: git merge and cherry-pick and duplicated commits?
From: skillzero @ 2009-01-14 18:33 UTC (permalink / raw)
  To: git
In-Reply-To: <496DA3B2.1070807@viscovery.net>

On Wed, Jan 14, 2009 at 12:34 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:

> After you created the fixup, you have this situation:
>
>    o--o--o   <- A (feature branch)
>   /
> --o--x        <- X (the fix-up branch)
>   \
>    o--o--o   <- Z (probably your master)
>
> You merge the fix-up into the feature branch and continue developing the
> feature:
>
>    o--o--o--M--o--o   <- A
>   /        /
> --o--x-----'           <- X
>   \
>    o--o--o            <- Z
>
> Other people need the fix in Z right now, so you merge it into Z as well:
>
>    o--o--o--M--o--o   <- A
>   /        /
> --o--x-----<           <- X
>   \        \
>    o--o--o--N         <- Z
>
> You complete your feature and merge it into Z:
>
>    o--o--o--M--o--o     <- A
>   /        /       \
> --o--x-----<         \   <- X
>   \        \         \
>    o--o--o--N---------O <- Z
>
> The fix-up commit is only once in your history.

Thanks for the info. That's what I was hoping, but I was thinking that
I'd get duplicate commits if I did that. I'll have to try it out when
I run into this situation again.

Related to this, is there a way to easily find the common merge base
given a bunch of a branches? When I want to fix a bug, I want to say
"Given branches A, B, C, D, and E, where should I fork my bug fix
branch from so that I can merge this branch into all those branches
without getting duplicate commits?".

^ permalink raw reply

* Re: [PATCH 3/3] implement pattern matching in ce_path_match
From: Junio C Hamano @ 2009-01-14 18:39 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: git, johannes
In-Reply-To: <1231944876-29930-4-git-send-email-drizzd@aon.at>

Clemens Buchacher <drizzd@aon.at> writes:

> With this patch ce_path_match uses match_pathspec in order to perform
> pattern matching.

We have two conflicting definitions of pattern matching in our system.
I'd make it more explicit which kind of pattern matching you are talking
about here.

The family of operations based on the diff-tree machinery (e.g. path
limited revision walking "git log A..B -- dir1/dir2") define the pattern
matching as "leading path match (exact match is just a special case of
this)".  Other operations that work on paths in the work tree and the
index (e.g. grep, ls-files) uses "leading path match, but fall back to
globbing".

In the longer term we really should unify them by teaching the former to
fall back to globbing without getting undue performance hit, and this
patch may be a step in the right direction.  There are optimizations that
assume the "leading path" semantics to trim the input early and avoid
opening and descending into a tree object if pathspec patterns cannot
possibly match (see tree-diff.c::tree_entry_interesting() for an example),
and we need to teach them to notice a glob wildcard in an earlier part of
a pathspec and to descend into some trees that they would have skipped
with the old definition of pathspec.

> @@ -49,7 +60,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
>  		rev->always_show_header = 0;
>  	if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
>  		rev->always_show_header = 0;
> -		if (rev->diffopt.nr_paths != 1)
> +		if (rev->diffopt.nr_paths != 1 || has_special(rev->diffopt.paths[0]))
>  			usage("git logs can only follow renames on one pathname at a time");
>  	}

The reason match_pathspec() first tries exact match and then falls back to
globbing is so that the user can say "I have a file whose name ends with a
question mark, please match it literally."  This patch defeats it, but it
probably is a minor point.

1/3 and 2/3 in the series looked good.

Thanks.

^ permalink raw reply

* Re: [PATCH take 3 0/4] color-words improvements
From: Teemu Likonen @ 2009-01-14 18:54 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Thomas Rast, git, Santi Béjar
In-Reply-To: <alpine.DEB.1.00.0901141840100.3586@pacific.mpi-cbg.de>

Johannes Schindelin (2009-01-14 18:49 +0100) wrote:

> Can anybody think of undesired behavior as a consequence?
>
> Johannes Schindelin (4):
>   Add color_fwrite_lines(), a function coloring each line individually
>   color-words: refactor word splitting and use ALLOC_GROW()
>   color-words: change algorithm to allow for 0-character word
>     boundaries
>   color-words: take an optional regular expression describing words

There is something I don't understand. Maybe it's a bug or maybe it's my
limitation. I'd appreciate if you care to explain the reason of the
following output. Suppose we have two files and the line diff looks like
this:

    --- 1/a
    +++ 2/b
    @@ -1 +1 @@
    -aaa (aaa)
    +aaa (aaa) aaa

With --color-diff=a+ it looks like 

    aaa (aaa)aaa) aaa
         ^^^^~~~~ ~~~

^ = red, ~ = green

Why show changes in the "aaa)" part when it didn't actually change?

^ permalink raw reply

* Re: [PATCH take 3 0/4] color-words improvements
From: Teemu Likonen @ 2009-01-14 18:57 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <87ljtdk9b3.fsf@iki.fi>

Teemu Likonen (2009-01-14 20:54 +0200) wrote:

> With --color-diff=a+ it looks like 

Obviously I meant --color-words=a+

^ permalink raw reply

* Re: [BUG] assertion failure in builtin-mv.c with "git mv -k"
From: Junio C Hamano @ 2009-01-14 19:02 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Johannes Schindelin, Matthieu Moy, git
In-Reply-To: <496E0D1C.20807@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> So, should I prepare a series like:
>
> 1: test case and mark known fail
> 2: the 1 line fix
> 3: mark test pass
>
> Or should 2+3 be squashed into one?

If "git mv" already has its own sets of tests with a good coverage, please
strive to add a case that covers your fix to an existing script.  Then
step #1 above would be a patch to add a few "test_expect_failure" tests to
an existing file, and step #3 would be a patch to flip expect_failure to
expect_success.

And in such a case, for a single liner, all three can be squashed in to a
single patch.  It would show what changed in the code and have a few new
test_expect_success tests added to the test suite, and it would be obvious
to anybody who looks at such a change 6 months down the road that the test
cases added by the patch are the cases that did not work without the
changes to the code.  It never makes sense to separate steps #2 and #3 for
any fix.

If "git mv" did not have adequate test coverage, then please add a test
script with both expect_success (for cases that should have been there
when somebody worked on "git mv" originally), and expect_failure to expose
the bug you found in your first patch.  Again, the second patch would
update the code and flip expect_failure to expect_success.

I see there is t7001-mv and even though it claims to concentrate on its
operation from subdirectory, it has tests for more basic modes of the
operation.

So my recommendation would be to have a single patch that:

 (1) retitles t7001;
 (2) adds your new -k tests to it; and
 (3) adds the 1-liner fix.

^ permalink raw reply

* Re: [PATCH 3/3] implement pattern matching in ce_path_match
From: Clemens Buchacher @ 2009-01-14 19:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, johannes
In-Reply-To: <7vljtd20m6.fsf@gitster.siamese.dyndns.org>

On Wed, Jan 14, 2009 at 10:39:29AM -0800, Junio C Hamano wrote:
> Clemens Buchacher <drizzd@aon.at> writes:
> 
> > With this patch ce_path_match uses match_pathspec in order to perform
> > pattern matching.
> 
> We have two conflicting definitions of pattern matching in our system.
> I'd make it more explicit which kind of pattern matching you are talking
> about here.

Right, will fix.

> In the longer term we really should unify them by teaching the former to
> fall back to globbing without getting undue performance hit, and this
> patch may be a step in the right direction.  There are optimizations that
> assume the "leading path" semantics to trim the input early and avoid
> opening and descending into a tree object if pathspec patterns cannot
> possibly match (see tree-diff.c::tree_entry_interesting() for an example),
> and we need to teach them to notice a glob wildcard in an earlier part of
> a pathspec and to descend into some trees that they would have skipped
> with the old definition of pathspec.

I see. I can probably fix that this weekend.

> > @@ -49,7 +60,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
> >  		rev->always_show_header = 0;
> >  	if (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {
> >  		rev->always_show_header = 0;
> > -		if (rev->diffopt.nr_paths != 1)
> > +		if (rev->diffopt.nr_paths != 1 || has_special(rev->diffopt.paths[0]))
> >  			usage("git logs can only follow renames on one pathname at a time");
> >  	}
> 
> The reason match_pathspec() first tries exact match and then falls back to
> globbing is so that the user can say "I have a file whose name ends with a
> question mark, please match it literally."  This patch defeats it, but it
> probably is a minor point.

I was wondering actually if we should disallow such paths altogether,
since there would be no way to match only 'a?', if something like 'ab' also
exists. So if you added 'a?' by accident, you cannot even remove it without
also removing 'ab'.

I think we could at least add an option to disable globbing. Then we can
also disable the above check conditioned on that. If we allowed globbing
pattern for following renames wouldn't that result in following the first
file (or last in history) to match the pattern, which is potentially
confusing?

^ permalink raw reply

* Re: [2/3] git-daemon: use getnameinfo to resolve hostname
From: Jan Engelhardt @ 2009-01-14 19:25 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20090114122536.GA5939@coredump.intra.peff.net>


On Wednesday 2009-01-14 13:25, Jeff King wrote:
>On Wed, Jan 14, 2009 at 11:48:38AM +0100, Jan Engelhardt wrote:
>
>> This is much shorter than inet_ntop'ing, and also translated
>> unresolvable addresses into a string.
>
>Er, doesn't this totally change the meaning of REMOTE_ADDR from an IP
>address to a hostname? That seems bad because:
>[...]
>  - people already have hooks that compare REMOTE_ADDR against an
>    address, so we are breaking their hooks
>[...]
>So at the very least, you should be adding REMOTE_HOST in _addition_ to
>REMOTE_ADDR, not instead of.

Good catch. It's always good to have someone else look through it.
Changed, and below is the proposition as a non-mergable diff.

In case getnameinfo fails, the IP address from inet_ntop
should be left in addrbuf, is not it?

And yeah, it does not have a flag to disable DNS resolution, but
it's a draft for now.

---8<---
git-daemon: resolve source host's DNS

Try to resolve DNS addresses so that run_service() can print the
name of the host from which the request originated.
[addrbuf is passed to run_service as a result of patch 1/3]

---
 daemon.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: git-1.6.1/daemon.c
===================================================================
--- git-1.6.1.orig/daemon.c
+++ git-1.6.1/daemon.c
@@ -530,6 +530,10 @@ static int execute(struct sockaddr *addr
 #endif
 		}
 		setenv("REMOTE_ADDR", addrbuf, 1);
+		getnameinfo(addr, (addr->sa_family == AF_INET6) ?
+			sizeof(struct sockaddr_in6) :
+			sizeof(struct sockaddr_in),
+			addrbuf, sizeof(addrbuf), NULL, 0, 0);
 	}
 	else {
 		unsetenv("REMOTE_ADDR");

^ permalink raw reply

* Re: [PATCH take 3 0/4] color-words improvements
From: Johannes Schindelin @ 2009-01-14 19:28 UTC (permalink / raw)
  To: Teemu Likonen; +Cc: git
In-Reply-To: <87d4epk96e.fsf@iki.fi>

Hi,

On Wed, 14 Jan 2009, Teemu Likonen wrote:

> Teemu Likonen (2009-01-14 20:54 +0200) wrote:
> 
> > With --color-diff=a+ it looks like 
> 
> Obviously I meant --color-words=a+

Heh,  I missed that, even...  Thanks for the report!

-- snipsnap --
[WILL BE SQUASHED INTO 4/4] Fix find_word_boundaries()

Since newlines cannot be part of words, we have to stop at newlines even
if the regular expression's match contains one.

Of course, I fscked up the range where to look for the newline when I
changed the function from find_word_boundary().
---
 diff.c                |    2 +-
 t/t4034-diff-words.sh |   20 ++++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/diff.c b/diff.c
index d5d7171..1408717 100644
--- a/diff.c
+++ b/diff.c
@@ -384,7 +384,7 @@ static int find_word_boundaries(mmfile_t *buffer, regex_t *word_regex,
 		regmatch_t match[1];
 		if (!regexec(word_regex, buffer->ptr + *begin, 1, match, 0)) {
 			char *p = memchr(buffer->ptr + *begin + match[0].rm_so,
-					'\n', match[0].rm_eo);
+					'\n', match[0].rm_eo - match[0].rm_so);
 			*end = p ? p - buffer->ptr : match[0].rm_eo + *begin;
 			*begin += match[0].rm_so;
 			return *begin >= *end;
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 0ed7e53..1137131 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -83,4 +83,24 @@ test_expect_success 'word diff with a regular expression' '
 
 '
 
+echo 'aaa (aaa)' > pre
+echo 'aaa (aaa) aaa' > post
+
+cat > expect <<\EOF
+<WHITE>diff --git a/pre b/post<RESET>
+<WHITE>index c29453b..be22f37 100644<RESET>
+<WHITE>--- a/pre<RESET>
+<WHITE>+++ b/post<RESET>
+<BROWN>@@ -1 +1 @@<RESET>
+aaa (aaa)<GREEN> aaa<RESET>
+EOF
+
+test_expect_success "Teemo's example" '
+
+	test_must_fail git diff --no-index --color-words='a+' pre post > output &&
+	decrypt_color < output > output.decrypted &&
+	test_cmp expect output.decrypted
+
+'
+
 test_done
-- 
1.6.1.295.gb16478

^ permalink raw reply related

* Re: [PATCH take 3 0/4] color-words improvements
From: Johannes Schindelin @ 2009-01-14 19:32 UTC (permalink / raw)
  To: Teemu Likonen; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0901142028010.3586@pacific.mpi-cbg.de>

Hi,

On Wed, 14 Jan 2009, Johannes Schindelin wrote:

> +aaa (aaa)<GREEN> aaa<RESET>

Of course, the space must be on the other side of the <GREEN>...  All this 
will be fixed, and more.

Ciao,
Dscho

^ permalink raw reply

* Re: git merge and cherry-pick and duplicated commits?
From: Peter Baumann @ 2009-01-14 19:40 UTC (permalink / raw)
  To: skillzero; +Cc: git
In-Reply-To: <2729632a0901141033p47b4d8dah46f5bac27307d306@mail.gmail.com>

On Wed, Jan 14, 2009 at 10:33:02AM -0800, skillzero@gmail.com wrote:
> On Wed, Jan 14, 2009 at 12:34 AM, Johannes Sixt <j.sixt@viscovery.net>
> wrote:
> 
[ ... ]
  
> Related to this, is there a way to easily find the common merge base
> given a bunch of a branches? When I want to fix a bug, I want to say
> "Given branches A, B, C, D, and E, where should I fork my bug fix
> branch from so that I can merge this branch into all those branches
> without getting duplicate commits?".

You should have a look at 'git help merge-base'

-Peter

^ permalink raw reply

* [PATCH] color-words: make regex configurable via attributes
From: Thomas Rast @ 2009-01-14 19:46 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Santi Béjar, Junio C Hamano
In-Reply-To: <alpine.DEB.1.00.0901141840100.3586@pacific.mpi-cbg.de>

Make the --color-words splitting regular expression configurable via
the diff driver's 'wordregex' attribute.  The user can then set the
driver on a file in .gitattributes.  If a regex is given on the
command line, it overrides the driver's setting.

We also provide built-in regexes for the languages that already had
funcname patterns, and add an appropriate diff driver entry for C/++.
(The patterns are designed to run UTF-8 sequences into a single chunk
to make sure they remain readable.)

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

This is the old 3/4 combined with a test similar to the one it had in
the old 4/4, built on top of Dscho's take 3.  I researched the
operators for each language, but the identifier and number formats may
be off in some cases.


 Documentation/diff-options.txt  |    3 +
 Documentation/gitattributes.txt |   21 ++++++++++
 diff.c                          |   10 +++++
 t/t4034-diff-words.sh           |   40 ++++++++++++++++++++
 userdiff.c                      |   78 +++++++++++++++++++++++++++++++-------
 userdiff.h                      |    1 +
 6 files changed, 138 insertions(+), 15 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 2c1fa4b..ef0e2f5 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -97,6 +97,9 @@ endif::git-format-patch[]
 Optionally, you can pass a regular expression that tells Git what the
 words are that you are looking for; The default is to interpret any
 stretch of non-whitespace as a word.
+The regex can also be set via a diff driver, see
+linkgit:gitattributes[1]; giving it explicitly overrides any diff
+driver setting.
 
 --no-renames::
 	Turn off rename detection, even when the configuration
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 8af22ec..17707ba 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -317,6 +317,8 @@ patterns are available:
 
 - `bibtex` suitable for files with BibTeX coded references.
 
+- `cpp` suitable for source code in the C and C++ languages.
+
 - `html` suitable for HTML/XHTML documents.
 
 - `java` suitable for source code in the Java language.
@@ -334,6 +336,25 @@ patterns are available:
 - `tex` suitable for source code for LaTeX documents.
 
 
+Customizing word diff
+^^^^^^^^^^^^^^^^^^^^^
+
+You can customize the rules that `git diff --color-words` uses to
+split words in a line, by specifying an appropriate regular expression
+in the "diff.*.wordregex" configuration variable.  For example, in TeX
+a backslash followed by a sequence of letters forms a command, but
+several such commands can be run together without intervening
+whitespace.  To separate them, use a regular expression such as
+
+------------------------
+[diff "tex"]
+	wordregex = "\\\\[a-zA-Z]+|[{}]|\\\\.|[^\\{}[:space:]]+"
+------------------------
+
+A built-in pattern is provided for all languages listed in the last
+section.
+
+
 Performing text diffs of binary files
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/diff.c b/diff.c
index eb67431..08bdc86 100644
--- a/diff.c
+++ b/diff.c
@@ -1372,6 +1372,12 @@ int diff_filespec_is_binary(struct diff_filespec *one)
 	return one->driver->funcname.pattern ? &one->driver->funcname : NULL;
 }
 
+static const char *userdiff_word_regex(struct diff_filespec *one)
+{
+	diff_filespec_load_driver(one);
+	return one->driver->word_regex;
+}
+
 void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b)
 {
 	if (!options->a_prefix)
@@ -1532,6 +1538,10 @@ static void builtin_diff(const char *name_a,
 			ecbdata.diff_words =
 				xcalloc(1, sizeof(struct diff_words_data));
 			ecbdata.diff_words->file = o->file;
+			if (!o->word_regex)
+				o->word_regex = userdiff_word_regex(one);
+			if (!o->word_regex)
+				o->word_regex = userdiff_word_regex(two);
 			if (o->word_regex) {
 				ecbdata.diff_words->word_regex = (regex_t *)
 					xmalloc(sizeof(regex_t));
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 0ed7e53..d6731d1 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -83,4 +83,44 @@ test_expect_success 'word diff with a regular expression' '
 
 '
 
+cat > expect-by-chars <<\EOF
+<WHITE>diff --git a/pre b/post<RESET>
+<WHITE>index 330b04f..5ed8eff 100644<RESET>
+<WHITE>--- a/pre<RESET>
+<WHITE>+++ b/post<RESET>
+<BROWN>@@ -1,3 +1,7 @@<RESET>
+<RED>h(4)<RESET><GREEN>h(4),hh[44]<RESET>
+<RESET>
+a = b + c<RESET>
+
+<GREEN>aa = a<RESET>
+
+<GREEN>aeff = aeff * ( aaa )<RESET>
+EOF
+
+test_expect_success 'set a diff driver' '
+	git config diff.testdriver.wordregex "[^[:space:]]" &&
+	cat <<EOF > .gitattributes
+test_* diff=testdriver
+EOF
+'
+
+test_expect_success 'use default supplied by driver' '
+
+	test_must_fail git diff --no-index --color-words \
+		pre post > output &&
+	decrypt_color < output > output.decrypted &&
+	test_cmp expect-by-chars output.decrypted
+
+'
+
+test_expect_success 'option overrides default' '
+
+	test_must_fail git diff --no-index --color-words="[a-z]+" \
+		pre post > output &&
+	decrypt_color < output > output.decrypted &&
+	test_cmp expect output.decrypted
+
+'
+
 test_done
diff --git a/userdiff.c b/userdiff.c
index 3681062..79f9cb9 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -6,14 +6,20 @@
 static int ndrivers;
 static int drivers_alloc;
 
-#define FUNCNAME(name, pattern) \
-	{ name, NULL, -1, { pattern, REG_EXTENDED } }
+#define PATTERNS(name, pattern, wordregex)			\
+	{ name, NULL, -1, { pattern, REG_EXTENDED }, NULL, wordregex }
 static struct userdiff_driver builtin_drivers[] = {
-FUNCNAME("html", "^[ \t]*(<[Hh][1-6][ \t].*>.*)$"),
-FUNCNAME("java",
+PATTERNS("html", "^[ \t]*(<[Hh][1-6][ \t].*>.*)$",
+	 "[^<>= \t]+|[^[:space:]]|[\x80-\xff]+"),
+PATTERNS("java",
 	 "!^[ \t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\n"
-	 "^[ \t]*(([ \t]*[A-Za-z_][A-Za-z_0-9]*){2,}[ \t]*\\([^;]*)$"),
-FUNCNAME("objc",
+	 "^[ \t]*(([ \t]*[A-Za-z_][A-Za-z_0-9]*){2,}[ \t]*\\([^;]*)$",
+	 "[a-zA-Z_][a-zA-Z0-9_]*"
+	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
+	 "|[-+*/<>%&^|=!]="
+	 "|--|\\+\\+|<<=?|>>>?=?|&&|\\|\\|"
+	 "|[^[:space:]]|[\x80-\xff]+"),
+PATTERNS("objc",
 	 /* Negate C statements that can look like functions */
 	 "!^[ \t]*(do|for|if|else|return|switch|while)\n"
 	 /* Objective-C methods */
@@ -21,20 +27,60 @@
 	 /* C functions */
 	 "^[ \t]*(([ \t]*[A-Za-z_][A-Za-z_0-9]*){2,}[ \t]*\\([^;]*)$\n"
 	 /* Objective-C class/protocol definitions */
-	 "^(@(implementation|interface|protocol)[ \t].*)$"),
-FUNCNAME("pascal",
+	 "^(@(implementation|interface|protocol)[ \t].*)$",
+	 /* -- */
+	 "[a-zA-Z_][a-zA-Z0-9_]*"
+	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
+	 "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"
+	 "|[^[:space:]]|[\x80-\xff]+"),
+PATTERNS("pascal",
 	 "^((procedure|function|constructor|destructor|interface|"
 		"implementation|initialization|finalization)[ \t]*.*)$"
 	 "\n"
-	 "^(.*=[ \t]*(class|record).*)$"),
-FUNCNAME("php", "^[\t ]*((function|class).*)"),
-FUNCNAME("python", "^[ \t]*((class|def)[ \t].*)$"),
-FUNCNAME("ruby", "^[ \t]*((class|module|def)[ \t].*)$"),
-FUNCNAME("bibtex", "(@[a-zA-Z]{1,}[ \t]*\\{{0,1}[ \t]*[^ \t\"@',\\#}{~%]*).*$"),
-FUNCNAME("tex", "^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$"),
+	 "^(.*=[ \t]*(class|record).*)$",
+	 /* -- */
+	 "[a-zA-Z_][a-zA-Z0-9_]*"
+	 "|[-+0-9.e]+|0[xXbB]?[0-9a-fA-F]+"
+	 "|<>|<=|>=|:=|\\.\\."
+	 "|[^[:space:]]|[\x80-\xff]+"),
+PATTERNS("php", "^[\t ]*((function|class).*)",
+	 /* -- */
+	 "[a-zA-Z_][a-zA-Z0-9_]*"
+	 "|[-+0-9.e]+|0[xXbB]?[0-9a-fA-F]+"
+	 "|[-+*/<>%&^|=!.]=|--|\\+\\+|<<=?|>>=?|===|&&|\\|\\||::|->"
+	 "|[^[:space:]]|[\x80-\xff]+"),
+PATTERNS("python", "^[ \t]*((class|def)[ \t].*)$",
+	 /* -- */
+	 "[a-zA-Z_][a-zA-Z0-9_]*"
+	 "|[-+0-9.e]+[jJlL]?|0[xX]?[0-9a-fA-F]+[lL]?"
+	 "|[-+*/<>%&^|=!]=|//=?|<<=?|>>=?|\\*\\*=?"
+	 "|[^[:space:]|[\x80-\xff]+"),
+	 /* -- */
+PATTERNS("ruby", "^[ \t]*((class|module|def)[ \t].*)$",
+	 /* -- */
+	 "(@|@@|\\$)?[a-zA-Z_][a-zA-Z0-9_]*"
+	 "|[-+0-9.e]+|0[xXbB]?[0-9a-fA-F]+|\\?(\\\\C-)?(\\\\M-)?."
+	 "|//=?|[-+*/<>%&^|=!]=|<<=?|>>=?|===|\\.{1,3}|::|[!=]~"
+	 "|[^[:space:]|[\x80-\xff]+"),
+PATTERNS("bibtex", "(@[a-zA-Z]{1,}[ \t]*\\{{0,1}[ \t]*[^ \t\"@',\\#}{~%]*).*$",
+	 "[={}\"]|[^={}\" \t]+"),
+PATTERNS("tex", "^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$",
+	 "\\\\[a-zA-Z@]+|[{}]|\\\\.|[^\\{} \t]+"),
+PATTERNS("cpp",
+	 /* Jump targets or access declarations */
+	 "!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:.*$\n"
+	 /* C functions at top level */
+	 "^([A-Za-z_][A-Za-z_0-9]*([ \t]+[A-Za-z_][A-Za-z_0-9]*){1,}[ \t]*\\([^;]*)$\n"
+	 /* compound type at top level */
+	 "^((struct|class|enum)[^;]*)$",
+	 /* -- */
+	 "[a-zA-Z_][a-zA-Z0-9_]*"
+	 "|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
+	 "|[-+*/<>%&^|=!]=|--|\\+\\+|<<=?|>>=?|&&|\\|\\||::|->"
+	 "|[^[:space:]]|[\x80-\xff]+"),
 { "default", NULL, -1, { NULL, 0 } },
 };
-#undef FUNCNAME
+#undef PATTERNS
 
 static struct userdiff_driver driver_true = {
 	"diff=true",
@@ -134,6 +180,8 @@ int userdiff_config(const char *k, const char *v)
 		return parse_string(&drv->external, k, v);
 	if ((drv = parse_driver(k, v, "textconv")))
 		return parse_string(&drv->textconv, k, v);
+	if ((drv = parse_driver(k, v, "wordregex")))
+		return parse_string(&drv->word_regex, k, v);
 
 	return 0;
 }
diff --git a/userdiff.h b/userdiff.h
index ba29457..2aab13e 100644
--- a/userdiff.h
+++ b/userdiff.h
@@ -12,6 +12,7 @@ struct userdiff_driver {
 	int binary;
 	struct userdiff_funcname funcname;
 	const char *textconv;
+	const char *word_regex;
 };
 
 int userdiff_config(const char *k, const char *v);
-- 
1.6.1.140.ge720e.dirty

^ permalink raw reply related

* Re: [PATCH 4/4] color-words: take an optional regular expression describing words
From: Thomas Rast @ 2009-01-14 19:55 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git, Santi Béjar
In-Reply-To: <alpine.DEB.1.00.0901141851350.3586@pacific.mpi-cbg.de>

[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]

Johannes Schindelin wrote:
> ---color-words::
> +--color-words[=regex]::
>  	Show colored word diff, i.e. color words which have changed.
> ++
> +Optionally, you can pass a regular expression that tells Git what the
> +words are that you are looking for; The default is to interpret any
> +stretch of non-whitespace as a word.

Perhaps you could resurrect the documentation from my series, adjusted
for the different newline rule:


--color-words[=<regex>]::
	Show colored word diff, i.e., color words which have changed.
	By default, a new word only starts at whitespace, so that a
	'word' is defined as a maximal sequence of non-whitespace
	characters.  The optional argument <regex> can be used to
	configure this.  It can also be set via a diff driver, see
	linkgit:gitattributes[1]; if a <regex> is given explicitly, it
	overrides any diff driver setting.
+
The <regex> must be an (extended) regular expression.  When set, every
non-overlapping match of the <regex> is considered a word.  Anything
between these matches is considered whitespace and ignored for the
purposes of finding differences.  You may want to append
`|[^[:space:]]` to your regular expression to make sure that it
matches all non-whitespace characters.  A match that contains a
newline is silently truncated at the newline.


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


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH take 3 0/4] color-words improvements
From: Thomas Rast @ 2009-01-14 19:58 UTC (permalink / raw)
  To: Teemu Likonen; +Cc: Johannes Schindelin, Junio C Hamano, git, Santi Béjar
In-Reply-To: <87ljtdk9b3.fsf@iki.fi>

[-- Attachment #1: Type: text/plain, Size: 518 bytes --]

Teemu Likonen wrote:
>     -aaa (aaa)
>     +aaa (aaa) aaa

Bug aside, examples like this one make me wonder if we should force a
"last resort" match for `[^[:space:]]`.  For example,

      -aaa [aaa]
      +aaa (aaa) aaa

would still give you

      aaa (aaa)<GREEN> aaa<RESET>

which may be unexpected.

Of course, when diffing a language where something other than the
"usual" whitespace should be ignored, this behaviour would be useful.


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




[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* [PATCH 2/3] bash-completion: Try bash completions before simple filetype
From: ted @ 2009-01-14 20:02 UTC (permalink / raw)
  To: spearce; +Cc: git, gitster, Ted Pavlic
In-Reply-To: <1231963342-24461-1-git-send-email-ted@tedpavlic.com>

From: Ted Pavlic <ted@tedpavlic.com>

  When a git completion is not found, a bash shell should try bash-type
  completions first before going to standard filetype completions. This
  patch /adds/ "-o bashdefault" to the completion line. If that option
  is not available, it uses the old method.

  This behavior was inspired by Mercurial's bash completion script.

Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
---
 contrib/completion/git-completion.bash |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 5d1515c..201f9a6 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1766,13 +1766,16 @@ _gitk ()
 	__git_complete_revlist
 }
 
-complete -o default -o nospace -F _git git
-complete -o default -o nospace -F _gitk gitk
+complete -o bashdefault -o default -o nospace -F _git git 2>/dev/null \
+	|| complete -o default -o nospace -F _git git
+complete -o bashdefault -o default -o nospace -F _gitk gitk 2>/dev/null \
+	|| complete -o default -o nospace -F _gitk gitk
 
 # The following are necessary only for Cygwin, and only are needed
 # when the user has tab-completed the executable name and consequently
 # included the '.exe' suffix.
 #
 if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
-complete -o default -o nospace -F _git git.exe
+complete -o bashdefault -o default -o nospace -F _git git.exe 2>/dev/null \
+	|| complete -o default -o nospace -F _git git.exe
 fi
-- 
1.6.1.87.g15624

^ permalink raw reply related

* [PATCH 3/3] bash-completion: Added comments to remind about required arguments
From: ted @ 2009-01-14 20:02 UTC (permalink / raw)
  To: spearce; +Cc: git, gitster, Ted Pavlic
In-Reply-To: <1231963342-24461-2-git-send-email-ted@tedpavlic.com>

From: Ted Pavlic <ted@tedpavlic.com>

  Adds a few simple comments above commands that take arguments. These
  comments are meant to remind developers of potential problems that can
  occur when the script is sourced on systems with "set -u." Any
  function which "requires" arguments really ought to be called with
  explicit arguments given.

  Also adds a #!bash to the top of bash completions so that editing
  software can always identify that the file is of sh type.

Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
---
 contrib/completion/git-completion.bash |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 201f9a6..f8b845a 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1,3 +1,4 @@
+#!bash
 #
 # bash completion support for core Git.
 #
@@ -50,6 +51,8 @@ case "$COMP_WORDBREAKS" in
 *)   COMP_WORDBREAKS="$COMP_WORDBREAKS:"
 esac
 
+# __gitdir accepts 0 or 1 arguments (i.e., location)
+# returns location of .git repo
 __gitdir ()
 {
 	if [ -z "${1-}" ]; then
@@ -67,6 +70,8 @@ __gitdir ()
 	fi
 }
 
+# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
+# returns text to add to bash PS1 prompt (includes branch name)
 __git_ps1 ()
 {
 	local g="$(git rev-parse --git-dir 2>/dev/null)"
@@ -119,6 +124,7 @@ __git_ps1 ()
 	fi
 }
 
+# __gitcomp_1 requires 2 arguments
 __gitcomp_1 ()
 {
 	local c IFS=' '$'\t'$'\n'
@@ -131,6 +137,8 @@ __gitcomp_1 ()
 	done
 }
 
+# __gitcomp accepts 1, 2, 3, or 4 arguments
+# generates completion reply with compgen
 __gitcomp ()
 {
 	local cur="${COMP_WORDS[COMP_CWORD]}"
@@ -150,6 +158,7 @@ __gitcomp ()
 	esac
 }
 
+# __git_heads accepts 0 or 1 arguments (to pass to __gitdir)
 __git_heads ()
 {
 	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
@@ -168,6 +177,7 @@ __git_heads ()
 	done
 }
 
+# __git_tags accepts 0 or 1 arguments (to pass to __gitdir)
 __git_tags ()
 {
 	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
@@ -186,6 +196,7 @@ __git_tags ()
 	done
 }
 
+# __git_refs accepts 0 or 1 arguments (to pass to __gitdir)
 __git_refs ()
 {
 	local i is_hash=y dir="$(__gitdir "${1-}")"
@@ -218,6 +229,7 @@ __git_refs ()
 	done
 }
 
+# __git_refs2 requires 1 argument (to pass to __git_refs)
 __git_refs2 ()
 {
 	local i
@@ -226,6 +238,7 @@ __git_refs2 ()
 	done
 }
 
+# __git_refs_remotes requires 1 argument (to pass to ls-remote)
 __git_refs_remotes ()
 {
 	local cmd i is_hash=y
@@ -470,6 +483,7 @@ __git_aliases ()
 	done
 }
 
+# __git_aliased_command requires 1 argument
 __git_aliased_command ()
 {
 	local word cmdline=$(git --git-dir="$(__gitdir)" \
@@ -482,6 +496,7 @@ __git_aliased_command ()
 	done
 }
 
+# __git_find_subcommand requires 1 argument
 __git_find_subcommand ()
 {
 	local word subcommand c=1
-- 
1.6.1.87.g15624

^ permalink raw reply related

* [PATCH 1/3] bash-completion: Support running when set -u is enabled
From: ted @ 2009-01-14 20:02 UTC (permalink / raw)
  To: spearce; +Cc: git, gitster, Ted Pavlic

From: Ted Pavlic <ted@tedpavlic.com>

  Under "set -u" semantics, it is an error to access undefined
  variables. Some user environments may enable this setting in the
  interactive shell.

  In any context where the completion functions access an undefined
  variable, accessing a default empty string (aka "${1-}" instead of
  "$1") is a reasonable way to code the function, as it silences
  the undefined variable error while still supplying an empty string.

  In this patch, functions that should always take an argument still use
  $1. Functions that have optional arguments use ${1-}.

Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
---
 contrib/completion/git-completion.bash |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7b074d7..5d1515c 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -52,7 +52,7 @@ esac
 
 __gitdir ()
 {
-	if [ -z "$1" ]; then
+	if [ -z "${1-}" ]; then
 		if [ -n "$__git_dir" ]; then
 			echo "$__git_dir"
 		elif [ -d .git ]; then
@@ -111,7 +111,7 @@ __git_ps1 ()
 			fi
 		fi
 
-		if [ -n "$1" ]; then
+		if [ -n "${1-}" ]; then
 			printf "$1" "${b##refs/heads/}$r"
 		else
 			printf " (%s)" "${b##refs/heads/}$r"
@@ -143,8 +143,8 @@ __gitcomp ()
 		;;
 	*)
 		local IFS=$'\n'
-		COMPREPLY=($(compgen -P "$2" \
-			-W "$(__gitcomp_1 "$1" "$4")" \
+		COMPREPLY=($(compgen -P "${2-}" \
+			-W "$(__gitcomp_1 "${1-}" "${4-}")" \
 			-- "$cur"))
 		;;
 	esac
@@ -152,13 +152,13 @@ __gitcomp ()
 
 __git_heads ()
 {
-	local cmd i is_hash=y dir="$(__gitdir "$1")"
+	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
 	if [ -d "$dir" ]; then
 		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
 			refs/heads
 		return
 	fi
-	for i in $(git ls-remote "$1" 2>/dev/null); do
+	for i in $(git ls-remote "${1-}" 2>/dev/null); do
 		case "$is_hash,$i" in
 		y,*) is_hash=n ;;
 		n,*^{}) is_hash=y ;;
@@ -170,13 +170,13 @@ __git_heads ()
 
 __git_tags ()
 {
-	local cmd i is_hash=y dir="$(__gitdir "$1")"
+	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
 	if [ -d "$dir" ]; then
 		git --git-dir="$dir" for-each-ref --format='%(refname:short)' \
 			refs/tags
 		return
 	fi
-	for i in $(git ls-remote "$1" 2>/dev/null); do
+	for i in $(git ls-remote "${1-}" 2>/dev/null); do
 		case "$is_hash,$i" in
 		y,*) is_hash=n ;;
 		n,*^{}) is_hash=y ;;
@@ -188,7 +188,7 @@ __git_tags ()
 
 __git_refs ()
 {
-	local i is_hash=y dir="$(__gitdir "$1")"
+	local i is_hash=y dir="$(__gitdir "${1-}")"
 	local cur="${COMP_WORDS[COMP_CWORD]}" format refs
 	if [ -d "$dir" ]; then
 		case "$cur" in
-- 
1.6.1.87.g15624

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox