Bo Yang <struggleyb.nku@xxxxxxxxx> writes:
> Since ranges may change in different branches, we should
> make sure we do not pass range to parent until all the
> ranges get 'combined' at the commit which is a split commit.
> So, topological traversing is necessary.
Without explaining what a "split commit" is, "So, ... is necessary"
doesn't really justify this.
>
> Signed-off-by: Bo Yang <struggleyb.nku@xxxxxxxxx>
> ---
> builtin/log.c | 5 ++++-
> line.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> line.h | 2 ++
> 3 files changed, 60 insertions(+), 1 deletions(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 1e90b03..47b386d 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -615,7 +615,10 @@ int cmd_log(int argc, const char **argv, const char *prefix)
> memset(&opt, 0, sizeof(opt));
> opt.def = "HEAD";
> cmd_log_init(argc, argv, prefix, &rev, &opt);
> - return cmd_log_walk(&rev);
> + if (rev.line)
> + return cmd_line_log_walk(&rev);
> + else
> + return cmd_log_walk(&rev);
> }
>
> /* format-patch */
> diff --git a/line.c b/line.c
> index 8813376..5c8f77a 100644
> --- a/line.c
> +++ b/line.c
> @@ -1196,3 +1196,57 @@ static void line_log_flush(struct rev_info *rev, struct commit *c)
> }
> }
>
> +int cmd_line_log_walk(struct rev_info *rev)
> +{
> + struct commit *commit;
> + struct commit_list *list = NULL;
> + struct diff_line_range *r = NULL;
> +
> + if (prepare_revision_walk(rev))
> + die("revision walk prepare failed");
> +
> + list = rev->commits;
> + if (list) {
> + list->item->object.flags |= RANGE_UPDATE;
> + list = list->next;
> + }
> + /* Clear the flags */
> + while (list) {
> + list->item->object.flags &= 0x0;
Just assign "= 0" instead, please.
But more importantly, why is it safe to clear all the flags? Even if your
traversal is limited (i.e. prepare_revision_walk() already walked
everything), at least wouldn't you need to keep flags like SYMMETRIC_LEFT,
UNINTERESTING, BOUNDARY etc. depending on what the end user asked from the
command line?
If you ask prepare_revison_walk()->limit_list() callchain to run a
limited and topo-sorted traversal, wouldn't the resulting revs->list
have commits in the order you need already? Why do you need to have a
custom walker here?
I am not suggesting to roll this logic into prepare_revision_walk() and
limit_list(); there are a lot more than "we need topological order" going
on in this function, and the log message needs to explain what they are.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html