Re: [PATCH v3 2/2] log: add --show-linear-break to help see non-linear history

2014-03-20 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 Option explanation is in rev-list-options.txt. The interaction with -z
 is left undecided.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---

Thanks.

  * Revert back to the old option name --show-linear-break
  * Get rid of saved_linear, use another flag in struct object instead

I cannot offhand say if I like this change or not.  A flag bit is a
scarce and limited resource; commit slabs felt more suited for
implementation of corner case eye-candies.

  * Fix not showing the break bar after a root commit if the dag graph
has multiple roots

I definitely do not like the way a commit-list data structure is
abused to hold a phoney element that points at a NULL with its item
pointer.  Allocate a single bit in revs that says I haven't done
anything yet if you want to catch the first-ness without breaking
what commit_list_insert() and friends are expecting to see---they
never expect to see a NULL asked to be on the list, AFAIK.

  * Make it work with --graph (although I don't really see the point of
using both at the same time)

I do not see the point, either.  I vaguely recall that the previous
iteration refused the combination at the option parser level, which
I think would be the right thing to do.

  * Let the next contributor deal with -z

That is fine.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 2/2] log: add --show-linear-break to help see non-linear history

2014-03-20 Thread Duy Nguyen
On Fri, Mar 21, 2014 at 2:15 AM, Junio C Hamano gits...@pobox.com wrote:
  * Get rid of saved_linear, use another flag in struct object instead

 I cannot offhand say if I like this change or not.  A flag bit is a
 scarce and limited resource; commit slabs felt more suited for
 implementation of corner case eye-candies.

My thinking was like this:

OK an int for a flag is wasteful and Junio suggested that unsigned
char is used. But that still wastes 7 bits. So what if I rename it to
commit_flags and make it usable as a flag storage for other parts as
well? Wait don't we have some flags in struct object#flags. It turns
out we have _7_ flags left that nobody touches. Let's take one. If we
run out of flags in future, we can bring back commit_flags slab,
rearrange the flags and move rarely used ones to commit_flags.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 2/2] log: add --show-linear-break to help see non-linear history

2014-03-19 Thread Nguyễn Thái Ngọc Duy
Option explanation is in rev-list-options.txt. The interaction with -z
is left undecided.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 * Revert back to the old option name --show-linear-break
 * Get rid of saved_linear, use another flag in struct object instead
 * Fix not showing the break bar after a root commit if the dag graph
   has multiple roots
 * Make it work with --graph (although I don't really see the point of
   using both at the same time)
 * Let the next contributor deal with -z

 Documentation/rev-list-options.txt |  7 ++
 log-tree.c |  9 
 object.h   |  2 +-
 revision.c | 45 +++---
 revision.h |  9 +++-
 5 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 9a3da36..b813961 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -758,6 +758,13 @@ This enables parent rewriting, see 'History 
Simplification' below.
 This implies the `--topo-order` option by default, but the
 `--date-order` option may also be specified.
 
+--show-linear-break[=barrier]::
+   When --graph is not used, all history branches are flattened
+   which can make it hard to see that the two consecutive commits
+   do not belong to a linear branch. This option puts a barrier
+   in between them in that case. If `barrier` is specified, it
+   is the string that will be shown instead of the default one.
+
 ifdef::git-rev-list[]
 --count::
Print a number stating how many commits would have been
diff --git a/log-tree.c b/log-tree.c
index 5ce217d..c51a878 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -805,12 +805,21 @@ int log_tree_commit(struct rev_info *opt, struct commit 
*commit)
if (opt-line_level_traverse)
return line_log_print(opt, commit);
 
+   if (opt-track_linear  !opt-linear  !opt-reverse_output_stage) {
+   if (opt-graph)
+   graph_show_padding(opt-graph);
+   else
+   puts(\n);
+   printf(%s\n, opt-break_bar);
+   }
shown = log_tree_diff(opt, commit, log);
if (!shown  opt-loginfo  opt-always_show_header) {
log.parent = NULL;
show_log(opt);
shown = 1;
}
+   if (opt-track_linear  !opt-linear  opt-reverse_output_stage)
+   printf(\n%s\n, opt-break_bar);
opt-loginfo = NULL;
maybe_flush_or_die(stdout, stdout);
return shown;
diff --git a/object.h b/object.h
index 768490b..9ee1959 100644
--- a/object.h
+++ b/object.h
@@ -28,7 +28,7 @@ struct object_array {
 #define TYPE_BITS   3
 /*
  * object flag allocation:
- * revision.h:  0--10
+ * revision.h:  0--10 26
  * fetch-pack.c:0---4
  * walker.c:0-2
  * upload-pack.c: 11-19
diff --git a/revision.c b/revision.c
index 78b5c3a..b9afdce 100644
--- a/revision.c
+++ b/revision.c
@@ -1831,6 +1831,18 @@ static int handle_revision_opt(struct rev_info *revs, 
int argc, const char **arg
revs-notes_opt.use_default_notes = 1;
} else if (!strcmp(arg, --show-signature)) {
revs-show_signature = 1;
+   } else if (!strcmp(arg, --show-linear-break) ||
+  starts_with(arg, --show-linear-break=)) {
+   if (starts_with(arg, --show-linear-break=))
+   revs-break_bar = xstrdup(arg + 20);
+   else
+   revs-break_bar = ..;
+   revs-track_linear = 1;
+   /*
+* make track_linear() not a break bar before the
+* first shown commit.
+*/
+   commit_list_insert(NULL, revs-previous_parents);
} else if (starts_with(arg, --show-notes=) ||
   starts_with(arg, --notes=)) {
struct strbuf buf = STRBUF_INIT;
@@ -2896,6 +2908,22 @@ enum commit_action simplify_commit(struct rev_info 
*revs, struct commit *commit)
return action;
 }
 
+static void track_linear(struct rev_info *revs, struct commit *commit)
+{
+   struct commit_list *p;
+   for (p = revs-previous_parents; p; p = p-next)
+   if (p-item == NULL || /* first commit */
+   !hashcmp(p-item-object.sha1, commit-object.sha1))
+   break;
+   revs-linear = p != NULL;
+   if (revs-reverse) {
+   if (revs-linear)
+   commit-object.flags |= TRACK_LINEAR;
+   }
+   free_commit_list(revs-previous_parents);
+   revs-previous_parents = copy_commit_list(commit-parents);
+}
+
 static struct commit *get_revision_1(struct rev_info *revs)
 {
if (!revs-commits)
@@ -2935,6 +2963,8 @@ static struct