git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Pickaxe fixes.
@ 2005-05-28  9:53 ` Junio C Hamano
  2005-05-28 10:59   ` Thomas Glanzmann
  0 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2005-05-28  9:53 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

A bug in the command line argument parsing code was making
pickaxe not to work at all in diff-cache and diff-files commands.
Embarrassingly enough, the working pickaxe in diff-tree tells me
that it was not working in these two commands from day one.
This patch fixes it.

Also updates the documentation to describe the --pickaxe-all option.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

Documentation/git-diff-cache.txt |    6 +++++-
Documentation/git-diff-files.txt |    6 +++++-
Documentation/git-diff-tree.txt  |    7 ++++++-
diff-cache.c                     |    2 +-
diff-files.c                     |    2 +-
5 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-diff-cache.txt b/Documentation/git-diff-cache.txt
--- a/Documentation/git-diff-cache.txt
+++ b/Documentation/git-diff-cache.txt
@@ -9,7 +9,7 @@ git-diff-cache - Compares content and mo
 
 SYNOPSIS
 --------
-'git-diff-cache' [-p] [-r] [-z] [-m] [-M] [-R] [-C] [-S<string>] [--cached] <tree-ish> [<path>...]
+'git-diff-cache' [-p] [-r] [-z] [-m] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [--cached] <tree-ish> [<path>...]
 
 DESCRIPTION
 -----------
@@ -44,6 +44,10 @@ OPTIONS
 -S<string>::
 	Look for differences that contains the change in <string>.
 
+--pickaxe-all::
+	When -S finds a change, show all the changes in that
+	changeset, not just the files that contains the change
+	in <string>.
 
 -R::
 	Output diff in reverse.
diff --git a/Documentation/git-diff-files.txt b/Documentation/git-diff-files.txt
--- a/Documentation/git-diff-files.txt
+++ b/Documentation/git-diff-files.txt
@@ -9,7 +9,7 @@ git-diff-files - Compares files in the w
 
 SYNOPSIS
 --------
-'git-diff-files' [-p] [-q] [-r] [-z] [-M] [-C] [-R] [-S<string>] [<pattern>...]
+'git-diff-files' [-p] [-q] [-r] [-z] [-M] [-C] [-R] [-S<string>] [--pickaxe-all] [<pattern>...]
 
 DESCRIPTION
 -----------
@@ -38,6 +38,10 @@ OPTIONS
 -S<string>::
 	Look for differences that contains the change in <string>.
 
+--pickaxe-all::
+	When -S finds a change, show all the changes in that
+	changeset, not just the files that contains the change
+	in <string>.
 
 -r::
 	This flag does not mean anything.  It is there only to match
diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt
--- a/Documentation/git-diff-tree.txt
+++ b/Documentation/git-diff-tree.txt
@@ -9,7 +9,7 @@ git-diff-tree - Compares the content and
 
 SYNOPSIS
 --------
-'git-diff-tree' [-p] [-r] [-z] [--stdin] [-M] [-R] [-C] [-S<string>] [-m] [-s] [-v] [-t] <tree-ish> <tree-ish> [<pattern>]\*
+'git-diff-tree' [-p] [-r] [-z] [--stdin] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [-m] [-s] [-v] [-t] <tree-ish> <tree-ish> [<pattern>]\*
 
 DESCRIPTION
 -----------
@@ -45,6 +45,11 @@ OPTIONS
 -S<string>::
 	Look for differences that contains the change in <string>.
 
+--pickaxe-all::
+	When -S finds a change, show all the changes in that
+	changeset, not just the files that contains the change
+	in <string>.
+
 -r::
 	recurse
 
diff --git a/diff-cache.c b/diff-cache.c
--- a/diff-cache.c
+++ b/diff-cache.c
@@ -206,7 +206,7 @@ int main(int argc, const char **argv)
 			diff_setup_opt |= DIFF_SETUP_REVERSE;
 			continue;
 		}
-		if (!strcmp(arg, "-S")) {
+		if (!strncmp(arg, "-S", 2)) {
 			pickaxe = arg + 2;
 			continue;
 		}
diff --git a/diff-files.c b/diff-files.c
--- a/diff-files.c
+++ b/diff-files.c
@@ -53,7 +53,7 @@ int main(int argc, const char **argv)
 			diff_output_format = DIFF_FORMAT_MACHINE;
 		else if (!strcmp(argv[1], "-R"))
 			diff_setup_opt |= DIFF_SETUP_REVERSE;
-		else if (!strcmp(argv[1], "-S"))
+		else if (!strncmp(argv[1], "-S", 2))
 			pickaxe = argv[1] + 2;
 		else if (!strcmp(argv[1], "--pickaxe-all"))
 			pickaxe_opts = DIFF_PICKAXE_ALL;
------------------------------------------------


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] Add -O<orderfile> option to diff-* brothers.
@ 2005-05-28  9:54 Junio C Hamano
  2005-05-28  9:53 ` [PATCH] Pickaxe fixes Junio C Hamano
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Junio C Hamano @ 2005-05-28  9:54 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

A new diffcore filter diffcore-order is introduced.  This takes
a text file each of whose line is a shell glob pattern.  Patches
matching the glob pattern on an earlier line in the file are
output before patches matching the glob pattern on a later line,
and patches not matching any glob pattern is output last.

A typical orderfile for git project would probably look like
this:

    README
    Documentation
    Makefile
    *.h
    *.c

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

Documentation/git-diff-cache.txt |    6 +
Documentation/git-diff-files.txt |    6 +
Documentation/git-diff-tree.txt  |    6 +
Makefile                         |    3 
diff-cache.c                     |    7 ++
diff-files.c                     |    5 +
diff-tree.c                      |    7 ++
diff.h                           |    2 
diffcore-order.c                 |  121 +++++++++++++++++++++++++++++++++++++++
9 files changed, 159 insertions(+), 4 deletions(-)
new file (100644): diffcore-order.c

diff --git a/Documentation/git-diff-cache.txt b/Documentation/git-diff-cache.txt
--- a/Documentation/git-diff-cache.txt
+++ b/Documentation/git-diff-cache.txt
@@ -9,7 +9,7 @@ git-diff-cache - Compares content and mo
 
 SYNOPSIS
 --------
-'git-diff-cache' [-p] [-r] [-z] [-m] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [--cached] <tree-ish> [<path>...]
+'git-diff-cache' [-p] [-r] [-z] [-m] [-M] [-R] [-C] [-O<orderfile>] [-S<string>] [--pickaxe-all] [--cached] <tree-ish> [<path>...]
 
 DESCRIPTION
 -----------
@@ -49,6 +49,10 @@ OPTIONS
 	changeset, not just the files that contains the change
 	in <string>.
 
+-O<orderfile>::
+	Output the patch in the order specified in the
+	<orderfile>, which has one shell glob pattern per line.
+
 -R::
 	Output diff in reverse.
 
diff --git a/Documentation/git-diff-files.txt b/Documentation/git-diff-files.txt
--- a/Documentation/git-diff-files.txt
+++ b/Documentation/git-diff-files.txt
@@ -9,7 +9,7 @@ git-diff-files - Compares files in the w
 
 SYNOPSIS
 --------
-'git-diff-files' [-p] [-q] [-r] [-z] [-M] [-C] [-R] [-S<string>] [--pickaxe-all] [<pattern>...]
+'git-diff-files' [-p] [-q] [-r] [-z] [-M] [-C] [-R] [-O<orderfile>] [-S<string>] [--pickaxe-all] [<pattern>...]
 
 DESCRIPTION
 -----------
@@ -43,6 +43,10 @@ OPTIONS
 	changeset, not just the files that contains the change
 	in <string>.
 
+-O<orderfile>::
+	Output the patch in the order specified in the
+	<orderfile>, which has one shell glob pattern per line.
+
 -r::
 	This flag does not mean anything.  It is there only to match
 	git-diff-tree.  Unlike git-diff-tree, git-diff-files always looks
diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt
--- a/Documentation/git-diff-tree.txt
+++ b/Documentation/git-diff-tree.txt
@@ -9,7 +9,7 @@ git-diff-tree - Compares the content and
 
 SYNOPSIS
 --------
-'git-diff-tree' [-p] [-r] [-z] [--stdin] [-M] [-R] [-C] [-S<string>] [--pickaxe-all] [-m] [-s] [-v] [-t] <tree-ish> <tree-ish> [<pattern>]\*
+'git-diff-tree' [-p] [-r] [-z] [--stdin] [-M] [-R] [-C] [-O<orderfile>] [-S<string>] [--pickaxe-all] [-m] [-s] [-v] [-t] <tree-ish> <tree-ish> [<pattern>]\*
 
 DESCRIPTION
 -----------
@@ -50,6 +50,10 @@ OPTIONS
 	changeset, not just the files that contains the change
 	in <string>.
 
+-O<orderfile>::
+	Output the patch in the order specified in the
+	<orderfile>, which has one shell glob pattern per line.
+
 -r::
 	recurse
 
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -48,7 +48,7 @@ LIB_OBJS += strbuf.o
 
 LIB_H += diff.h count-delta.h
 LIB_OBJS += diff.o diffcore-rename.o diffcore-pickaxe.o diffcore-pathspec.o \
-	count-delta.o
+	count-delta.o diffcore-order.o
 
 LIB_OBJS += gitenv.o
 
@@ -130,6 +130,7 @@ diff.o: $(LIB_H) diffcore.h
 diffcore-rename.o : $(LIB_H) diffcore.h
 diffcore-pathspec.o : $(LIB_H) diffcore.h
 diffcore-pickaxe.o : $(LIB_H) diffcore.h
+diffcore-order.o : $(LIB_H) diffcore.h
 
 test: all
 	$(MAKE) -C t/ all
diff --git a/diff-cache.c b/diff-cache.c
--- a/diff-cache.c
+++ b/diff-cache.c
@@ -9,6 +9,7 @@ static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
 static int pickaxe_opts = 0;
+static const char *orderfile = NULL;
 
 /* A file entry went away or appeared */
 static void show_file(const char *prefix, struct cache_entry *ce, unsigned char *sha1, unsigned int mode)
@@ -210,6 +211,10 @@ int main(int argc, const char **argv)
 			pickaxe = arg + 2;
 			continue;
 		}
+		if (!strncmp(arg, "-O", 2)) {
+			orderfile = arg + 2;
+			continue;
+		}
 		if (!strcmp(arg, "--pickaxe-all")) {
 			pickaxe_opts = DIFF_PICKAXE_ALL;
 			continue;
@@ -246,6 +251,8 @@ int main(int argc, const char **argv)
 		diffcore_rename(detect_rename, diff_score_opt);
 	if (pickaxe)
 		diffcore_pickaxe(pickaxe, pickaxe_opts);
+	if (orderfile)
+		diffcore_order(orderfile);
 	diff_flush(diff_output_format, 1);
 	return ret;
 }
diff --git a/diff-files.c b/diff-files.c
--- a/diff-files.c
+++ b/diff-files.c
@@ -15,6 +15,7 @@ static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
 static int pickaxe_opts = 0;
+static const char *orderfile = NULL;
 static int silent = 0;
 
 static void show_unmerge(const char *path)
@@ -55,6 +56,8 @@ int main(int argc, const char **argv)
 			diff_setup_opt |= DIFF_SETUP_REVERSE;
 		else if (!strncmp(argv[1], "-S", 2))
 			pickaxe = argv[1] + 2;
+		else if (!strncmp(argv[1], "-O", 2))
+			orderfile = argv[1] + 2;
 		else if (!strcmp(argv[1], "--pickaxe-all"))
 			pickaxe_opts = DIFF_PICKAXE_ALL;
 		else if (!strncmp(argv[1], "-M", 2)) {
@@ -122,6 +125,8 @@ int main(int argc, const char **argv)
 		diffcore_rename(detect_rename, diff_score_opt);
 	if (pickaxe)
 		diffcore_pickaxe(pickaxe, pickaxe_opts);
+	if (orderfile)
+		diffcore_order(orderfile);
 	diff_flush(diff_output_format, 1);
 	return 0;
 }
diff --git a/diff-tree.c b/diff-tree.c
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -14,6 +14,7 @@ static int diff_setup_opt = 0;
 static int diff_score_opt = 0;
 static const char *pickaxe = NULL;
 static int pickaxe_opts = 0;
+static const char *orderfile = NULL;
 static const char *header = NULL;
 static const char *header_prefix = "";
 
@@ -269,6 +270,8 @@ static int call_diff_flush(void)
 		diff_flush(DIFF_FORMAT_NO_OUTPUT, 0);
 		return 0;
 	}
+	if (orderfile)
+		diffcore_order(orderfile);
 	if (header) {
 		const char *fmt = "%s";
 		if (diff_output_format == DIFF_FORMAT_MACHINE)
@@ -510,6 +513,10 @@ int main(int argc, const char **argv)
 			pickaxe = arg + 2;
 			continue;
 		}
+		if (!strncmp(arg, "-O", 2)) {
+			orderfile = arg + 2;
+			continue;
+		}
 		if (!strcmp(arg, "--pickaxe-all")) {
 			pickaxe_opts = DIFF_PICKAXE_ALL;
 			continue;
diff --git a/diff.h b/diff.h
--- a/diff.h
+++ b/diff.h
@@ -43,6 +43,8 @@ extern void diffcore_pickaxe(const char 
 
 extern void diffcore_pathspec(const char **pathspec);
 
+extern void diffcore_order(const char *orderfile);
+
 extern int diff_queue_is_empty(void);
 
 #define DIFF_FORMAT_HUMAN	0
diff --git a/diffcore-order.c b/diffcore-order.c
new file mode 100644
--- /dev/null
+++ b/diffcore-order.c
@@ -0,0 +1,121 @@
+/*
+ * Copyright (C) 2005 Junio C Hamano
+ */
+#include "cache.h"
+#include "diff.h"
+#include "diffcore.h"
+#include <fnmatch.h>
+
+static char **order;
+static int order_cnt;
+
+static void prepare_order(const char *orderfile)
+{
+	int fd, cnt, pass;
+	void *map;
+	char *cp, *endp;
+	struct stat st;
+
+	if (order)
+		return;
+
+	fd = open(orderfile, O_RDONLY);
+	if (fd < 0)
+		return;
+	if (fstat(fd, &st)) {
+		close(fd);
+		return;
+	}
+	map = mmap(NULL, st.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
+	close(fd);
+	if (-1 == (int)(long)map)
+		return;
+	endp = map + st.st_size;
+	for (pass = 0; pass < 2; pass++) {
+		cnt = 0;
+		cp = map;
+		while (cp < endp) {
+			char *ep;
+			for (ep = cp; ep < endp && *ep != '\n'; ep++)
+				;
+			/* cp to ep has one line */
+			if (*cp == '\n' || *cp == '#')
+				; /* comment */
+			else if (pass == 0)
+				cnt++;
+			else {
+				if (*ep == '\n') {
+					*ep = 0;
+					order[cnt] = cp;
+				}
+				else {
+					order[cnt] = xmalloc(ep-cp+1);
+					memcpy(order[cnt], cp, ep-cp);
+					order[cnt][ep-cp] = 0;
+				}
+				cnt++;
+			}
+			if (ep < endp)
+				ep++;
+			cp = ep;
+		}
+		if (pass == 0) {
+			order_cnt = cnt;
+			order = xmalloc(sizeof(*order) * cnt);
+		}
+	}
+}
+
+struct pair_order {
+	struct diff_filepair *pair;
+	int orig_order;
+	int order;
+};
+
+static int match_order(const char *path)
+{
+	int i;
+	char p[PATH_MAX];
+
+	for (i = 0; i < order_cnt; i++) {
+		strcpy(p, path);
+		while (p[0]) {
+			char *cp;
+			if (!fnmatch(order[i], p, 0))
+				return i;
+			cp = strrchr(p, '/');
+			if (!cp)
+				break;
+			*cp = 0;
+		}
+	}
+	return order_cnt;
+}
+
+static int compare_pair_order(const void *a_, const void *b_)
+{
+	struct pair_order const *a, *b;
+	a = (struct pair_order const *)a_;
+	b = (struct pair_order const *)b_;
+	if (a->order != b->order)
+		return a->order - b->order;
+	return a->orig_order - b->orig_order;
+}
+
+void diffcore_order(const char *orderfile)
+{
+	struct diff_queue_struct *q = &diff_queued_diff;
+	struct pair_order *o = xmalloc(sizeof(*o) * q->nr);
+	int i;
+
+	prepare_order(orderfile);
+	for (i = 0; i < q->nr; i++) {
+		o[i].pair = q->queue[i];
+		o[i].orig_order = i;
+		o[i].order = match_order(o[i].pair->two->path);
+	}
+	qsort(o, q->nr, sizeof(*o), compare_pair_order);
+	for (i = 0; i < q->nr; i++)
+		q->queue[i] = o[i].pair;
+	return;
+}
------------------------------------------------


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Pickaxe fixes.
  2005-05-28  9:53 ` [PATCH] Pickaxe fixes Junio C Hamano
@ 2005-05-28 10:59   ` Thomas Glanzmann
  2005-05-28 16:16     ` Junio C Hamano
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Glanzmann @ 2005-05-28 10:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hello,

> Also updates the documentation to describe the --pickaxe-all option.
> A typical orderfile for git project would probably look like this ...

I can't believe it. I began to support pickaxe this morning in my little
git frontend and thought about exactly these two questions (patch
ordering and show me all deltas in the matched changeset). Thanks for
the patches! Btw. When doing the.  'git-whatchanged drivers/ide' thing
... does that show only deltas which happened actually in this tree or
if a changeset contains deltas inside and outside?

	Thomas

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add -O<orderfile> option to diff-* brothers.
  2005-05-28  9:54 [PATCH] Add -O<orderfile> option to diff-* brothers Junio C Hamano
  2005-05-28  9:53 ` [PATCH] Pickaxe fixes Junio C Hamano
@ 2005-05-28 11:12 ` Petr Baudis
  2005-05-28 11:19   ` Thomas Glanzmann
  2005-05-28 16:14   ` Junio C Hamano
  2005-05-28 16:44 ` [PATCH] Fix leak in diffcore-order.c Junio C Hamano
  2005-05-29 18:56 ` [PATCH] Add -O<orderfile> option to diff-* brothers Linus Torvalds
  3 siblings, 2 replies; 14+ messages in thread
From: Petr Baudis @ 2005-05-28 11:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git

Dear diary, on Sat, May 28, 2005 at 11:54:39AM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> told me that...
> A new diffcore filter diffcore-order is introduced.  This takes
> a text file each of whose line is a shell glob pattern.  Patches
> matching the glob pattern on an earlier line in the file are
> output before patches matching the glob pattern on a later line,
> and patches not matching any glob pattern is output last.
> 
> A typical orderfile for git project would probably look like
> this:
> 
>     README
>     Documentation
>     Makefile
>     *.h
>     *.c
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>

Hmm, is this really useful in practice? diff itself doesn't appear to
have it either, and I haven't seen something like this before. So is it
worth the code?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add -O<orderfile> option to diff-* brothers.
  2005-05-28 11:12 ` [PATCH] Add -O<orderfile> option to diff-* brothers Petr Baudis
@ 2005-05-28 11:19   ` Thomas Glanzmann
  2005-05-28 16:25     ` Junio C Hamano
  2005-05-28 16:14   ` Junio C Hamano
  1 sibling, 1 reply; 14+ messages in thread
From: Thomas Glanzmann @ 2005-05-28 11:19 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Junio C Hamano, git

Hello,

> Hmm, is this really useful in practice? diff itself doesn't appear to
> have it either, and I haven't seen something like this before. So is it
> worth the code?

I think it is. For example on git development it is nice to see the
Documentation first. So you know what it is supposed to do and later you
see the actual implementation. I think Junio requested this exactly
before.

	Thoms

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add -O<orderfile> option to diff-* brothers.
  2005-05-28 11:12 ` [PATCH] Add -O<orderfile> option to diff-* brothers Petr Baudis
  2005-05-28 11:19   ` Thomas Glanzmann
@ 2005-05-28 16:14   ` Junio C Hamano
  1 sibling, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2005-05-28 16:14 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Linus Torvalds, git

>>>>> "PB" == Petr Baudis <pasky@ucw.cz> writes:

PB> Hmm, is this really useful in practice? diff itself doesn't appear to
PB> have it either, and I haven't seen something like this before. So is it
PB> worth the code?

CVS nor SVN does not do pickaxe as far as I know either, but I
think some people in GIT community are already having fun with
pickaxe.

The amount of the code is small, and it does not impact if you
do not use the option.  I am more concerned by (1) the fact that
each new diffcore filters would carve out its own niche from the
scarse command option namespace; (2) you have to duplicate the
option parsing in all three diff-* brothers; and (3) diff-tree
is sometimes an odd-man-out among those three and it is not so
simple to unify the option parsing.  A help in that area is very
much appreciated.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Pickaxe fixes.
  2005-05-28 10:59   ` Thomas Glanzmann
@ 2005-05-28 16:16     ` Junio C Hamano
  2005-05-28 16:22       ` Thomas Glanzmann
  0 siblings, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2005-05-28 16:16 UTC (permalink / raw)
  To: Thomas Glanzmann; +Cc: git

>>>>> "TG" == Thomas Glanzmann <sithglan@stud.uni-erlangen.de> writes:

TG> Btw. When doing the.  'git-whatchanged drivers/ide' thing
TG> ... does that show only deltas which happened actually in
TG> this tree or if a changeset contains deltas inside and
TG> outside?

That's up to Linus right now.  The behaviour is different
before and after the 12-part patch I sent out yesterday [*1*].

My earlier thinking was to use the pathspec (e.g. drivers/ide)
as the output filter, but Linus corrected me yesterday morning
that he wants the pathspec as the input filter.  What this means
to your question is:

 - Under my earlier thinking, if drivers/ide/foo is a rename
   from drivers/usb/bar, diffcore gets both creation of foo and
   deletion of bar, diffcore-rename matches them up, and
   diffcore-pathspec filters the output to drivers/ide tree.
   Earlier pathspec picked up filepair that had the specified
   path on either src or dst side, so this would have been shown
   as a rename from usb/bar to ide/foo.

 - After the 12-part patch, the pathspec comes first and
   establishes a narrowed down world we operate in.  So in the
   same situation, diffcore is fed only creation of foo but does
   not see deletion of bar (which happened in drivers/usb, which
   is outside our world).  Hence diffcore-rename does not have
   anything to match up and this would be shown as a creation of
   ide/foo.

I agree with Linus that the latter is the semantics we usually
want.  If we make diffcore filters stackable (I mean, the order
of applications controllable by the program and the user), we
could also have the older semantics when the user wants it, but
I would do that after this series stabilizes.


[Footnote]

*1* When I talk about time of the day, it is in US Pacific
timezone.




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Pickaxe fixes.
  2005-05-28 16:16     ` Junio C Hamano
@ 2005-05-28 16:22       ` Thomas Glanzmann
  2005-05-30  7:35         ` Junio C Hamano
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Glanzmann @ 2005-05-28 16:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hello,

> I agree with Linus that the latter is the semantics we usually
> want.  If we make diffcore filters stackable (I mean, the order
> of applications controllable by the program and the user), we
> could also have the older semantics when the user wants it, but
> I would do that after this series stabilizes.

thanks for the elaboration on this topic. However at the moment I don't
have an opinion on this, I have to use it a bit longer. But it is a good
thing that I know by now that it limits its view to the subdirectory
after your patch-train is applied.

	Thomas

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add -O<orderfile> option to diff-* brothers.
  2005-05-28 11:19   ` Thomas Glanzmann
@ 2005-05-28 16:25     ` Junio C Hamano
  0 siblings, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2005-05-28 16:25 UTC (permalink / raw)
  To: Thomas Glanzmann; +Cc: Petr Baudis, git

>>>>> "TG" == Thomas Glanzmann <sithglan@stud.uni-erlangen.de> writes:

TG> I think it is. For example on git development it is nice to see the
TG> Documentation first. So you know what it is supposed to do and later you
TG> see the actual implementation. I think Junio requested this exactly
TG> before.

Well, yes and no.  I do want to see the patches in the suggested
order because that would make things easier to understand if
they are related.

The request you are referring to was not about the order of
patches in one e-mail message.  It was more like "please show us
documentation first so that we can pick nits in the design (goal
of implementation) before you start to code".  That way, if
there were a change (like -A flag in checkout-cache) that people
would find objectionable, the effort to code the change and
the effort to review the code can both be saved.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] Fix leak in diffcore-order.c
  2005-05-28  9:54 [PATCH] Add -O<orderfile> option to diff-* brothers Junio C Hamano
  2005-05-28  9:53 ` [PATCH] Pickaxe fixes Junio C Hamano
  2005-05-28 11:12 ` [PATCH] Add -O<orderfile> option to diff-* brothers Petr Baudis
@ 2005-05-28 16:44 ` Junio C Hamano
  2005-05-29 18:56 ` [PATCH] Add -O<orderfile> option to diff-* brothers Linus Torvalds
  3 siblings, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2005-05-28 16:44 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

A temporary array used for sorting the filepairs were not freed
after use.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diffcore-order.c |    1 +
1 files changed, 1 insertion(+)

diff --git a/diffcore-order.c b/diffcore-order.c
--- a/diffcore-order.c
+++ b/diffcore-order.c
@@ -117,5 +117,6 @@ void diffcore_order(const char *orderfil
 	qsort(o, q->nr, sizeof(*o), compare_pair_order);
 	for (i = 0; i < q->nr; i++)
 		q->queue[i] = o[i].pair;
+	free(o);
 	return;
 }
------------------------------------------------


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add -O<orderfile> option to diff-* brothers.
  2005-05-28  9:54 [PATCH] Add -O<orderfile> option to diff-* brothers Junio C Hamano
                   ` (2 preceding siblings ...)
  2005-05-28 16:44 ` [PATCH] Fix leak in diffcore-order.c Junio C Hamano
@ 2005-05-29 18:56 ` Linus Torvalds
  2005-05-29 20:32   ` Junio C Hamano
  2005-05-30  0:23   ` Junio C Hamano
  3 siblings, 2 replies; 14+ messages in thread
From: Linus Torvalds @ 2005-05-29 18:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git



On Sat, 28 May 2005, Junio C Hamano wrote:
>
> A new diffcore filter diffcore-order is introduced.  This takes
> a text file each of whose line is a shell glob pattern.  Patches
> matching the glob pattern on an earlier line in the file are
> output before patches matching the glob pattern on a later line,
> and patches not matching any glob pattern is output last.

This smells to me like a "cool feature, because we can" rather than a
"this is useful because of xxx".

You could equally well do it with 

	git-diff-cache .. | sort-filenames | git-diff-helper

in porcelain, if somebody really wants this (but I can't see people 
clamoring for it, since nobody else has ever done soemthing like this, 
afaik).

In other words: what is the problem this is trying to solve?

		Linus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add -O<orderfile> option to diff-* brothers.
  2005-05-29 18:56 ` [PATCH] Add -O<orderfile> option to diff-* brothers Linus Torvalds
@ 2005-05-29 20:32   ` Junio C Hamano
  2005-05-30  0:23   ` Junio C Hamano
  1 sibling, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2005-05-29 20:32 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> This smells to me like a "cool feature, because we can"
LT> rather than a "this is useful because of xxx".

You could even do the rename/copy detection outside with a
standalone diff-raw filter, just like your "sort-filenames" pipe
example.  Rename/copy is not done that way primarily for the
performance reasons.  Pickaxe is not done that way because we
wanted to conditionally suppress diff-tree headers, and it is
easier to implement the suppression inside rather than as a
"sort-filename" like filter.

This diffcore-order is more like "cool feature, because we can,
_and_ it is simpler to do it inside rather than outside, now we
already have the infrastructure inside to do this kind of thing
for other purposes anyway".





^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add -O<orderfile> option to diff-* brothers.
  2005-05-29 18:56 ` [PATCH] Add -O<orderfile> option to diff-* brothers Linus Torvalds
  2005-05-29 20:32   ` Junio C Hamano
@ 2005-05-30  0:23   ` Junio C Hamano
  1 sibling, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2005-05-30  0:23 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

I realize I did not answer your question.

LT> In other words: what is the problem this is trying to solve?

To produce a patch that is easier to review, using customized
patch order list for projects.  I envision that Porcelain
noticing the existence of ${GIT-.git}/patch-order file and
adding -O to its diff-* argument would make the world a better
place.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Pickaxe fixes.
  2005-05-28 16:22       ` Thomas Glanzmann
@ 2005-05-30  7:35         ` Junio C Hamano
  0 siblings, 0 replies; 14+ messages in thread
From: Junio C Hamano @ 2005-05-30  7:35 UTC (permalink / raw)
  To: Thomas Glanzmann; +Cc: git

>>>>> "TG" == Thomas Glanzmann <sithglan@stud.uni-erlangen.de> writes:

TG> ... However at the moment I don't
TG> have an opinion on this, I have to use it a bit longer. But it is a good
TG> thing that I know by now that it limits its view to the subdirectory
TG> after your patch-train is applied.

Your opinion on this would not count anymore ;-).  Pathspec is now
at the beginning of the processiong chain.


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2005-05-30  7:33 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-05-28  9:54 [PATCH] Add -O<orderfile> option to diff-* brothers Junio C Hamano
2005-05-28  9:53 ` [PATCH] Pickaxe fixes Junio C Hamano
2005-05-28 10:59   ` Thomas Glanzmann
2005-05-28 16:16     ` Junio C Hamano
2005-05-28 16:22       ` Thomas Glanzmann
2005-05-30  7:35         ` Junio C Hamano
2005-05-28 11:12 ` [PATCH] Add -O<orderfile> option to diff-* brothers Petr Baudis
2005-05-28 11:19   ` Thomas Glanzmann
2005-05-28 16:25     ` Junio C Hamano
2005-05-28 16:14   ` Junio C Hamano
2005-05-28 16:44 ` [PATCH] Fix leak in diffcore-order.c Junio C Hamano
2005-05-29 18:56 ` [PATCH] Add -O<orderfile> option to diff-* brothers Linus Torvalds
2005-05-29 20:32   ` Junio C Hamano
2005-05-30  0:23   ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).