[PATCH 0/3] git-rev-list libification effort: the next stage

[PATCH 0/3] git-rev-list libification effort: the next stage

[Linus Torvalds]

Ok, the following three patches that I'll send out are still pretty rough, 
but they actually get us to the first real point of this whole exercise: 
writing one of the trivial git helper scripts in C.

In particular, at the end, we have "git log" being implemented as this 
trivial C function:

	#define LOGSIZE (65536)

	static int cmd_log(int argc, char **argv, char **envp)
	{
		struct rev_info rev;
		struct commit *commit;
		char *buf = xmalloc(LOGSIZE);

		argc = setup_revisions(argc, argv, &rev, "HEAD");
		prepare_revision_walk(&rev);
		setup_pager();
		while ((commit = get_revision(&rev)) != NULL) {
			pretty_print_commit(CMIT_FMT_DEFAULT, commit, ~0, buf, LOGSIZE, 18);
			printf("%s\n", buf);
		}
		free(buf);
		return 0;
	}

which is actually a pretty good example of what I wanted to do.  It's
not perfect yet (it doesn't parse the "--pretty=xxx" option yet, nor the
"--since" and "--until" dates, for example), but I think this is all
going in the right direction. 

			Linus

[PATCH 1/3] git-rev-list libification: rev-list walking

[Linus Torvalds]

This actually moves the "meat" of the revision walking from rev-list.c
to the new library code in revision.h. It introduces the new functions

	void prepare_revision_walk(struct rev_info *revs);
	struct commit *get_revision(struct rev_info *revs);

to prepare and then walk the revisions that we have.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

All the same old warnigns apply!! This is a bit more intrusive than the 
previous patch series, since it actually changes how things work. It 
passes all the tests I threw at it (well, actually, I only tested the end 
result of the whole series, bad me), but I'd obviously like to remind 
everybody that this is some really core code, and mistakes are bad.

I didn't worry about cleaning code up. It probably could be cleaned up, 
but I worked at trying to just move as much of it as-is from rev-list.c as 
possible, while leaving any code that was really only relevant to rev-list 
itself alone.

diff --git a/rev-list.c b/rev-list.c
index 2e80930..94f22dd 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -8,11 +8,10 @@
 #include "diff.h"
 #include "revision.h"
 
-/* bits #0 and #1 in revision.h */
+/* bits #0-2 in revision.h */
 
-#define COUNTED		(1u << 2)
-#define SHOWN		(1u << 3)
-#define TREECHANGE	(1u << 4)
+#define COUNTED		(1u << 3)
+#define SHOWN		(1u << 4)
 #define TMP_MARK	(1u << 5) /* for isolated cases; clean after use */
 
 static const char rev_list_usage[] =
@@ -213,17 +212,17 @@ static struct object_list **process_tree
 	return p;
 }
 
-static void show_commit_list(struct commit_list *list)
+static void show_commit_list(struct rev_info *revs)
 {
+	struct commit *commit;
 	struct object_list *objects = NULL, **p = &objects, *pending;
-	while (list) {
-		struct commit *commit = pop_most_recent_commit(&list, SEEN);
 
+	while ((commit = get_revision(revs)) != NULL) {
 		p = process_tree(commit->tree, p, NULL, "");
 		if (process_commit(commit) == STOP)
 			break;
 	}
-	for (pending = revs.pending_objects; pending; pending = pending->next) {
+	for (pending = revs->pending_objects; pending; pending = pending->next) {
 		struct object *obj = pending->item;
 		const char *name = pending->name;
 		if (obj->flags & (UNINTERESTING | SEEN))
@@ -259,19 +258,6 @@ static void show_commit_list(struct comm
 	}
 }
 
-static int everybody_uninteresting(struct commit_list *orig)
-{
-	struct commit_list *list = orig;
-	while (list) {
-		struct commit *commit = list->item;
-		list = list->next;
-		if (commit->object.flags & UNINTERESTING)
-			continue;
-		return 0;
-	}
-	return 1;
-}
-
 /*
  * This is a truly stupid algorithm, but it's only
  * used for bisection, and we just don't care enough.
@@ -379,224 +365,12 @@ static void mark_edges_uninteresting(str
 	}
 }
 
-#define TREE_SAME	0
-#define TREE_NEW	1
-#define TREE_DIFFERENT	2
-static int tree_difference = TREE_SAME;
-
-static void file_add_remove(struct diff_options *options,
-		    int addremove, unsigned mode,
-		    const unsigned char *sha1,
-		    const char *base, const char *path)
-{
-	int diff = TREE_DIFFERENT;
-
-	/*
-	 * Is it an add of a new file? It means that
-	 * the old tree didn't have it at all, so we
-	 * will turn "TREE_SAME" -> "TREE_NEW", but
-	 * leave any "TREE_DIFFERENT" alone (and if
-	 * it already was "TREE_NEW", we'll keep it
-	 * "TREE_NEW" of course).
-	 */
-	if (addremove == '+') {
-		diff = tree_difference;
-		if (diff != TREE_SAME)
-			return;
-		diff = TREE_NEW;
-	}
-	tree_difference = diff;
-}
-
-static void file_change(struct diff_options *options,
-		 unsigned old_mode, unsigned new_mode,
-		 const unsigned char *old_sha1,
-		 const unsigned char *new_sha1,
-		 const char *base, const char *path)
-{
-	tree_difference = TREE_DIFFERENT;
-}
-
-static struct diff_options diff_opt = {
-	.recursive = 1,
-	.add_remove = file_add_remove,
-	.change = file_change,
-};
-
-static int compare_tree(struct tree *t1, struct tree *t2)
-{
-	if (!t1)
-		return TREE_NEW;
-	if (!t2)
-		return TREE_DIFFERENT;
-	tree_difference = TREE_SAME;
-	if (diff_tree_sha1(t1->object.sha1, t2->object.sha1, "", &diff_opt) < 0)
-		return TREE_DIFFERENT;
-	return tree_difference;
-}
-
-static int same_tree_as_empty(struct tree *t1)
-{
-	int retval;
-	void *tree;
-	struct tree_desc empty, real;
-
-	if (!t1)
-		return 0;
-
-	tree = read_object_with_reference(t1->object.sha1, "tree", &real.size, NULL);
-	if (!tree)
-		return 0;
-	real.buf = tree;
-
-	empty.buf = "";
-	empty.size = 0;
-
-	tree_difference = 0;
-	retval = diff_tree(&empty, &real, "", &diff_opt);
-	free(tree);
-
-	return retval >= 0 && !tree_difference;
-}
-
-static void try_to_simplify_commit(struct commit *commit)
-{
-	struct commit_list **pp, *parent;
-
-	if (!commit->tree)
-		return;
-
-	if (!commit->parents) {
-		if (!same_tree_as_empty(commit->tree))
-			commit->object.flags |= TREECHANGE;
-		return;
-	}
-
-	pp = &commit->parents;
-	while ((parent = *pp) != NULL) {
-		struct commit *p = parent->item;
-
-		if (p->object.flags & UNINTERESTING) {
-			pp = &parent->next;
-			continue;
-		}
-
-		parse_commit(p);
-		switch (compare_tree(p->tree, commit->tree)) {
-		case TREE_SAME:
-			parent->next = NULL;
-			commit->parents = parent;
-			return;
-
-		case TREE_NEW:
-			if (revs.remove_empty_trees && same_tree_as_empty(p->tree)) {
-				*pp = parent->next;
-				continue;
-			}
-		/* fallthrough */
-		case TREE_DIFFERENT:
-			pp = &parent->next;
-			continue;
-		}
-		die("bad tree compare for commit %s", sha1_to_hex(commit->object.sha1));
-	}
-	commit->object.flags |= TREECHANGE;
-}
-
-static void add_parents_to_list(struct commit *commit, struct commit_list **list)
-{
-	struct commit_list *parent = commit->parents;
-
-	/*
-	 * If the commit is uninteresting, don't try to
-	 * prune parents - we want the maximal uninteresting
-	 * set.
-	 *
-	 * Normally we haven't parsed the parent
-	 * yet, so we won't have a parent of a parent
-	 * here. However, it may turn out that we've
-	 * reached this commit some other way (where it
-	 * wasn't uninteresting), in which case we need
-	 * to mark its parents recursively too..
-	 */
-	if (commit->object.flags & UNINTERESTING) {
-		while (parent) {
-			struct commit *p = parent->item;
-			parent = parent->next;
-			parse_commit(p);
-			p->object.flags |= UNINTERESTING;
-			if (p->parents)
-				mark_parents_uninteresting(p);
-			if (p->object.flags & SEEN)
-				continue;
-			p->object.flags |= SEEN;
-			insert_by_date(p, list);
-		}
-		return;
-	}
-
-	/*
-	 * Ok, the commit wasn't uninteresting. Try to
-	 * simplify the commit history and find the parent
-	 * that has no differences in the path set if one exists.
-	 */
-	if (revs.paths)
-		try_to_simplify_commit(commit);
-
-	parent = commit->parents;
-	while (parent) {
-		struct commit *p = parent->item;
-
-		parent = parent->next;
-
-		parse_commit(p);
-		if (p->object.flags & SEEN)
-			continue;
-		p->object.flags |= SEEN;
-		insert_by_date(p, list);
-	}
-}
-
-static struct commit_list *limit_list(struct commit_list *list)
-{
-	struct commit_list *newlist = NULL;
-	struct commit_list **p = &newlist;
-	while (list) {
-		struct commit_list *entry = list;
-		struct commit *commit = list->item;
-		struct object *obj = &commit->object;
-
-		list = list->next;
-		free(entry);
-
-		if (revs.max_age != -1 && (commit->date < revs.max_age))
-			obj->flags |= UNINTERESTING;
-		if (revs.unpacked && has_sha1_pack(obj->sha1))
-			obj->flags |= UNINTERESTING;
-		add_parents_to_list(commit, &list);
-		if (obj->flags & UNINTERESTING) {
-			mark_parents_uninteresting(commit);
-			if (everybody_uninteresting(list))
-				break;
-			continue;
-		}
-		if (revs.min_age != -1 && (commit->date > revs.min_age))
-			continue;
-		p = &commit_list_insert(commit, p)->next;
-	}
-	if (revs.tree_objects)
-		mark_edges_uninteresting(newlist);
-	if (bisect_list)
-		newlist = find_bisection(newlist);
-	return newlist;
-}
-
 int main(int argc, const char **argv)
 {
 	struct commit_list *list;
 	int i;
 
-	argc = setup_revisions(argc, argv, &revs);
+	argc = setup_revisions(argc, argv, &revs, NULL);
 
 	for (i = 1 ; i < argc; i++) {
 		const char *arg = argv[i];
@@ -672,24 +446,18 @@ int main(int argc, const char **argv)
 	    (!(revs.tag_objects||revs.tree_objects||revs.blob_objects) && !revs.pending_objects))
 		usage(rev_list_usage);
 
-	if (revs.paths)
-		diff_tree_setup_paths(revs.paths);
+	prepare_revision_walk(&revs);
+	if (revs.tree_objects)
+		mark_edges_uninteresting(revs.commits);
+
+	if (bisect_list)
+		revs.commits = find_bisection(revs.commits);
 
 	save_commit_buffer = verbose_header;
 	track_object_refs = 0;
 
-	if (!merge_order) {		
-		sort_by_date(&list);
-		if (list && !revs.limited && revs.max_count == 1 &&
-		    !revs.tag_objects && !revs.tree_objects && !revs.blob_objects) {
-			show_commit(list->item);
-			return 0;
-		}
-	        if (revs.limited)
-			list = limit_list(list);
-		if (revs.topo_order)
-			sort_in_topological_order(&list, revs.lifo);
-		show_commit_list(list);
+	if (!merge_order) {
+		show_commit_list(&revs);
 	} else {
 #ifndef NO_OPENSSL
 		if (sort_list_in_merge_order(list, &process_commit)) {
diff --git a/revision.c b/revision.c
index 0422593..fb728c1 100644
--- a/revision.c
+++ b/revision.c
@@ -3,6 +3,7 @@
 #include "blob.h"
 #include "tree.h"
 #include "commit.h"
+#include "diff.h"
 #include "refs.h"
 #include "revision.h"
 
@@ -183,6 +184,229 @@ static struct commit *get_commit_referen
 	die("%s is unknown object", name);
 }
 
+static int everybody_uninteresting(struct commit_list *orig)
+{
+	struct commit_list *list = orig;
+	while (list) {
+		struct commit *commit = list->item;
+		list = list->next;
+		if (commit->object.flags & UNINTERESTING)
+			continue;
+		return 0;
+	}
+	return 1;
+}
+
+#define TREE_SAME	0
+#define TREE_NEW	1
+#define TREE_DIFFERENT	2
+static int tree_difference = TREE_SAME;
+
+static void file_add_remove(struct diff_options *options,
+		    int addremove, unsigned mode,
+		    const unsigned char *sha1,
+		    const char *base, const char *path)
+{
+	int diff = TREE_DIFFERENT;
+
+	/*
+	 * Is it an add of a new file? It means that
+	 * the old tree didn't have it at all, so we
+	 * will turn "TREE_SAME" -> "TREE_NEW", but
+	 * leave any "TREE_DIFFERENT" alone (and if
+	 * it already was "TREE_NEW", we'll keep it
+	 * "TREE_NEW" of course).
+	 */
+	if (addremove == '+') {
+		diff = tree_difference;
+		if (diff != TREE_SAME)
+			return;
+		diff = TREE_NEW;
+	}
+	tree_difference = diff;
+}
+
+static void file_change(struct diff_options *options,
+		 unsigned old_mode, unsigned new_mode,
+		 const unsigned char *old_sha1,
+		 const unsigned char *new_sha1,
+		 const char *base, const char *path)
+{
+	tree_difference = TREE_DIFFERENT;
+}
+
+static struct diff_options diff_opt = {
+	.recursive = 1,
+	.add_remove = file_add_remove,
+	.change = file_change,
+};
+
+static int compare_tree(struct tree *t1, struct tree *t2)
+{
+	if (!t1)
+		return TREE_NEW;
+	if (!t2)
+		return TREE_DIFFERENT;
+	tree_difference = TREE_SAME;
+	if (diff_tree_sha1(t1->object.sha1, t2->object.sha1, "", &diff_opt) < 0)
+		return TREE_DIFFERENT;
+	return tree_difference;
+}
+
+static int same_tree_as_empty(struct tree *t1)
+{
+	int retval;
+	void *tree;
+	struct tree_desc empty, real;
+
+	if (!t1)
+		return 0;
+
+	tree = read_object_with_reference(t1->object.sha1, "tree", &real.size, NULL);
+	if (!tree)
+		return 0;
+	real.buf = tree;
+
+	empty.buf = "";
+	empty.size = 0;
+
+	tree_difference = 0;
+	retval = diff_tree(&empty, &real, "", &diff_opt);
+	free(tree);
+
+	return retval >= 0 && !tree_difference;
+}
+
+static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
+{
+	struct commit_list **pp, *parent;
+
+	if (!commit->tree)
+		return;
+
+	if (!commit->parents) {
+		if (!same_tree_as_empty(commit->tree))
+			commit->object.flags |= TREECHANGE;
+		return;
+	}
+
+	pp = &commit->parents;
+	while ((parent = *pp) != NULL) {
+		struct commit *p = parent->item;
+
+		if (p->object.flags & UNINTERESTING) {
+			pp = &parent->next;
+			continue;
+		}
+
+		parse_commit(p);
+		switch (compare_tree(p->tree, commit->tree)) {
+		case TREE_SAME:
+			parent->next = NULL;
+			commit->parents = parent;
+			return;
+
+		case TREE_NEW:
+			if (revs->remove_empty_trees && same_tree_as_empty(p->tree)) {
+				*pp = parent->next;
+				continue;
+			}
+		/* fallthrough */
+		case TREE_DIFFERENT:
+			pp = &parent->next;
+			continue;
+		}
+		die("bad tree compare for commit %s", sha1_to_hex(commit->object.sha1));
+	}
+	commit->object.flags |= TREECHANGE;
+}
+
+static void add_parents_to_list(struct rev_info *revs, struct commit *commit, struct commit_list **list)
+{
+	struct commit_list *parent = commit->parents;
+
+	/*
+	 * If the commit is uninteresting, don't try to
+	 * prune parents - we want the maximal uninteresting
+	 * set.
+	 *
+	 * Normally we haven't parsed the parent
+	 * yet, so we won't have a parent of a parent
+	 * here. However, it may turn out that we've
+	 * reached this commit some other way (where it
+	 * wasn't uninteresting), in which case we need
+	 * to mark its parents recursively too..
+	 */
+	if (commit->object.flags & UNINTERESTING) {
+		while (parent) {
+			struct commit *p = parent->item;
+			parent = parent->next;
+			parse_commit(p);
+			p->object.flags |= UNINTERESTING;
+			if (p->parents)
+				mark_parents_uninteresting(p);
+			if (p->object.flags & SEEN)
+				continue;
+			p->object.flags |= SEEN;
+			insert_by_date(p, list);
+		}
+		return;
+	}
+
+	/*
+	 * Ok, the commit wasn't uninteresting. Try to
+	 * simplify the commit history and find the parent
+	 * that has no differences in the path set if one exists.
+	 */
+	if (revs->paths)
+		try_to_simplify_commit(revs, commit);
+
+	parent = commit->parents;
+	while (parent) {
+		struct commit *p = parent->item;
+
+		parent = parent->next;
+
+		parse_commit(p);
+		if (p->object.flags & SEEN)
+			continue;
+		p->object.flags |= SEEN;
+		insert_by_date(p, list);
+	}
+}
+
+static void limit_list(struct rev_info *revs)
+{
+	struct commit_list *list = revs->commits;
+	struct commit_list *newlist = NULL;
+	struct commit_list **p = &newlist;
+
+	while (list) {
+		struct commit_list *entry = list;
+		struct commit *commit = list->item;
+		struct object *obj = &commit->object;
+
+		list = list->next;
+		free(entry);
+
+		if (revs->max_age != -1 && (commit->date < revs->max_age))
+			obj->flags |= UNINTERESTING;
+		if (revs->unpacked && has_sha1_pack(obj->sha1))
+			obj->flags |= UNINTERESTING;
+		add_parents_to_list(revs, commit, &list);
+		if (obj->flags & UNINTERESTING) {
+			mark_parents_uninteresting(commit);
+			if (everybody_uninteresting(list))
+				break;
+			continue;
+		}
+		if (revs->min_age != -1 && (commit->date > revs->min_age))
+			continue;
+		p = &commit_list_insert(commit, p)->next;
+	}
+	revs->commits = newlist;
+}
+
 static void add_one_commit(struct commit *commit, struct rev_info *revs)
 {
 	if (!commit || (commit->object.flags & SEEN))
@@ -214,10 +438,9 @@ static void handle_all(struct rev_info *
  *
  * Returns the number of arguments left ("new argc").
  */
-int setup_revisions(int argc, const char **argv, struct rev_info *revs)
+int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def)
 {
 	int i, flags, seen_dashdash;
-	const char *def = NULL;
 	const char **unrecognized = argv+1;
 	int left = 1;
 
@@ -381,3 +604,23 @@ int setup_revisions(int argc, const char
 	*unrecognized = NULL;
 	return left;
 }
+
+void prepare_revision_walk(struct rev_info *revs)
+{
+	if (revs->paths)
+		diff_tree_setup_paths(revs->paths);
+	sort_by_date(&revs->commits);
+	if (revs->limited)
+		limit_list(revs);
+	if (revs->topo_order)
+		sort_in_topological_order(&revs->commits, revs->lifo);
+}
+
+struct commit *get_revision(struct rev_info *revs)
+{
+	if (!revs->commits)
+		return NULL;
+	return pop_most_recent_commit(&revs->commits, SEEN);
+}
+
+
diff --git a/revision.h b/revision.h
index a22f198..0bed3c0 100644
--- a/revision.h
+++ b/revision.h
@@ -3,6 +3,7 @@
 
 #define SEEN		(1u<<0)
 #define UNINTERESTING   (1u<<1)
+#define TREECHANGE	(1u<<2)
 
 struct rev_info {
 	/* Starting list */
@@ -32,7 +33,10 @@ struct rev_info {
 };
 
 /* revision.c */
-extern int setup_revisions(int argc, const char **argv, struct rev_info *revs);
+extern int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def);
+extern void prepare_revision_walk(struct rev_info *revs);
+extern struct commit *get_revision(struct rev_info *revs);
+
 extern void mark_parents_uninteresting(struct commit *commit);
 extern void mark_tree_uninteresting(struct tree *tree);
 

[PATCH 2/3] Introduce trivial new pager.c helper infrastructure

[Linus Torvalds]

This introduces the new function

	void setup_pager(void);

to set up output to be written through a pager applocation.

All in preparation for doing the simple scripts in C.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

Ok, this should be pretty obvious, which is not to say that it shouldn't 
be improved (ie it only handles trivial definitions of PAGER). Any obvious 
improvements are left as an exercise for the reader, as not important for 
my current goal of actually having something working.


diff --git a/Makefile b/Makefile
index 3575489..0b1a998 100644
--- a/Makefile
+++ b/Makefile
@@ -205,7 +205,7 @@ LIB_OBJS = \
 	quote.o read-cache.o refs.o run-command.o \
 	server-info.o setup.o sha1_file.o sha1_name.o strbuf.o \
 	tag.o tree.o usage.o config.o environment.o ctype.o copy.o \
-	fetch-clone.o revision.o \
+	fetch-clone.o revision.o pager.o \
 	$(DIFF_OBJS)
 
 LIBS = $(LIB_FILE)
diff --git a/cache.h b/cache.h
index 58eec00..3af6b86 100644
--- a/cache.h
+++ b/cache.h
@@ -352,4 +352,7 @@ extern int copy_fd(int ifd, int ofd);
 extern int receive_unpack_pack(int fd[2], const char *me, int quiet);
 extern int receive_keep_pack(int fd[2], const char *me, int quiet);
 
+/* pager.c */
+extern void setup_pager(void);
+
 #endif /* CACHE_H */
diff --git a/pager.c b/pager.c
new file mode 100644
index 0000000..1364e15
--- /dev/null
+++ b/pager.c
@@ -0,0 +1,48 @@
+#include "cache.h"
+
+/*
+ * This is split up from the rest of git so that we might do
+ * something different on Windows, for example.
+ */
+
+static void run_pager(void)
+{
+	const char *prog = getenv("PAGER");
+	if (!prog)
+		prog = "less";
+	setenv("LESS", "-S", 0);
+	execlp(prog, prog, NULL);
+}
+
+void setup_pager(void)
+{
+	pid_t pid;
+	int fd[2];
+
+	if (!isatty(1))
+		return;
+	if (pipe(fd) < 0)
+		return;
+	pid = fork();
+	if (pid < 0) {
+		close(fd[0]);
+		close(fd[1]);
+		return;
+	}
+
+	/* return in the child */
+	if (!pid) {
+		dup2(fd[1], 1);
+		close(fd[0]);
+		close(fd[1]);
+		return;
+	}
+
+	/* The original process turns into the PAGER */
+	dup2(fd[0], 0);
+	close(fd[0]);
+	close(fd[1]);
+
+	run_pager();
+	exit(255);
+}

[PATCH 3/3] Tie it all together: "git log"

[Linus Torvalds]

This is what the previous diffs all built up to.

We can do "git log" as a trivial small helper function inside git.c, 
because the infrastructure is all there for us to use as a library.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
----

Again, this may not do exactly what the current "git log" does. That's not 
the point. The point is to introduce the fundamental functionality, so 
that people can play with this and improve on it, and fix any of my stupid 
bugs.

It should be pretty easy to change some of the other rev-list-walking 
functions to use the library interfaces too, instead of executing an 
external "git-rev-list" process. This was a perfect example of how to get 
something working, though.

		Linus

diff --git a/Makefile b/Makefile
index 0b1a998..ead13be 100644
--- a/Makefile
+++ b/Makefile
@@ -450,7 +450,7 @@ strip: $(PROGRAMS) git$X
 
 git$X: git.c $(LIB_FILE)
 	$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
-		$(CFLAGS) $(COMPAT_CFLAGS) -o $@ $(filter %.c,$^) $(LIB_FILE)
+		$(ALL_CFLAGS) -o $@ $(filter %.c,$^) $(LIB_FILE) $(LIBS)
 
 $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 	rm -f $@
diff --git a/git.c b/git.c
index 993cd0d..b0da6b1 100644
--- a/git.c
+++ b/git.c
@@ -12,6 +12,10 @@
 #include "git-compat-util.h"
 #include "exec_cmd.h"
 
+#include "cache.h"
+#include "commit.h"
+#include "revision.h"
+
 #ifndef PATH_MAX
 # define PATH_MAX 4096
 #endif
@@ -245,6 +249,25 @@ static int cmd_help(int argc, char **arg
 	return 0;
 }
 
+#define LOGSIZE (65536)
+
+static int cmd_log(int argc, char **argv, char **envp)
+{
+	struct rev_info rev;
+	struct commit *commit;
+	char *buf = xmalloc(LOGSIZE);
+
+	argc = setup_revisions(argc, argv, &rev, "HEAD");
+	prepare_revision_walk(&rev);
+	setup_pager();
+	while ((commit = get_revision(&rev)) != NULL) {
+		pretty_print_commit(CMIT_FMT_DEFAULT, commit, ~0, buf, LOGSIZE, 18);
+		printf("%s\n", buf);
+	}
+	free(buf);
+	return 0;
+}
+
 #define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
 
 static void handle_internal_command(int argc, char **argv, char **envp)
@@ -256,6 +279,7 @@ static void handle_internal_command(int 
 	} commands[] = {
 		{ "version", cmd_version },
 		{ "help", cmd_help },
+		{ "log", cmd_log },
 	};
 	int i;
 

Re: [PATCH 3/3] Tie it all together: "git log"

[Linus Torvalds]

On Tue, 28 Feb 2006, Linus Torvalds wrote:
> 
> Again, this may not do exactly what the current "git log" does. That's not 
> the point. The point is to introduce the fundamental functionality, so 
> that people can play with this and improve on it, and fix any of my stupid 
> bugs.

Btw, before anybody even pipes up: the missing piece here is the nasty 
"filter_commit()" that rev-list.c does, and that really should be moved 
into revision.c, and this is where I hit on the "--merge-order" issues.

So for example, if you do "git log -- <filename>" with the new git, it 
won't filter out the commits that just change the passed-in <filename> 
properly, because the filtering code still exists only in git-rev-list 
(even if revision.c now does the traversal).

Same goes for the max-count-based filtering, for the same reason.

So the "process_commit()" handling should be moved into "get_revision()", 
but since the merge-order code also hooks into it...

Anyway, apart from that issue (which I think should be trivial to sort out 
if we accept breaking --merge-order), the rest looks like it should just 
get more testing and handling of the few missing flags from rev-parse in 
revision.c, and it should be good.

		Linus

Re: [PATCH 3/3] Tie it all together: "git log"

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> Anyway, apart from that issue (which I think should be trivial to sort out 
> if we accept breaking --merge-order), the rest looks like it should just 
> get more testing and handling of the few missing flags from rev-parse in 
> revision.c, and it should be good.

I would say we should just rip merge-order out.  Who uses it,
and why does it not work with topo-order, again?

Re: [PATCH 3/3] Tie it all together: "git log"

[Linus Torvalds]

On Tue, 28 Feb 2006, Junio C Hamano wrote:
> 
> I would say we should just rip merge-order out.  Who uses it,
> and why does it not work with topo-order, again?

Well, assuming breaking --merge-order is fine, here's a patch (on top of 
the other ones) that makes

	git log <filename>

actually work, as far as I can tell. 

I didn't add the logic for --before/--after flags, but that should be 
pretty trivial, and is independent of this anyway.

Perhaps more importantly, I didn't remove the tests that now start 
failing, nor did I remove the actual code to do --merge-order ;/

		Linus

----
diff --git a/rev-list.c b/rev-list.c
index 94f22dd..6993b1a 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -8,10 +8,9 @@
 #include "diff.h"
 #include "revision.h"
 
-/* bits #0-2 in revision.h */
+/* bits #0-3 in revision.h */
 
-#define COUNTED		(1u << 3)
-#define SHOWN		(1u << 4)
+#define COUNTED		(1u << 4)
 #define TMP_MARK	(1u << 5) /* for isolated cases; clean after use */
 
 static const char rev_list_usage[] =
@@ -25,7 +24,6 @@ static const char rev_list_usage[] =
 "    --remove-empty\n"
 "    --all\n"
 "  ordering output:\n"
-"    --merge-order [ --show-breaks ]\n"
 "    --topo-order\n"
 "    --date-order\n"
 "  formatting output:\n"
@@ -47,22 +45,9 @@ static int show_parents = 0;
 static int hdr_termination = 0;
 static const char *commit_prefix = "";
 static enum cmit_fmt commit_format = CMIT_FMT_RAW;
-static int merge_order = 0;
-static int show_breaks = 0;
-static int stop_traversal = 0;
-static int no_merges = 0;
 
 static void show_commit(struct commit *commit)
 {
-	commit->object.flags |= SHOWN;
-	if (show_breaks) {
-		commit_prefix = "| ";
-		if (commit->object.flags & DISCONTINUITY) {
-			commit_prefix = "^ ";     
-		} else if (commit->object.flags & BOUNDARY) {
-			commit_prefix = "= ";
-		} 
-        }        		
 	printf("%s%s", commit_prefix, sha1_to_hex(commit->object.sha1));
 	if (show_parents) {
 		struct commit_list *parents = commit->parents;
@@ -96,73 +81,6 @@ static void show_commit(struct commit *c
 	fflush(stdout);
 }
 
-static int rewrite_one(struct commit **pp)
-{
-	for (;;) {
-		struct commit *p = *pp;
-		if (p->object.flags & (TREECHANGE | UNINTERESTING))
-			return 0;
-		if (!p->parents)
-			return -1;
-		*pp = p->parents->item;
-	}
-}
-
-static void rewrite_parents(struct commit *commit)
-{
-	struct commit_list **pp = &commit->parents;
-	while (*pp) {
-		struct commit_list *parent = *pp;
-		if (rewrite_one(&parent->item) < 0) {
-			*pp = parent->next;
-			continue;
-		}
-		pp = &parent->next;
-	}
-}
-
-static int filter_commit(struct commit * commit)
-{
-	if (stop_traversal && (commit->object.flags & BOUNDARY))
-		return STOP;
-	if (commit->object.flags & (UNINTERESTING|SHOWN))
-		return CONTINUE;
-	if (revs.min_age != -1 && (commit->date > revs.min_age))
-		return CONTINUE;
-	if (revs.max_age != -1 && (commit->date < revs.max_age)) {
-		stop_traversal=1;
-		return CONTINUE;
-	}
-	if (no_merges && (commit->parents && commit->parents->next))
-		return CONTINUE;
-	if (revs.paths && revs.dense) {
-		if (!(commit->object.flags & TREECHANGE))
-			return CONTINUE;
-		rewrite_parents(commit);
-	}
-	return DO;
-}
-
-static int process_commit(struct commit * commit)
-{
-	int action=filter_commit(commit);
-
-	if (action == STOP) {
-		return STOP;
-	}
-
-	if (action == CONTINUE) {
-		return CONTINUE;
-	}
-
-	if (revs.max_count != -1 && !revs.max_count--)
-		return STOP;
-
-	show_commit(commit);
-
-	return CONTINUE;
-}
-
 static struct object_list **process_blob(struct blob *blob,
 					 struct object_list **p,
 					 struct name_path *path,
@@ -219,8 +137,7 @@ static void show_commit_list(struct rev_
 
 	while ((commit = get_revision(revs)) != NULL) {
 		p = process_tree(commit->tree, p, NULL, "");
-		if (process_commit(commit) == STOP)
-			break;
+		show_commit(commit);
 	}
 	for (pending = revs->pending_objects; pending; pending = pending->next) {
 		struct object *obj = pending->item;
@@ -416,10 +333,6 @@ int main(int argc, const char **argv)
 				commit_prefix = "commit ";
 			continue;
 		}
-		if (!strncmp(arg, "--no-merges", 11)) {
-			no_merges = 1;
-			continue;
-		}
 		if (!strcmp(arg, "--parents")) {
 			show_parents = 1;
 			continue;
@@ -428,14 +341,6 @@ int main(int argc, const char **argv)
 			bisect_list = 1;
 			continue;
 		}
-		if (!strcmp(arg, "--merge-order")) {
-		        merge_order = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--show-breaks")) {
-			show_breaks = 1;
-			continue;
-		}
 		usage(rev_list_usage);
 
 	}
@@ -456,17 +361,7 @@ int main(int argc, const char **argv)
 	save_commit_buffer = verbose_header;
 	track_object_refs = 0;
 
-	if (!merge_order) {
-		show_commit_list(&revs);
-	} else {
-#ifndef NO_OPENSSL
-		if (sort_list_in_merge_order(list, &process_commit)) {
-			die("merge order sort failed\n");
-		}
-#else
-		die("merge order sort unsupported, OpenSSL not linked");
-#endif
-	}
+	show_commit_list(&revs);
 
 	return 0;
 }
diff --git a/revision.c b/revision.c
index fb728c1..f98fae9 100644
--- a/revision.c
+++ b/revision.c
@@ -381,6 +381,9 @@ static void limit_list(struct rev_info *
 	struct commit_list *newlist = NULL;
 	struct commit_list **p = &newlist;
 
+	if (revs->paths)
+		diff_tree_setup_paths(revs->paths);
+
 	while (list) {
 		struct commit_list *entry = list;
 		struct commit *commit = list->item;
@@ -436,12 +439,13 @@ static void handle_all(struct rev_info *
  * Parse revision information, filling in the "rev_info" structure,
  * and removing the used arguments from the argument list.
  *
- * Returns the number of arguments left ("new argc").
+ * Returns the number of arguments left that weren't recognized
+ * (which are also moved to the head of the argument list)
  */
 int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def)
 {
 	int i, flags, seen_dashdash;
-	const char **unrecognized = argv+1;
+	const char **unrecognized = argv + 1;
 	int left = 1;
 
 	memset(revs, 0, sizeof(*revs));
@@ -525,6 +529,10 @@ int setup_revisions(int argc, const char
 				revs->remove_empty_trees = 1;
 				continue;
 			}
+			if (!strncmp(arg, "--no-merges", 11)) {
+				revs->no_merges = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--objects")) {
 				revs->tag_objects = 1;
 				revs->tree_objects = 1;
@@ -601,14 +609,11 @@ int setup_revisions(int argc, const char
 	}
 	if (revs->paths)
 		revs->limited = 1;
-	*unrecognized = NULL;
 	return left;
 }
 
 void prepare_revision_walk(struct rev_info *revs)
 {
-	if (revs->paths)
-		diff_tree_setup_paths(revs->paths);
 	sort_by_date(&revs->commits);
 	if (revs->limited)
 		limit_list(revs);
@@ -616,11 +621,67 @@ void prepare_revision_walk(struct rev_in
 		sort_in_topological_order(&revs->commits, revs->lifo);
 }
 
+static int rewrite_one(struct commit **pp)
+{
+	for (;;) {
+		struct commit *p = *pp;
+		if (p->object.flags & (TREECHANGE | UNINTERESTING))
+			return 0;
+		if (!p->parents)
+			return -1;
+		*pp = p->parents->item;
+	}
+}
+
+static void rewrite_parents(struct commit *commit)
+{
+	struct commit_list **pp = &commit->parents;
+	while (*pp) {
+		struct commit_list *parent = *pp;
+		if (rewrite_one(&parent->item) < 0) {
+			*pp = parent->next;
+			continue;
+		}
+		pp = &parent->next;
+	}
+}
+
 struct commit *get_revision(struct rev_info *revs)
 {
-	if (!revs->commits)
+	struct commit_list *list = revs->commits;
+	struct commit *commit;
+
+	if (!list)
 		return NULL;
-	return pop_most_recent_commit(&revs->commits, SEEN);
-}
 
+	/* Check the max_count ... */
+	commit = list->item;
+	switch (revs->max_count) {
+	case -1:
+		break;
+	case 0:
+		return NULL;
+	default:
+		revs->max_count--;
+	}
 
+	do {
+		commit = pop_most_recent_commit(&revs->commits, SEEN);
+		if (commit->object.flags & (UNINTERESTING|SHOWN))
+			continue;
+		if (revs->min_age != -1 && (commit->date > revs->min_age))
+			continue;
+		if (revs->max_age != -1 && (commit->date < revs->max_age))
+			return NULL;
+		if (revs->no_merges && commit->parents && commit->parents->next)
+			continue;
+		if (revs->paths && revs->dense) {
+			if (!(commit->object.flags & TREECHANGE))
+				continue;
+			rewrite_parents(commit);
+		}
+		commit->object.flags |= SHOWN;
+		return commit;
+	} while (revs->commits);
+	return NULL;
+}
diff --git a/revision.h b/revision.h
index 0bed3c0..0043c16 100644
--- a/revision.h
+++ b/revision.h
@@ -4,6 +4,7 @@
 #define SEEN		(1u<<0)
 #define UNINTERESTING   (1u<<1)
 #define TREECHANGE	(1u<<2)
+#define SHOWN		(1u<<3)
 
 struct rev_info {
 	/* Starting list */
@@ -16,6 +17,7 @@ struct rev_info {
 
 	/* Traversal flags */
 	unsigned int	dense:1,
+			no_merges:1,
 			remove_empty_trees:1,
 			lifo:1,
 			topo_order:1,

[PATCH 1/2] git-log (internal): add approxidate.

[Junio C Hamano]

Next will be the pretty-print format.

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

---
  Linus Torvalds <torvalds@osdl.org> writes:

  > I didn't add the logic for --before/--after flags, but that should be 
  > pretty trivial, and is independent of this anyway.
  >
  > Perhaps more importantly, I didn't remove the tests that now start 
  > failing, nor did I remove the actual code to do --merge-order ;/

  I've done the janitorial, and have two more on top.  Here is the
  first one.  I'd appreciate comments on the second one.

 revision.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

2eba658eaffdf4c5c9d0767b49e4c27d7281cda6
diff --git a/revision.c b/revision.c
index c84f146..4885871 100644
--- a/revision.c
+++ b/revision.c
@@ -492,6 +492,26 @@ int setup_revisions(int argc, const char
 				revs->limited = 1;
 				continue;
 			}
+			if (!strncmp(arg, "--since=", 8)) {
+				revs->max_age = approxidate(arg + 8);
+				revs->limited = 1;
+				continue;
+			}
+			if (!strncmp(arg, "--after=", 8)) {
+				revs->max_age = approxidate(arg + 8);
+				revs->limited = 1;
+				continue;
+			}
+			if (!strncmp(arg, "--before=", 9)) {
+				revs->min_age = approxidate(arg + 9);
+				revs->limited = 1;
+				continue;
+			}
+			if (!strncmp(arg, "--until=", 8)) {
+				revs->min_age = approxidate(arg + 8);
+				revs->limited = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--all")) {
 				handle_all(revs, flags);
 				continue;
-- 
1.2.3.g9425

[PATCH 2/2] git-log (internal): more options.

[Junio C Hamano]

This ports the following options from rev-list based git-log
implementation:

 * -<n>, -n<n>, and -n <n>.  I am still wondering if we want
    this natively supported by setup_revisions(), which already
    takes --max-count.  We may want to move them in the next
    round.  Also I am not sure if we can get away with not
    setting revs->limited when we set max-count.  The latest
    rev-list.c and revision.c in this series do not, so I left
    them as they are.

 * --pretty and --pretty=<fmt>.

 * --abbrev=<n> and --no-abbrev.

The previous commit already handles time-based limiters
(--since, --until and friends).  The remaining things that
rev-list based git-log happens to do are not useful in a pure
log-viewing purposes, and not ported:

 * --bisect (obviously).

 * --header.  I am actually in favor of doing the NUL
   terminated record format, but rev-list based one always
   passed --pretty, which defeated this option.  Maybe next
   round.

 * --parents.  I do not think of a reason a log viewer wants
   this.  The flag is primarily for feeding squashed history
   via pipe to downstream tools.

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

---

 * comes on top of --since/--until patch which in turn comes on
   top of janitorial "remove merge-order" change.

 git.c      |   70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 rev-list.c |    5 ++--
 revision.h |    1 +
 3 files changed, 72 insertions(+), 4 deletions(-)

4e365d2558356cd091ebf57f689c477a86822d53
diff --git a/git.c b/git.c
index b0da6b1..bf68dac 100644
--- a/git.c
+++ b/git.c
@@ -256,12 +256,80 @@ static int cmd_log(int argc, char **argv
 	struct rev_info rev;
 	struct commit *commit;
 	char *buf = xmalloc(LOGSIZE);
+	static enum cmit_fmt commit_format = CMIT_FMT_DEFAULT;
+	int abbrev = DEFAULT_ABBREV;
+	int show_parents = 0;
+	const char *commit_prefix = "commit ";
 
 	argc = setup_revisions(argc, argv, &rev, "HEAD");
+	while (1 < argc) {
+		char *arg = argv[1];
+		/* accept -<digit>, like traditilnal "head" */
+		if ((*arg == '-') && isdigit(arg[1])) {
+			rev.max_count = atoi(arg + 1);
+		}
+		else if (!strcmp(arg, "-n")) {
+			if (argc < 2)
+				die("-n requires an argument");
+			rev.max_count = atoi(argv[2]);
+			argc--; argv++;
+		}
+		else if (!strncmp(arg,"-n",2)) {
+			rev.max_count = atoi(arg + 2);
+		}
+		else if (!strncmp(arg, "--pretty", 8)) {
+			commit_format = get_commit_format(arg + 8);
+			if (commit_format == CMIT_FMT_ONELINE)
+				commit_prefix = "";
+		}
+		else if (!strcmp(arg, "--parents")) {
+			show_parents = 1;
+		}
+		else if (!strcmp(arg, "--no-abbrev")) {
+			abbrev = 0;
+		}
+		else if (!strncmp(arg, "--abbrev=", 9)) {
+			abbrev = strtoul(arg + 9, NULL, 10);
+			if (abbrev && abbrev < MINIMUM_ABBREV)
+				abbrev = MINIMUM_ABBREV;
+			else if (40 < abbrev)
+				abbrev = 40;
+		}
+		else
+			die("unrecognized argument: %s", arg);
+		argc--; argv++;
+	}
+
 	prepare_revision_walk(&rev);
 	setup_pager();
 	while ((commit = get_revision(&rev)) != NULL) {
-		pretty_print_commit(CMIT_FMT_DEFAULT, commit, ~0, buf, LOGSIZE, 18);
+		printf("%s%s", commit_prefix,
+		       sha1_to_hex(commit->object.sha1));
+		if (show_parents) {
+			struct commit_list *parents = commit->parents;
+			while (parents) {
+				struct object *o = &(parents->item->object);
+				parents = parents->next;
+				if (o->flags & TMP_MARK)
+					continue;
+				printf(" %s", sha1_to_hex(o->sha1));
+				o->flags |= TMP_MARK;
+			}
+			/* TMP_MARK is a general purpose flag that can
+			 * be used locally, but the user should clean
+			 * things up after it is done with them.
+			 */
+			for (parents = commit->parents;
+			     parents;
+			     parents = parents->next)
+				parents->item->object.flags &= ~TMP_MARK;
+		}
+		if (commit_format == CMIT_FMT_ONELINE)
+			putchar(' ');
+		else
+			putchar('\n');
+		pretty_print_commit(commit_format, commit, ~0, buf,
+				    LOGSIZE, abbrev);
 		printf("%s\n", buf);
 	}
 	free(buf);
diff --git a/rev-list.c b/rev-list.c
index 6af8d86..8e4d83e 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -7,10 +7,9 @@
 #include "diff.h"
 #include "revision.h"
 
-/* bits #0-3 in revision.h */
+/* bits #0-4 in revision.h */
 
-#define COUNTED		(1u << 4)
-#define TMP_MARK	(1u << 5) /* for isolated cases; clean after use */
+#define COUNTED		(1u<<5)
 
 static const char rev_list_usage[] =
 "git-rev-list [OPTION] <commit-id>... [ -- paths... ]\n"
diff --git a/revision.h b/revision.h
index 0043c16..31e8f61 100644
--- a/revision.h
+++ b/revision.h
@@ -5,6 +5,7 @@
 #define UNINTERESTING   (1u<<1)
 #define TREECHANGE	(1u<<2)
 #define SHOWN		(1u<<3)
+#define TMP_MARK	(1u<<4) /* for isolated cases; clean after use */
 
 struct rev_info {
 	/* Starting list */
-- 
1.2.3.g9425

Re: [PATCH 2/2] git-log (internal): more options.

[Linus Torvalds]

On Wed, 1 Mar 2006, Junio C Hamano wrote:
>
> This ports the following options from rev-list based git-log
> implementation:
> 
>  * -<n>, -n<n>, and -n <n>.  I am still wondering if we want
>     this natively supported by setup_revisions(), which already
>     takes --max-count.  We may want to move them in the next
>     round.  Also I am not sure if we can get away with not
>     setting revs->limited when we set max-count.  The latest
>     rev-list.c and revision.c in this series do not, so I left
>     them as they are.
> 
>  * --pretty and --pretty=<fmt>.
> 
>  * --abbrev=<n> and --no-abbrev.

Looks good.

I _suspect_ that we want to handle them all in setup_revision(), but I 
wasn't sure, so I left them in rev-list.c originally.

Most helpers that want a list of commits probably want the printing 
options too, and the ones that do not probably simply don't care (ie if 
they silently pass a "--pretty=raw" without it affecting anything, who 
really cares?)

> The previous commit already handles time-based limiters
> (--since, --until and friends).  The remaining things that
> rev-list based git-log happens to do are not useful in a pure
> log-viewing purposes, and not ported:
> 
>  * --bisect (obviously).
> 
>  * --header.  I am actually in favor of doing the NUL
>    terminated record format, but rev-list based one always
>    passed --pretty, which defeated this option.  Maybe next
>    round.
> 
>  * --parents.  I do not think of a reason a log viewer wants
>    this.  The flag is primarily for feeding squashed history
>    via pipe to downstream tools.

I can actually imagine using "--parents" as a way of parsing both the 
commit log and the history. Of course, any such use is likely in a script, 
at which point the script probably doesn't actually want "git log", but 
just a raw "git-rev-list".

After all, the only _real_ difference between "git log" and "git-rev-list" 
is the purely syntactic one (things like defaulting to HEAD in "git log" 
and requiring revisions in git-rev-list), and the use of PAGER.

To me, the question whether a flag would be parsed in the "revision.c" 
library or in the "rev-list.c" binary was more a question of whether that 
flag makes sense for other things than just "git log". 

For example, "git whatchanged" and "git diff" could both use 
setup_revision(), although "git diff" wouldn't actually _walk_ the 
revisions (it would just look at the "revs->commits" list to see what was 
passed in).

"git whatchanged" would obviously take all the same flags "git log" does, 
and "git diff" could take them and just test the values for sanity (ie 
error out if min/max_date is not -1, for example).

"git show" is like a "git-whatchanged" except it wouldn't walk the diffs 
(I considered adding a "--nowalk" option to setup_revisions(), which would 
just suppress the "add_parents_to_list()" entirely)

			Linus

Re: [PATCH 2/2] git-log (internal): more options.

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> Most helpers that want a list of commits probably want the printing 
> options too, and the ones that do not probably simply don't care (ie if 
> they silently pass a "--pretty=raw" without it affecting anything, who 
> really cares?)

Perhaps (meaning, agree in general but not 100% convinced and
haven't made up my mind yet).

> I can actually imagine using "--parents" as a way of parsing both the 
> commit log and the history. Of course, any such use is likely in a script, 
> at which point the script probably doesn't actually want "git log", but 
> just a raw "git-rev-list".

Yes, that is exactly why I did not see why "log viewer" wants --parents.

> To me, the question whether a flag would be parsed in the "revision.c" 
> library or in the "rev-list.c" binary was more a question of whether that 
> flag makes sense for other things than just "git log". 

Good to know we are in agreement (iow, I wasn't totally off the
mark) that revision.c should handle things that are common.
That means:

 * --bisect and --parents are for scripted use only and do not
   concern log viewer, so we would leave them in rev-list.

 * --header is good for anything that shows more than one
   record, so it may be worthwhile to have it in generic.

> For example, "git whatchanged" and "git diff" could both use 
> setup_revision(), although "git diff" wouldn't actually _walk_ the 
> revisions (it would just look at the "revs->commits" list to see what was 
> passed in).
>
> "git whatchanged" would obviously take all the same flags "git log" does, 
> and "git diff" could take them and just test the values for sanity (ie 
> error out if min/max_date is not -1, for example).

Perhaps.

> "git show" is like a "git-whatchanged" except it wouldn't walk the diffs 
> (I considered adding a "--nowalk" option to setup_revisions(), which would 
> just suppress the "add_parents_to_list()" entirely)

Umm.  The current "git show -4" walks and I find the behaviour
useful.  They are the same program with different defaults.

Re: [PATCH 3/3] Tie it all together: "git log"

[Martin Langhoff]

On 3/1/06, Junio C Hamano <junkio@cox.net> wrote:
> I would say we should just rip merge-order out.  Who uses it,
> and why does it not work with topo-order, again?

IIRC archimport uses it, but there's no reason why topo-order wouldn't work.

cheers,


martin

Re: [PATCH 3/3] Tie it all together: "git log"

[Junio C Hamano]

"Martin Langhoff" <martin.langhoff@gmail.com> writes:

> On 3/1/06, Junio C Hamano <junkio@cox.net> wrote:
>> I would say we should just rip merge-order out.  Who uses it,
>> and why does it not work with topo-order, again?
>
> IIRC archimport uses it, but there's no reason why topo-order wouldn't work.

Thanks.  I'll push out a few patches on top of Linus' series,
with s/merge-order/topo-order/ on archimport.