[PATCH] log --graph: draw '>' and '<' with --left-right

[PATCH] log --graph: draw '>' and '<' with --left-right

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/technical/api-history-graph.txt |    2 +-
 graph.c                                       |   16 ++++++++++++++--
 graph.h                                       |    2 +-
 revision.c                                    |    2 +-
 4 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/Documentation/technical/api-history-graph.txt b/Documentation/technical/api-history-graph.txt
index ce1c08e..e955979 100644
--- a/Documentation/technical/api-history-graph.txt
+++ b/Documentation/technical/api-history-graph.txt
@@ -115,7 +115,7 @@ Sample usage
 
 ------------
 struct commit *commit;
-struct git_graph *graph = graph_init();
+struct git_graph *graph = graph_init(opts);
 
 while ((commit = get_revision(opts)) != NULL) {
 	graph_update(graph, commit);
diff --git a/graph.c b/graph.c
index 9d6ed30..85a9ba0 100644
--- a/graph.c
+++ b/graph.c
@@ -55,6 +55,10 @@ struct git_graph {
 	 */
 	struct commit *commit;
 	/*
+	 * For the --left-right option.
+	 */
+	struct rev_info *revs;
+	/*
 	 * The number of parents this commit has.
 	 * (Stored so we don't have to walk over them each time we need
 	 * this number)
@@ -125,10 +129,11 @@ struct git_graph {
 	int *new_mapping;
 };
 
-struct git_graph *graph_init(void)
+struct git_graph *graph_init(struct rev_info *opt)
 {
 	struct git_graph *graph = xmalloc(sizeof(struct git_graph));
 	graph->commit = NULL;
+	graph->revs = opt;
 	graph->num_parents = 0;
 	graph->expansion_row = 0;
 	graph->state = GRAPH_PADDING;
@@ -540,7 +545,14 @@ void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)
 
 		if (col_commit == graph->commit) {
 			seen_this = 1;
-			if (graph->num_parents > 1)
+			if (graph->revs && graph->revs->left_right) {
+				if (graph->commit->object.flags
+						& SYMMETRIC_LEFT)
+					strbuf_addch(sb, '<');
+				else
+					strbuf_addch(sb, '>');
+			}
+			else if (graph->num_parents > 1)
 				strbuf_addch(sb, 'M');
 			else
 				strbuf_addch(sb, '*');
diff --git a/graph.h b/graph.h
index a7748a5..eab4e3d 100644
--- a/graph.h
+++ b/graph.h
@@ -8,7 +8,7 @@ struct git_graph;
  * Create a new struct git_graph.
  * The graph should be freed with graph_release() when no longer needed.
  */
-struct git_graph *graph_init();
+struct git_graph *graph_init(struct rev_info *opt);
 
 /*
  * Destroy a struct git_graph and free associated memory.
diff --git a/revision.c b/revision.c
index 39ceef0..ce0f1ac 100644
--- a/revision.c
+++ b/revision.c
@@ -1206,7 +1206,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 			if (!prefixcmp(arg, "--graph")) {
 				revs->topo_order = 1;
 				revs->rewrite_parents = 1;
-				revs->graph = graph_init();
+				revs->graph = graph_init(revs);
 				continue;
 			}
 			if (!strcmp(arg, "--root")) {
-- 
1.5.5.1.498.gaaa3c0

Re: [PATCH] log --graph: draw '>' and '<' with --left-right

[Johannes Schindelin]

Hi,

On Thu, 22 May 2008, Johannes Schindelin wrote:

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

Sverre just notified me that the commit message is not brilliant.  How 
about this:

-- snip --
When calling "git log --left-right <branch1>...<branch2>", a single "<" or 
">" is shown in front of the commit line, to indicate which branch1 this 
commit comes from, branch1 or branch2.

However, it is easy to miss in the output of "git log --graph ...", since 
the graph still has "*" for regular commits and "M" for merge commits.  So 
imitate gitk, and show the "<" and ">" characters in the graph, too, 
instead of "*" (or "M").
-- snap --

Thanks,
Dscho

Re: [PATCH] log --graph: draw '>' and '<' with --left-right

[Sverre Rabbelier]

On Thu, May 22, 2008 at 2:47 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Sverre just notified me that the commit message is not brilliant.  How
> about this:
>
> -- snip --
> When calling "git log --left-right <branch1>...<branch2>", a single "<" or
> ">" is shown in front of the commit line, to indicate which branch1 this
> commit comes from, branch1 or branch2.
>
> However, it is easy to miss in the output of "git log --graph ...", since
> the graph still has "*" for regular commits and "M" for merge commits.  So
> imitate gitk, and show the "<" and ">" characters in the graph, too,
> instead of "*" (or "M").
> -- snap --

I admit that I didn't know how gitk does things, but yeah, this is a
lot more explanatory and makes sense :).

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] log --graph: draw '>' and '<' with --left-right

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> When calling "git log --left-right <branch1>...<branch2>", a single "<" or 
> ">" is shown in front of the commit line, to indicate which branch1 this 
> commit comes from, branch1 or branch2.
>
> However, it is easy to miss in the output of "git log --graph ...", since 
> the graph still has "*" for regular commits and "M" for merge commits.  So 
> imitate gitk, and show the "<" and ">" characters in the graph, too, 
> instead of "*" (or "M").

This certainly makes it more visible which one is left and which one is
right:

    < commit <205ffa9...
    | Author: Gustaf Hendeby <hendeby@isy.liu.se>
    |
    |     Make git add -n and git -u -n output consistent
    |
    < commit <38ed1d8...
    | Author: Junio C Hamano <gitster@pobox.com>
    |
    |     "git-add -n -u" should not add but just report
    ...

than

    * commit <205ffa9...
    | Author: Gustaf Hendeby <hendeby@isy.liu.se>
    |
    |     Make git add -n and git -u -n output consistent
    |
    * commit <38ed1d8...
    | Author: Junio C Hamano <gitster@pobox.com>
    |
    |     "git-add -n -u" should not add but just report
    |
    ...

But is it just me who now finds the original marker redundant and ugly?
IOW, I wonder if it is better to show this:

    < commit 205ffa9...
    | Author: Gustaf Hendeby <hendeby@isy.liu.se>
    |
    |     Make git add -n and git -u -n output consistent
    |
    < commit 38ed1d8...
    | Author: Junio C Hamano <gitster@pobox.com>
    |
    |     "git-add -n -u" should not add but just report
    ...

This is with --pretty=short, --pretty=oneline makes the issue even more
obvious.

Adam?

Re: [PATCH] log --graph: draw '>' and '<' with --left-right

[Adam Simpkins]

On Sat, May 24, 2008 at 10:27:12PM -0700, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > When calling "git log --left-right <branch1>...<branch2>", a single "<" or 
> > ">" is shown in front of the commit line, to indicate which branch1 this 
> > commit comes from, branch1 or branch2.
> >
> > However, it is easy to miss in the output of "git log --graph ...", since 
> > the graph still has "*" for regular commits and "M" for merge commits.  So 
> > imitate gitk, and show the "<" and ">" characters in the graph, too, 
> > instead of "*" (or "M").
> 
> This certainly makes it more visible which one is left and which one is
> right:
>
> ...
> 
> But is it just me who now finds the original marker redundant and ugly?
> IOW, I wonder if it is better to show this:
> 
>     < commit 205ffa9...
>     | Author: Gustaf Hendeby <hendeby@isy.liu.se>
>     |
>     |     Make git add -n and git -u -n output consistent
>     |
>     < commit 38ed1d8...
>     | Author: Junio C Hamano <gitster@pobox.com>
>     |
>     |     "git-add -n -u" should not add but just report
>     ...
> 
> This is with --pretty=short, --pretty=oneline makes the issue even more
> obvious.

Yes, I agree that the original marker is now redundant, and it looks
better without it.

My recent patch to fix the "log --graph --boundary" behavior also
changes the graph code to print boundary commits as 'o' instead of
'*'.  We could probably also remove the original '-' prefix for
boundary commits when the graph output is enabled.

I was looking at the code, and noticed that it also outputs a '^'
prefix for non-boundary commits that are marked UNINTERESTING.  In
what situations will get_revision() return a commit that is
UNINTERESTING but not BOUNDARY?  I'm asking since the graph code
currently treats these commits as uninteresting, and won't print
branch lines for them.  If there are situations where these commits
are included in the log output, the graph code probably needs to be
updated to handle it.

-- 
Adam Simpkins
adam@adamsimpkins.net

[PATCH] log and rev-list: don't print extra prefixes already shown in the graph

[Adam Simpkins]

With the --graph option, the graph already outputs 'o' instead of '*'
for boundary commits, and '<' or '>' when --left-right is specified.
The extra -/</> prefix in front of the commit itself isn't needed.

(This change also disables the '^' prefix for UNINTERESTING commits.
The graph code currently doesn't print anything special for these
commits, since it assumes no UNINTERESTING, non-BOUNDARY commits are
displayed.  This is potentially a bug if UNINTERESTING non-BOUNDARY
commits can actually be displayed via some code path.)

Signed-off-by: Adam Simpkins <adam@adamsimpkins.net>
---

Here's simple patch to remove the redundant </>/- prefix in the log
message.  As the commit message mentions, I'm not sure if it's doing the
right thing with respect to UNINTERESTING commits, though.

 builtin-rev-list.c |   21 ++++++++++++---------
 log-tree.c         |   41 +++++++++++++++++++++++------------------
 2 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 54d55cc..b474527 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -65,15 +65,18 @@ static void show_commit(struct commit *commit)
 		printf("%lu ", commit->date);
 	if (header_prefix)
 		fputs(header_prefix, stdout);
-	if (commit->object.flags & BOUNDARY)
-		putchar('-');
-	else if (commit->object.flags & UNINTERESTING)
-		putchar('^');
-	else if (revs.left_right) {
-		if (commit->object.flags & SYMMETRIC_LEFT)
-			putchar('<');
-		else
-			putchar('>');
+
+	if (!revs.graph) {
+		if (commit->object.flags & BOUNDARY)
+			putchar('-');
+		else if (commit->object.flags & UNINTERESTING)
+			putchar('^');
+		else if (revs.left_right) {
+			if (commit->object.flags & SYMMETRIC_LEFT)
+				putchar('<');
+			else
+				putchar('>');
+		}
 	}
 	if (revs.abbrev_commit && revs.abbrev)
 		fputs(find_unique_abbrev(commit->object.sha1, revs.abbrev),
diff --git a/log-tree.c b/log-tree.c
index 1474d1f..5505606 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -228,15 +228,17 @@ void show_log(struct rev_info *opt)
 	if (!opt->verbose_header) {
 		graph_show_commit(opt->graph);
 
-		if (commit->object.flags & BOUNDARY)
-			putchar('-');
-		else if (commit->object.flags & UNINTERESTING)
-			putchar('^');
-		else if (opt->left_right) {
-			if (commit->object.flags & SYMMETRIC_LEFT)
-				putchar('<');
-			else
-				putchar('>');
+		if (!opt->graph) {
+			if (commit->object.flags & BOUNDARY)
+				putchar('-');
+			else if (commit->object.flags & UNINTERESTING)
+				putchar('^');
+			else if (opt->left_right) {
+				if (commit->object.flags & SYMMETRIC_LEFT)
+					putchar('<');
+				else
+					putchar('>');
+			}
 		}
 		fputs(diff_unique_abbrev(commit->object.sha1, abbrev_commit), stdout);
 		if (opt->print_parents)
@@ -293,15 +295,18 @@ void show_log(struct rev_info *opt)
 		fputs(diff_get_color_opt(&opt->diffopt, DIFF_COMMIT), stdout);
 		if (opt->commit_format != CMIT_FMT_ONELINE)
 			fputs("commit ", stdout);
-		if (commit->object.flags & BOUNDARY)
-			putchar('-');
-		else if (commit->object.flags & UNINTERESTING)
-			putchar('^');
-		else if (opt->left_right) {
-			if (commit->object.flags & SYMMETRIC_LEFT)
-				putchar('<');
-			else
-				putchar('>');
+
+		if (!opt->graph) {
+			if (commit->object.flags & BOUNDARY)
+				putchar('-');
+			else if (commit->object.flags & UNINTERESTING)
+				putchar('^');
+			else if (opt->left_right) {
+				if (commit->object.flags & SYMMETRIC_LEFT)
+					putchar('<');
+				else
+					putchar('>');
+			}
 		}
 		fputs(diff_unique_abbrev(commit->object.sha1, abbrev_commit),
 		      stdout);
-- 
1.5.5.1.383.g8c56f.dirty