Git development
 help / color / mirror / Atom feed
* Re: [PATCH take 3 0/4] color-words improvements
From: Johannes Schindelin @ 2009-01-14 21:07 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Junio C Hamano, git, Santi Béjar
In-Reply-To: <200901142104.16134.trast@student.ethz.ch>

Hi,

On Wed, 14 Jan 2009, Thomas Rast wrote:

> Johannes Schindelin wrote:
> > 
> > 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().
> 
> Umm.... but you seem to do?

Oh, right!  I think the culprit is in fn_out_diff_words_aux(), which calls 
fwrite() directly for the common words.

> Ack on the new regex semantics, though I'd have implemented it via dying 
> on '\n' instead of silently splitting there (and restarting a new 
> match!).

Hmm.  I'd rather not die() in the middle of it.

Maybe we can even handle newlines correctly by replacing them with NULs 
which libxdiff handles just fine?

> Thus, Ack on 4/4 once the boundary bug is fixed.  Thanks for your work!

Phew.  I was almost convinced you would hate me for my criticiscm.

Thanks,
Dscho

^ permalink raw reply

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

The tests adds a third commit with a multi-line note. The output of
git log -2 is then checked to see if the note lines are wrapped
correctly, and that there's a line separator between the two commits.

Signed-off-by: Tor Arne Vestbø <tavestbo@trolltech.com>
---
 t/t3301-notes.sh |   32 +++++++++++++++++++++++++++++++-
 1 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index ba42c45..9393a25 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -59,7 +59,37 @@ EOF
 test_expect_success 'show notes' '
 	! (git cat-file commit HEAD | grep b1) &&
 	git log -1 > output &&
-	git diff expect output
+	test_cmp expect output
+'
+test_expect_success 'create multi-line notes (setup)' '
+	: > a3 &&
+	git add a3 &&
+	test_tick &&
+	git commit -m 3rd &&
+	MSG="b3
+c3c3c3c3
+d3d3d3" git notes edit
+'
+
+cat > expect-multiline << EOF
+commit 1584215f1d29c65e99c6c6848626553fdd07fd75
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:15:13 2005 -0700
+
+    3rd
+
+Notes:
+    b3
+    c3c3c3c3
+    d3d3d3
+EOF
+
+printf "\n" >> expect-multiline
+cat expect >> expect-multiline
+
+test_expect_success 'show multi-line notes' '
+	git log -2 > output &&
+	test_cmp expect-multiline output
 '
 
 test_done
-- 
1.6.0.2.GIT

^ permalink raw reply related

* [PATCH replacement for take 3 4/4] color-words: take an optional regular expression describing words
From: Johannes Schindelin @ 2009-01-14 20:46 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Santi Béjar, Junio C Hamano, Teemu Likonen
In-Reply-To: <alpine.DEB.1.00.0901142142120.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>
---

	This basically contains the fix I sent earlier.

	As for the documentation, I would not have any issue with your 
	patch replacing my documentation in favor of yours.

 Documentation/diff-options.txt |    6 +++-
 diff.c                         |   64 ++++++++++++++++++++++++++++++++++-----
 diff.h                         |    1 +
 t/t4034-diff-words.sh          |   39 ++++++++++++++++++++++++
 4 files changed, 100 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..1408717 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 - match[0].rm_so);
+			*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 b22195f..f4810e9 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -63,4 +63,43 @@ 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' '
+
+	word_diff --color-words='[a-z]+'
+
+'
+
+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 "test parsing words for newline" '
+
+	word_diff --color-words='a+'
+
+'
+
 test_done
-- 
1.6.1.295.g5d331

^ permalink raw reply related

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

[-- Attachment #1: Type: TEXT/PLAIN, Size: 8924 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>
---

	I changed the test script to avoid repeating the same three-command
	mantra all the time.

 diff.c                |  153 +++++++++++++++++++++++++++----------------------
 t/t4034-diff-words.sh |   66 +++++++++++++++++++++
 2 files changed, 151 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..b22195f
--- /dev/null
+++ b/t/t4034-diff-words.sh
@@ -0,0 +1,66 @@
+#!/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'
+}
+
+word_diff () {
+	test_must_fail git diff --no-index "$@" pre post > output &&
+	decrypt_color < output > output.decrypted &&
+	test_cmp expect output.decrypted
+}
+
+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' '
+
+	word_diff --color-words
+
+'
+
+test_done
-- 
1.6.1.295.g5d331


^ permalink raw reply related

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

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

Johannes Schindelin wrote:
> How about making this an extra paragraph?

Sure, why not.  Though I'm still in favour of taking some longer
version, possibly from my old series.

> On Wed, 14 Jan 2009, Thomas Rast wrote:
> > +- `cpp` suitable for source code in the C and C++ languages.
> > +
> 
> How about "written in C or C++"?

I was just trying to be consistent with all other items; all
programming languages are listed as "Foo language".

> > +A built-in pattern is provided for all languages listed in the last
> > +section.
> 
> Wow.  But how about "previous section"?

Indeed, thanks.

> > +#define PATTERNS(name, pattern, wordregex)			\
> > +	{ name, NULL, -1, { pattern, REG_EXTENDED }, NULL, wordregex }
> 
> You could get rid of that NULL if...
[...]
> ... you inserted word_regex before textconv.  In a way, I find this more 
> logical, since both funcname and word_regex have sensible defaults 
> (provided by you), whereas textconv is strictly a user's option.

Ok, I'll do that.

-- 
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: git merge and cherry-pick and duplicated commits?
From: Junio C Hamano @ 2009-01-14 20:16 UTC (permalink / raw)
  To: skillzero; +Cc: git
In-Reply-To: <2729632a0901141033p47b4d8dah46f5bac27307d306@mail.gmail.com>

skillzero@gmail.com writes:

> 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 do not necessarily have to fork from, nor merge into, any of them.

If you fixed a bug, you would hopefully know where the bug was injected at
into your history.  You may have bisected it down to one commit $BAD.  You
can fork your fix on top of that $BAD commit:

	$ git checkout -b fix-bug-foo $BAD

All of the branches that share the commit have the bug, so your fix could
be merged to all of them if you really wanted to, and you should do so if
these A...E branches are meant to be consumed on their own.

But if the branches A...E you are about are for developing independent
topics, and if their theme won't get affected by the bug, it is much
better not to merge the fix in.  You will have the merge for the fix in
your integration branch anyway.  It is preferable not to contaminate an
independent topic branch whose purpose is to cook its own theme with an
unrelated bugfix, even if it is brought in as a merge.

^ permalink raw reply

* Re: [PATCH] color-words: make regex configurable via attributes
From: Johannes Schindelin @ 2009-01-14 20:12 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Santi Béjar, Junio C Hamano
In-Reply-To: <1231962401-26974-1-git-send-email-trast@student.ethz.ch>

Hi,

On Wed, 14 Jan 2009, Thomas Rast wrote:

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

How about making this an extra paragraph?

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

How about "written in C or C++"?

> +A built-in pattern is provided for all languages listed in the last
> +section.

Wow.  But how about "previous section"?

> 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

That was fast!

> +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
> +
> +'

I am actually just about to post new revisions of the last two patches 
where this would read

	test_expect_success 'use default supplied by driver' '

		word_diff --color-words

	'

instead...

I don't want to get bitten by stupid mistakes again, though, so I let it 
run with valgrind while glancing over the code.  Stay tuned.

> +#define PATTERNS(name, pattern, wordregex)			\
> +	{ name, NULL, -1, { pattern, REG_EXTENDED }, NULL, wordregex }

You could get rid of that NULL if...

> 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;
>  };

... you inserted word_regex before textconv.  In a way, I find this more 
logical, since both funcname and word_regex have sensible defaults 
(provided by you), whereas textconv is strictly a user's option.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 1/3] Purest update to bash completions to prevent unbounded variable errors.
From: Ted Pavlic @ 2009-01-14 20:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, git
In-Reply-To: <7vvdsj6lfq.fsf@gitster.siamese.dyndns.org>

>> Would you like me to modify the commit message and resubmit?
>
> Luckily or unluckily, you need to do that anyway, as your patches are
> whitespace damaged.
>
> Please don't send "format=flowed".

Ok. Resubmitted using 'git send-email'. I think the submissions look 
much better now.<?>

--Ted


-- 
Ted Pavlic <ted@tedpavlic.com>

   Please visit my ALS association page:
         http://web.alsa.org/goto/tedpavlic
   My family appreciates your support in the fight to defeat ALS.

^ permalink raw reply

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

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

Johannes Schindelin wrote:
> 
> 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().

Umm.... but you seem to do?

Ack on the new regex semantics, though I'd have implemented it via
dying on '\n' instead of silently splitting there (and restarting a
new match!).  [I actually _have_ implemented it, but your patch beat
me to it. :-)]

Thus, Ack on 4/4 once the boundary bug is fixed.  Thanks for your
work!

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

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

* 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

* 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

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

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


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