Re: [PATCH v7 5/5] Implement line-history search (git log -L)
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 7, 2012, 17:42 UTC
- Message-ID
- <7vhaunhvc8.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <61a797a048c43d64352ef86a1b224f017e7161ae.1339063659.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> This is a rewrite of much of Bo's work, mainly in an effort to split > it into smaller, easier to understand routines.
You mentioned "splitting" in the cover letter, but it does seem to need a bit more work.
Show 14 quoted lines
> diff --git a/builtin/log.c b/builtin/log.c
> index 906dca4..4a0d5da 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -38,6 +39,12 @@
> ...
> +static int log_line_range_callback(const struct option *option, const char *arg, int unset)
> +{
> + struct line_opt_callback_data *data = option->value;
> + struct line_log_data *r;
> + const char *name_start, *range_arg, *full_path;
> ...
> + ALLOC_GROW(r->args, r->arg_nr+1, r->arg_alloc);
> + r->args[r->arg_nr++] = range_arg;Assignment discards qualifiers from pointer target type; this pointer does not have to be "const char *" perhaps?
Show 6 quoted lines
> @@ -94,15 +135,23 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
> {
> struct userformat_want w;
> int quiet = 0, source = 0;
> + static struct line_opt_callback_data line_cb = {0};
> + static int full_line_diff;Variable unused.
Show 11 quoted lines
> diff --git a/line.c b/line.c
> index a7f33ed..d837bb3 100644
> --- a/line.c
> +++ b/line.c
> @@ -1,6 +1,459 @@
> ...
> +static void diff_ranges_release (struct diff_ranges *diff)
> +{
> + range_set_release(&diff->parent);
> + range_set_release(&diff->target);
> +}Unused.
Show 5 quoted lines
> +static struct line_log_data *
> +search_line_log_data(struct line_log_data *list, const char *path)
> +{
> + return search_line_log_data_1(list, path, NULL);
> +}Unused.
Show 8 quoted lines
> +static void line_log_data_sort (struct line_log_data *list)
> +{
> + struct line_log_data *p = list;
> + while (p) {
> + sort_and_merge_range_set(&p->ranges);
> + p = p->next;
> + }
> +}Unused.
Show 34 quoted lines
> +static int ranges_overlap (struct range *a, struct range *b)
> +{
> + return !(a->end <= b->start || b->end <= a->start);
> +}
> +
> +/*
> + * Given a diff and the set of interesting ranges, determine all hunks
> + * of the diff which touch (overlap) at least one of the interesting
> + * ranges in the target.
> + */
> +static void diff_ranges_filter_touched (struct diff_ranges *out,
> + struct diff_ranges *diff,
> + struct range_set *rs)
> +{
> + int i, j = 0;
> +
> + assert(out->target.nr == 0);
> +
> + for (i = 0; i < diff->target.nr; i++) {
> + while (diff->target.ranges[i].start > rs->ranges[j].end) {
> + j++;
> + if (j == rs->nr)
> + return;
> + }
> + if (ranges_overlap(&diff->target.ranges[i], &rs->ranges[j])) {
> + range_set_append(&out->parent,
> + diff->parent.ranges[i].start,
> + diff->parent.ranges[i].end);
> + range_set_append(&out->target,
> + diff->target.ranges[i].start,
> + diff->target.ranges[i].end);
> + }
> + }
> +}Shouldn't the ranges be merged, not just appended?
Show 11 quoted lines
> diff --git a/log-tree.c b/log-tree.c > index c894930..3fb025d 100644 > --- a/log-tree.c > +++ b/log-tree.c > @@ -809,6 +809,9 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit) > log.parent = NULL; > opt->loginfo = &log; > > + if (opt->line_level_traverse) > + return line_log_print(opt, commit); > +
#include "line.h" is missing from the beginning of this file.
Show 21 quoted lines
> diff --git a/revision.h b/revision.h
> index 6d09550..01902cf 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -92,7 +92,8 @@ struct rev_info {
> cherry_mark:1,
> bisect:1,
> ancestry_path:1,
> - first_parent_only:1;
> + first_parent_only:1,
> + line_level_traverse:1;
>
> /* Diff flags */
> unsigned int diff:1,
> @@ -168,6 +169,9 @@ struct rev_info {
> int count_left;
> int count_right;
> int count_same;
> +
> + /* line level range that we are chasing */
> + struct decoration line_log_data;Good use of decoration.
> }; > > #define REV_TREE_SAME 0