From 1e159833c732d332eefd75d92bee22cf029ca84f Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:34 -0400 Subject: [PATCH 01/16] git-log.txt: place each -L option variation on its own line Standard practice in Git documentation is for each variation of an option (such as: -p / --porcelain) to be placed on its own line in the OPTIONS table. The -L option does not follow suit. It cuddles "-L ,:" and "-L ::", separated by a comma. This is inconsistent and potentially confusing since the comma separating them is typeset the same as the comma in ",". Fix this by placing each variation on its own line. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/git-log.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt index ac2694d04c..fe04e06acb 100644 --- a/Documentation/git-log.txt +++ b/Documentation/git-log.txt @@ -62,7 +62,8 @@ produced by --stat etc. Note that only message is considered, if also a diff is shown its size is not included. --L ,:, -L :::: +-L ,::: +-L :::: Trace the evolution of the line range given by "," (or the funcname regex ) within the . You may From 0ddd47193c225a9f7a658682d4e859771b45ab84 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:35 -0400 Subject: [PATCH 02/16] line-range-format.txt: clarify -L:regex usage form blame/log documentation describes -L option as: -L, -L: and can take one of these forms: * number * /regex/ * +offset or -offset * :regex which is incorrect and confusing since :regex is not one of the valid forms of or ; in fact, it must be -L's lone argument. Clarify by discussing : at the same indentation level as " and ...": -L, -L: and can take one of these forms: * number * /regex/ * +offset or -offset If : is given in place of and ... Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/blame-options.txt | 2 -- Documentation/git-log.txt | 2 -- Documentation/line-range-format.txt | 7 +++---- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt index 4e55b1564e..489032c7a3 100644 --- a/Documentation/blame-options.txt +++ b/Documentation/blame-options.txt @@ -15,8 +15,6 @@ ``-L '' or ``-L ,'' spans from to end of file. ``-L ,'' spans from start of file to . + - and can take one of these forms: - include::line-range-format.txt[] -l:: diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt index fe04e06acb..34097efea7 100644 --- a/Documentation/git-log.txt +++ b/Documentation/git-log.txt @@ -72,8 +72,6 @@ produced by --stat etc. give zero or one positive revision arguments. You can specify this option more than once. + - and can take one of these forms: - include::line-range-format.txt[] :: diff --git a/Documentation/line-range-format.txt b/Documentation/line-range-format.txt index 3e7ce72daa..a1b2f4e089 100644 --- a/Documentation/line-range-format.txt +++ b/Documentation/line-range-format.txt @@ -1,3 +1,5 @@ + and can take one of these forms: + - number + If or is a number, it specifies an @@ -15,11 +17,8 @@ starting at the line given by . + This is only valid for and will specify a number of lines before or after the line given by . -+ -- :regex + -If the option's argument is of the form :regex, it denotes the range +If ``:'' is given in place of and , it denotes the range from the first funcname line that matches , up to the next funcname line. -+ From c0babbe6953a8a085f270e98e9a94d2a6f3a381b Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:36 -0400 Subject: [PATCH 03/16] range-set: publish API for re-use by git-blame -L git-blame is slated to accept multiple -L ranges. git-log already accepts multiple -L's but its implementation of range-set, which organizes and normalizes -L ranges, is private. Publish the small subset of range-set API which is needed for git-blame multiple -L support. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- line-log.c | 10 +++++----- line-log.h | 12 ++++++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/line-log.c b/line-log.c index 1c3ac8dccd..bdadf35a1e 100644 --- a/line-log.c +++ b/line-log.c @@ -23,7 +23,7 @@ static void range_set_grow(struct range_set *rs, size_t extra) /* Either initialization would be fine */ #define RANGE_SET_INIT {0} -static void range_set_init(struct range_set *rs, size_t prealloc) +void range_set_init(struct range_set *rs, size_t prealloc) { rs->alloc = rs->nr = 0; rs->ranges = NULL; @@ -31,7 +31,7 @@ static void range_set_init(struct range_set *rs, size_t prealloc) range_set_grow(rs, prealloc); } -static void range_set_release(struct range_set *rs) +void range_set_release(struct range_set *rs) { free(rs->ranges); rs->alloc = rs->nr = 0; @@ -56,7 +56,7 @@ static void range_set_move(struct range_set *dst, struct range_set *src) } /* tack on a _new_ range _at the end_ */ -static void range_set_append_unsafe(struct range_set *rs, long a, long b) +void range_set_append_unsafe(struct range_set *rs, long a, long b) { assert(a <= b); range_set_grow(rs, 1); @@ -65,7 +65,7 @@ static void range_set_append_unsafe(struct range_set *rs, long a, long b) rs->nr++; } -static void range_set_append(struct range_set *rs, long a, long b) +void range_set_append(struct range_set *rs, long a, long b) { assert(rs->nr == 0 || rs->ranges[rs->nr-1].end <= a); range_set_append_unsafe(rs, a, b); @@ -107,7 +107,7 @@ static void range_set_check_invariants(struct range_set *rs) * In-place pass of sorting and merging the ranges in the range set, * to establish the invariants when we get the ranges from the user */ -static void sort_and_merge_range_set(struct range_set *rs) +void sort_and_merge_range_set(struct range_set *rs) { int i; int o = 0; /* output cursor */ diff --git a/line-log.h b/line-log.h index 8bea45fd78..a9212d84e4 100644 --- a/line-log.h +++ b/line-log.h @@ -25,6 +25,18 @@ struct diff_ranges { struct range_set target; }; +extern void range_set_init(struct range_set *, size_t prealloc); +extern void range_set_release(struct range_set *); +/* Range includes start; excludes end */ +extern void range_set_append_unsafe(struct range_set *, long start, long end); +/* New range must begin at or after end of last added range */ +extern void range_set_append(struct range_set *, long start, long end); +/* + * In-place pass of sorting and merging the ranges in the range set, + * to sort and make the ranges disjoint. + */ +extern void sort_and_merge_range_set(struct range_set *); + /* Linked list of interesting files and their associated ranges. The * list must be kept sorted by path. * From 753935749f21b10b306bc81559bba2912a7128a6 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:37 -0400 Subject: [PATCH 04/16] blame: inline one-line function into its lone caller As of 25ed3412 (Refactor parse_loc; 2013-03-28), blame.c:prepare_blame_range() became effectively a one-line function which merely passes its arguments along to another function. This indirection does not bring clarity to the code. Simplify by inlining prepare_blame_range() into its lone caller. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/blame.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index e70b089a67..9db01b0172 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -1937,18 +1937,6 @@ static const char *add_prefix(const char *prefix, const char *path) return prefix_path(prefix, prefix ? strlen(prefix) : 0, path); } -/* - * Parsing of -L option - */ -static void prepare_blame_range(struct scoreboard *sb, - const char *bottomtop, - long lno, - long *bottom, long *top) -{ - if (parse_range_arg(bottomtop, nth_line_cb, sb, lno, bottom, top, sb->path)) - usage(blame_usage); -} - static int git_blame_config(const char *var, const char *value, void *cb) { if (!strcmp(var, "blame.showroot")) { @@ -2493,8 +2481,9 @@ parse_done: lno = prepare_lines(&sb); bottom = top = 0; - if (bottomtop) - prepare_blame_range(&sb, bottomtop, lno, &bottom, &top); + if (bottomtop && parse_range_arg(bottomtop, nth_line_cb, &sb, lno, + &bottom, &top, sb.path)) + usage(blame_usage); if (lno < top || ((lno || bottom) && lno < bottom)) die("file %s has only %lu lines", path, lno); if (bottom < 1) From 58dbfa2e59a11b3dd060d69788af4c3decc7ee22 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:38 -0400 Subject: [PATCH 05/16] blame: accept multiple -L ranges git-blame accepts only a single -L option or none. Clients requiring blame information for multiple disjoint ranges are therefore forced either to invoke git-blame multiple times, once for each range, or only once with no -L option to cover the entire file, both of which can be costly. Teach git-blame to accept multiple -L ranges. Overlapping and out-of-order ranges are accepted. In this patch, the X in -LX,Y is absolute (for instance, /RE/ patterns search from line 1), and Y is relative to X. Follow-up patches provide more flexibility over how X is anchored. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/blame.c | 77 +++++++++++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 9db01b0172..2f4d9e2d75 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -22,6 +22,7 @@ #include "utf8.h" #include "userdiff.h" #include "line-range.h" +#include "line-log.h" static char blame_usage[] = N_("git blame [options] [rev-opts] [rev] [--] file"); @@ -2233,29 +2234,18 @@ static int blame_move_callback(const struct option *option, const char *arg, int return 0; } -static int blame_bottomtop_callback(const struct option *option, const char *arg, int unset) -{ - const char **bottomtop = option->value; - if (!arg) - return -1; - if (*bottomtop) - die("More than one '-L n,m' option given"); - *bottomtop = arg; - return 0; -} - int cmd_blame(int argc, const char **argv, const char *prefix) { struct rev_info revs; const char *path; struct scoreboard sb; struct origin *o; - struct blame_entry *ent; - long dashdash_pos, bottom, top, lno; + struct blame_entry *ent = NULL; + long dashdash_pos, lno; const char *final_commit_name = NULL; enum object_type type; - static const char *bottomtop = NULL; + static struct string_list range_list; static int output_option = 0, opt = 0; static int show_stats = 0; static const char *revs_file = NULL; @@ -2281,13 +2271,15 @@ int cmd_blame(int argc, const char **argv, const char *prefix) OPT_STRING(0, "contents", &contents_from, N_("file"), N_("Use 's contents as the final image")), { OPTION_CALLBACK, 'C', NULL, &opt, N_("score"), N_("Find line copies within and across files"), PARSE_OPT_OPTARG, blame_copy_callback }, { OPTION_CALLBACK, 'M', NULL, &opt, N_("score"), N_("Find line movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback }, - OPT_CALLBACK('L', NULL, &bottomtop, N_("n,m"), N_("Process only line range n,m, counting from 1"), blame_bottomtop_callback), + OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")), OPT__ABBREV(&abbrev), OPT_END() }; struct parse_opt_ctx_t ctx; int cmd_is_annotate = !strcmp(argv[0], "annotate"); + struct range_set ranges; + unsigned int range_i; git_config(git_blame_config, NULL); init_revisions(&revs, NULL); @@ -2480,23 +2472,46 @@ parse_done: num_read_blob++; lno = prepare_lines(&sb); - bottom = top = 0; - if (bottomtop && parse_range_arg(bottomtop, nth_line_cb, &sb, lno, - &bottom, &top, sb.path)) - usage(blame_usage); - if (lno < top || ((lno || bottom) && lno < bottom)) - die("file %s has only %lu lines", path, lno); - if (bottom < 1) - bottom = 1; - if (top < 1) - top = lno; - bottom--; + if (lno && !range_list.nr) + string_list_append(&range_list, xstrdup("1")); - ent = xcalloc(1, sizeof(*ent)); - ent->lno = bottom; - ent->num_lines = top - bottom; - ent->suspect = o; - ent->s_lno = bottom; + range_set_init(&ranges, range_list.nr); + for (range_i = 0; range_i < range_list.nr; ++range_i) { + long bottom, top; + if (parse_range_arg(range_list.items[range_i].string, + nth_line_cb, &sb, lno, + &bottom, &top, sb.path)) + usage(blame_usage); + if (lno < top || ((lno || bottom) && lno < bottom)) + die("file %s has only %lu lines", path, lno); + if (bottom < 1) + bottom = 1; + if (top < 1) + top = lno; + bottom--; + range_set_append_unsafe(&ranges, bottom, top); + } + sort_and_merge_range_set(&ranges); + + for (range_i = ranges.nr; range_i > 0; --range_i) { + const struct range *r = &ranges.ranges[range_i - 1]; + long bottom = r->start; + long top = r->end; + struct blame_entry *next = ent; + ent = xcalloc(1, sizeof(*ent)); + ent->lno = bottom; + ent->num_lines = top - bottom; + ent->suspect = o; + ent->s_lno = bottom; + ent->next = next; + if (next) + next->prev = ent; + origin_incref(o); + } + origin_decref(o); + + range_set_release(&ranges); + string_list_clear(&range_list, 0); sb.ent = ent; sb.path = path; From 91b5494e18965e51c924ca55aaa27e94ebd82dd2 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:39 -0400 Subject: [PATCH 06/16] t8001/t8002: blame: add tests of multiple -L options Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/annotate-tests.sh | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index ce5b8ed304..77083d9dde 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -271,6 +271,38 @@ test_expect_success 'blame -L ,Y (Y > nlines)' ' test_must_fail $PROG -L,12345 file ' +test_expect_success 'blame -L multiple (disjoint)' ' + check_count -L2,3 -L6,7 A 1 B1 1 B2 1 "A U Thor" 1 +' + +test_expect_success 'blame -L multiple (disjoint: unordered)' ' + check_count -L6,7 -L2,3 A 1 B1 1 B2 1 "A U Thor" 1 +' + +test_expect_success 'blame -L multiple (adjacent)' ' + check_count -L2,3 -L4,5 A 1 B 1 B2 1 D 1 +' + +test_expect_success 'blame -L multiple (adjacent: unordered)' ' + check_count -L4,5 -L2,3 A 1 B 1 B2 1 D 1 +' + +test_expect_success 'blame -L multiple (overlapping)' ' + check_count -L2,4 -L3,5 A 1 B 1 B2 1 D 1 +' + +test_expect_success 'blame -L multiple (overlapping: unordered)' ' + check_count -L3,5 -L2,4 A 1 B 1 B2 1 D 1 +' + +test_expect_success 'blame -L multiple (superset/subset)' ' + check_count -L2,8 -L3,5 A 1 B 1 B1 1 B2 1 C 1 D 1 "A U Thor" 1 +' + +test_expect_success 'blame -L multiple (superset/subset: unordered)' ' + check_count -L3,5 -L2,8 A 1 B 1 B1 1 B2 1 C 1 D 1 "A U Thor" 1 +' + test_expect_success 'setup -L :regex' ' tr Q "\\t" >hello.c <<-\EOF && int main(int argc, const char *argv[]) From 5bd9b79a2017daad1d600978b75c714eef74cac0 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:40 -0400 Subject: [PATCH 07/16] blame: document multiple -L support Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/blame-options.txt | 8 +++++--- Documentation/git-blame.txt | 10 +++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt index 489032c7a3..0cebc4f692 100644 --- a/Documentation/blame-options.txt +++ b/Documentation/blame-options.txt @@ -11,9 +11,11 @@ -L ,:: -L ::: - Annotate only the given line range. and are optional. - ``-L '' or ``-L ,'' spans from to end of file. - ``-L ,'' spans from start of file to . + Annotate only the given line range. May be specified multiple times. + Overlapping ranges are allowed. ++ + and are optional. ``-L '' or ``-L ,'' spans from + to end of file. ``-L ,'' spans from start of file to . + include::line-range-format.txt[] diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt index 6cea7f1ce1..f2c85cc633 100644 --- a/Documentation/git-blame.txt +++ b/Documentation/git-blame.txt @@ -9,7 +9,7 @@ SYNOPSIS -------- [verse] 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental] - [-L n,m | -L :fn] [-S ] [-M] [-C] [-C] [-C] [--since=] + [-L ] [-S ] [-M] [-C] [-C] [-C] [--since=] [--abbrev=] [ | --contents | --reverse ] [--] DESCRIPTION @@ -18,7 +18,8 @@ DESCRIPTION Annotates each line in the given file with information from the revision which last modified the line. Optionally, start annotating from the given revision. -The command can also limit the range of lines annotated. +When specified one or more times, `-L` restricts annotation to the requested +lines. The origin of lines is automatically followed across whole-file renames (currently there is no option to turn the rename-following @@ -130,7 +131,10 @@ SPECIFYING RANGES Unlike 'git blame' and 'git annotate' in older versions of git, the extent of the annotation can be limited to both line ranges and revision -ranges. When you are interested in finding the origin for +ranges. The `-L` option, which limits annotation to a range of lines, may be +specified multiple times. + +When you are interested in finding the origin for lines 40-60 for file `foo`, you can use the `-L` option like so (they mean the same thing -- both ask for 21 lines starting at line 40): From 815834e9aa6148b7815b9aea7db5d44640a4383a Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:41 -0400 Subject: [PATCH 08/16] line-range: teach -L/RE/ to search relative to anchor point Range specification -L/RE/ for blame/log unconditionally begins searching at line one. Mailing list discussion [1] suggests that, in the presence of multiple -L options, -L/RE/ should search relative to the endpoint of the previous -L range, if any. Teach the parsing machinery underlying blame's and log's -L options to accept a start point for -L/RE/ searches. Follow-up patches will upgrade blame and log to take advantage of this ability. [1]: http://thread.gmane.org/gmane.comp.version-control.git/229755/focus=229966 Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/blame.c | 2 +- line-log.c | 2 +- line-range.c | 30 ++++++++++++++++++++++++++---- line-range.h | 5 ++++- 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 2f4d9e2d75..7b084d8445 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -2479,7 +2479,7 @@ parse_done: for (range_i = 0; range_i < range_list.nr; ++range_i) { long bottom, top; if (parse_range_arg(range_list.items[range_i].string, - nth_line_cb, &sb, lno, + nth_line_cb, &sb, lno, 1, &bottom, &top, sb.path)) usage(blame_usage); if (lno < top || ((lno || bottom) && lno < bottom)) diff --git a/line-log.c b/line-log.c index bdadf35a1e..38f827ba9a 100644 --- a/line-log.c +++ b/line-log.c @@ -591,7 +591,7 @@ parse_lines(struct commit *commit, const char *prefix, struct string_list *args) cb_data.line_ends = ends; if (parse_range_arg(range_part, nth_line, &cb_data, - lines, &begin, &end, + lines, 1, &begin, &end, full_name)) die("malformed -L argument '%s'", range_part); if (lines < end || ((lines || begin) && lines < begin)) diff --git a/line-range.c b/line-range.c index 69e8d6b6c0..bbf3c0f448 100644 --- a/line-range.c +++ b/line-range.c @@ -6,6 +6,18 @@ /* * Parse one item in the -L option + * + * 'begin' is applicable only to relative range anchors. Absolute anchors + * ignore this value. + * + * When parsing "-L A,B", parse_loc() is called once for A and once for B. + * + * When parsing A, 'begin' must be a negative number, the absolute value of + * which is the line at which relative start-of-range anchors should be + * based. Beginning of file is represented by -1. + * + * When parsing B, 'begin' must be the positive line number immediately + * following the line computed for 'A'. */ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line, void *data, long lines, long begin, long *ret) @@ -46,6 +58,10 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line, *ret = num; return term; } + + if (begin < 0) + begin = -begin; + if (spec[0] != '/') return spec; @@ -85,7 +101,8 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line, else { char errbuf[1024]; regerror(reg_error, ®exp, errbuf, 1024); - die("-L parameter '%s': %s", spec + 1, errbuf); + die("-L parameter '%s' starting at line %ld: %s", + spec + 1, begin + 1, errbuf); } } @@ -210,11 +227,16 @@ static const char *parse_range_funcname(const char *arg, nth_line_fn_t nth_line_ } int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb, - void *cb_data, long lines, long *begin, long *end, - const char *path) + void *cb_data, long lines, long anchor, + long *begin, long *end, const char *path) { *begin = *end = 0; + if (anchor < 1) + anchor = 1; + if (anchor > lines) + anchor = lines + 1; + if (*arg == ':') { arg = parse_range_funcname(arg, nth_line_cb, cb_data, lines, begin, end, path); if (!arg || *arg) @@ -222,7 +244,7 @@ int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb, return 0; } - arg = parse_loc(arg, nth_line_cb, cb_data, lines, 1, begin); + arg = parse_loc(arg, nth_line_cb, cb_data, lines, -anchor, begin); if (*arg == ',') arg = parse_loc(arg + 1, nth_line_cb, cb_data, lines, *begin + 1, end); diff --git a/line-range.h b/line-range.h index ae3d0123b4..83ba3c25e8 100644 --- a/line-range.h +++ b/line-range.h @@ -9,6 +9,9 @@ * line 'lno' inside the 'cb_data'. The caller is expected to already * have a suitable map at hand to make this a constant-time lookup. * + * 'anchor' is the 1-based line at which relative range specifications + * should be anchored. Absolute ranges are unaffected by this value. + * * Returns 0 in case of success and -1 if there was an error. The * actual range is stored in *begin and *end. The counting starts * at 1! In case of error, the caller should show usage message. @@ -18,7 +21,7 @@ typedef const char *(*nth_line_fn_t)(void *data, long lno); extern int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb, - void *cb_data, long lines, + void *cb_data, long lines, long anchor, long *begin, long *end, const char *path); From 52f4d1264854485bfd50afeeed1933a3f9e05c96 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:42 -0400 Subject: [PATCH 09/16] blame: teach -L/RE/ to search from end of previous -L range Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- builtin/blame.c | 5 ++++- t/annotate-tests.sh | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/builtin/blame.c b/builtin/blame.c index 7b084d8445..1bf8056f6b 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -2280,6 +2280,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix) int cmd_is_annotate = !strcmp(argv[0], "annotate"); struct range_set ranges; unsigned int range_i; + long anchor; git_config(git_blame_config, NULL); init_revisions(&revs, NULL); @@ -2475,11 +2476,12 @@ parse_done: if (lno && !range_list.nr) string_list_append(&range_list, xstrdup("1")); + anchor = 1; range_set_init(&ranges, range_list.nr); for (range_i = 0; range_i < range_list.nr; ++range_i) { long bottom, top; if (parse_range_arg(range_list.items[range_i].string, - nth_line_cb, &sb, lno, 1, + nth_line_cb, &sb, lno, anchor, &bottom, &top, sb.path)) usage(blame_usage); if (lno < top || ((lno || bottom) && lno < bottom)) @@ -2490,6 +2492,7 @@ parse_done: top = lno; bottom--; range_set_append_unsafe(&ranges, bottom, top); + anchor = top + 1; } sort_and_merge_range_set(&ranges); diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index 77083d9dde..b963d36325 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -303,6 +303,26 @@ test_expect_success 'blame -L multiple (superset/subset: unordered)' ' check_count -L3,5 -L2,8 A 1 B 1 B1 1 B2 1 C 1 D 1 "A U Thor" 1 ' +test_expect_success 'blame -L /RE/ (relative)' ' + check_count -L3,3 -L/fox/ B1 1 B2 1 C 1 D 1 "A U Thor" 1 +' + +test_expect_success 'blame -L /RE/ (relative: no preceding range)' ' + check_count -L/dog/ A 1 B 1 B1 1 B2 1 C 1 D 1 "A U Thor" 1 +' + +test_expect_success 'blame -L /RE/ (relative: adjacent)' ' + check_count -L1,1 -L/dog/,+1 A 1 E 1 +' + +test_expect_success 'blame -L /RE/ (relative: not found)' ' + test_must_fail $PROG -L4,4 -L/dog/ file +' + +test_expect_success 'blame -L /RE/ (relative: end-of-file)' ' + test_must_fail $PROG -L, -L/$/ file +' + test_expect_success 'setup -L :regex' ' tr Q "\\t" >hello.c <<-\EOF && int main(int argc, const char *argv[]) From 3e0d79dbe3064cf065b6518520dd1489406200a9 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:43 -0400 Subject: [PATCH 10/16] log: teach -L/RE/ to search from end of previous -L range This is complicated slightly by having to remember the previous -L range for each file specified via -L:file. The existing implementation coalesces ranges for each file as each -L is parsed which makes it impossible to refer back to the previous -L range for any particular file. Re-implement to instead store each file's set of -L ranges verbatim, and then coalesce the ranges in a post-processing step. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- line-log.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/line-log.c b/line-log.c index 38f827ba9a..d40c79dc2b 100644 --- a/line-log.c +++ b/line-log.c @@ -291,7 +291,6 @@ static void line_log_data_insert(struct line_log_data **list, if (p) { range_set_append_unsafe(&p->ranges, begin, end); - sort_and_merge_range_set(&p->ranges); free(path); return; } @@ -299,7 +298,6 @@ static void line_log_data_insert(struct line_log_data **list, p = xcalloc(1, sizeof(struct line_log_data)); p->path = path; range_set_append(&p->ranges, begin, end); - sort_and_merge_range_set(&p->ranges); if (ip) { p->next = ip->next; ip->next = p; @@ -566,12 +564,14 @@ parse_lines(struct commit *commit, const char *prefix, struct string_list *args) struct nth_line_cb cb_data; struct string_list_item *item; struct line_log_data *ranges = NULL; + struct line_log_data *p; for_each_string_list_item(item, args) { const char *name_part, *range_part; char *full_name; struct diff_filespec *spec; long begin = 0, end = 0; + long anchor; name_part = skip_range_arg(item->string); if (!name_part || *name_part != ':' || !name_part[1]) @@ -590,8 +590,14 @@ parse_lines(struct commit *commit, const char *prefix, struct string_list *args) cb_data.lines = lines; cb_data.line_ends = ends; + p = search_line_log_data(ranges, full_name, NULL); + if (p && p->ranges.nr) + anchor = p->ranges.ranges[p->ranges.nr - 1].end + 1; + else + anchor = 1; + if (parse_range_arg(range_part, nth_line, &cb_data, - lines, 1, &begin, &end, + lines, anchor, &begin, &end, full_name)) die("malformed -L argument '%s'", range_part); if (lines < end || ((lines || begin) && lines < begin)) @@ -608,6 +614,9 @@ parse_lines(struct commit *commit, const char *prefix, struct string_list *args) ends = NULL; } + for (p = ranges; p; p = p->next) + sort_and_merge_range_set(&p->ranges); + return ranges; } From 0bc2cdd5507ea4acad06c6d872245e4a3fd2a4b6 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:44 -0400 Subject: [PATCH 11/16] line-range-format.txt: document -L/RE/ relative search Option -L/RE/ of blame/log now searches relative to the previous -L range, if any. Document this. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/line-range-format.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/line-range-format.txt b/Documentation/line-range-format.txt index a1b2f4e089..42d74f75ad 100644 --- a/Documentation/line-range-format.txt +++ b/Documentation/line-range-format.txt @@ -9,7 +9,9 @@ absolute line number (lines count from 1). - /regex/ + This form will use the first line matching the given -POSIX regex. If is a regex, it will search +POSIX regex. If is a regex, it will search from the end of +the previous `-L` range, if any, otherwise from the start of file. +If is a regex, it will search starting at the line given by . + From a6ac5f9864958f65269d8d58a049324403b039fd Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:45 -0400 Subject: [PATCH 12/16] line-range: teach -L^/RE/ to search from start of file The -L/RE/ option of blame/log searches from the end of the previous -L range, if any. Add new notation -L^/RE/ to override this behavior and search from start of file. The new ^/RE/ syntax is valid only as the argument of -L,. The argument, as usual, is relative to . Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/line-range-format.txt | 1 + line-range.c | 10 ++++++++-- t/annotate-tests.sh | 21 +++++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/Documentation/line-range-format.txt b/Documentation/line-range-format.txt index 42d74f75ad..cf84417060 100644 --- a/Documentation/line-range-format.txt +++ b/Documentation/line-range-format.txt @@ -11,6 +11,7 @@ absolute line number (lines count from 1). This form will use the first line matching the given POSIX regex. If is a regex, it will search from the end of the previous `-L` range, if any, otherwise from the start of file. +If is ``^/regex/'', it will search from the start of file. If is a regex, it will search starting at the line given by . + diff --git a/line-range.c b/line-range.c index bbf3c0f448..70484899ac 100644 --- a/line-range.c +++ b/line-range.c @@ -59,8 +59,14 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line, return term; } - if (begin < 0) - begin = -begin; + if (begin < 0) { + if (spec[0] != '^') + begin = -begin; + else { + begin = 1; + spec++; + } + } if (spec[0] != '/') return spec; diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index b963d36325..5a7d7c72d1 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -323,6 +323,23 @@ test_expect_success 'blame -L /RE/ (relative: end-of-file)' ' test_must_fail $PROG -L, -L/$/ file ' +test_expect_success 'blame -L ^/RE/ (absolute)' ' + check_count -L3,3 -L^/dog/,+2 A 1 B2 1 +' + +test_expect_success 'blame -L ^/RE/ (absolute: no preceding range)' ' + check_count -L^/dog/,+2 A 1 B2 1 +' + +test_expect_success 'blame -L ^/RE/ (absolute: not found)' ' + test_must_fail $PROG -L4,4 -L^/tambourine/ file +' + +test_expect_success 'blame -L ^/RE/ (absolute: end-of-file)' ' + n=$(expr $(wc -l hello.c <<-\EOF && int main(int argc, const char *argv[]) @@ -464,3 +481,7 @@ test_expect_success 'blame -L X,+N (non-numeric N)' ' test_expect_success 'blame -L X,-N (non-numeric N)' ' test_must_fail $PROG -L1,-N file ' + +test_expect_success 'blame -L ,^/RE/' ' + test_must_fail $PROG -L1,^/99/ file +' From 1ce761a524e34f2d629759cb57c67d13acbe4a7a Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:46 -0400 Subject: [PATCH 13/16] line-range: teach -L:RE to search from end of previous -L range For consistency with -L/RE/, teach -L:RE to search relative to the end of the previous -L range, if any. The new behavior invalidates one test in t4211 which assumes that -L:RE begins searching at start of file. This test will be resurrected in a follow-up patch which teaches -L:RE how to override the default relative search behavior. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/line-range-format.txt | 3 ++- line-range.c | 12 +++++++----- t/annotate-tests.sh | 16 ++++++++++++++++ t/t4211-line-log.sh | 1 - 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/Documentation/line-range-format.txt b/Documentation/line-range-format.txt index cf84417060..469d80b242 100644 --- a/Documentation/line-range-format.txt +++ b/Documentation/line-range-format.txt @@ -24,4 +24,5 @@ of lines before or after the line given by . + If ``:'' is given in place of and , it denotes the range from the first funcname line that matches , up to the next -funcname line. +funcname line. ``:'' searches from the end of the previous `-L` range, +if any, otherwise from the start of file. diff --git a/line-range.c b/line-range.c index 70484899ac..4bae4bff21 100644 --- a/line-range.c +++ b/line-range.c @@ -161,7 +161,7 @@ static const char *find_funcname_matching_regexp(xdemitconf_t *xecfg, const char } static const char *parse_range_funcname(const char *arg, nth_line_fn_t nth_line_cb, - void *cb_data, long lines, long *begin, long *end, + void *cb_data, long lines, long anchor, long *begin, long *end, const char *path) { char *pattern; @@ -187,7 +187,8 @@ static const char *parse_range_funcname(const char *arg, nth_line_fn_t nth_line_ pattern = xstrndup(arg+1, term-(arg+1)); - start = nth_line_cb(cb_data, 0); + anchor--; /* input is in human terms */ + start = nth_line_cb(cb_data, anchor); drv = userdiff_find_by_path(path); if (drv && drv->funcname.pattern) { @@ -205,7 +206,8 @@ static const char *parse_range_funcname(const char *arg, nth_line_fn_t nth_line_ p = find_funcname_matching_regexp(xecfg, (char*) start, ®exp); if (!p) - die("-L parameter '%s': no match", pattern); + die("-L parameter '%s' starting at line %ld: no match", + pattern, anchor + 1); *begin = 0; while (p > nth_line_cb(cb_data, *begin)) (*begin)++; @@ -244,7 +246,7 @@ int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb, anchor = lines + 1; if (*arg == ':') { - arg = parse_range_funcname(arg, nth_line_cb, cb_data, lines, begin, end, path); + arg = parse_range_funcname(arg, nth_line_cb, cb_data, lines, anchor, begin, end, path); if (!arg || *arg) return -1; return 0; @@ -269,7 +271,7 @@ int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb, const char *skip_range_arg(const char *arg) { if (*arg == ':') - return parse_range_funcname(arg, NULL, NULL, 0, NULL, NULL, NULL); + return parse_range_funcname(arg, NULL, NULL, 0, 0, NULL, NULL, NULL); arg = parse_loc(arg, NULL, NULL, 0, -1, NULL); diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index 5a7d7c72d1..4f7d6baef5 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -382,6 +382,22 @@ test_expect_success 'blame -L :nomatch' ' test_must_fail $PROG -L:nomatch hello.c ' +test_expect_success 'blame -L :RE (relative)' ' + check_count -f hello.c -L3,3 -L:ma.. F 1 H 4 +' + +test_expect_success 'blame -L :RE (relative: no preceding range)' ' + check_count -f hello.c -L:ma.. F 4 G 1 +' + +test_expect_success 'blame -L :RE (relative: not found)' ' + test_must_fail $PROG -L3,3 -L:tambourine hello.c +' + +test_expect_success 'blame -L :RE (relative: end-of-file)' ' + test_must_fail $PROG -L, -L:main hello.c +' + test_expect_success 'setup incremental' ' ( GIT_AUTHOR_NAME=I && diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index b01b3ddebb..8ba2d511e5 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -48,7 +48,6 @@ canned_test "-M -L '/long f/,/^}/:b.c' move-support" move-support-f canned_test "-M -L ':f:b.c' parallel-change" parallel-change-f-to-main canned_test "-L 4,12:a.c -L :main:a.c simple" multiple -canned_test "-L 4,18:a.c -L :main:a.c simple" multiple-overlapping canned_test "-L :main:a.c -L 4,18:a.c simple" multiple-overlapping canned_test "-L 4:a.c -L 8,12:a.c simple" multiple-superset canned_test "-L 8,12:a.c -L 4:a.c simple" multiple-superset From 215e76c7ff8be46b8206c45aed3b6ec29069d4fc Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:47 -0400 Subject: [PATCH 14/16] line-range: teach -L^:RE to search from start of file The -L:RE option of blame/log searches from the end of the previous -L range, if any. Add new notation -L^:RE to override this behavior and search from start of file. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/line-range-format.txt | 1 + line-range.c | 9 +++++++-- t/annotate-tests.sh | 17 +++++++++++++++++ t/t4211-line-log.sh | 1 + 4 files changed, 26 insertions(+), 2 deletions(-) diff --git a/Documentation/line-range-format.txt b/Documentation/line-range-format.txt index 469d80b242..d7f26039ca 100644 --- a/Documentation/line-range-format.txt +++ b/Documentation/line-range-format.txt @@ -26,3 +26,4 @@ If ``:'' is given in place of and , it denotes the range from the first funcname line that matches , up to the next funcname line. ``:'' searches from the end of the previous `-L` range, if any, otherwise from the start of file. +``^:'' searches from the start of file. diff --git a/line-range.c b/line-range.c index 4bae4bff21..ede0c6c98f 100644 --- a/line-range.c +++ b/line-range.c @@ -173,6 +173,11 @@ static const char *parse_range_funcname(const char *arg, nth_line_fn_t nth_line_ int reg_error; regex_t regexp; + if (*arg == '^') { + anchor = 1; + arg++; + } + assert(*arg == ':'); term = arg+1; while (*term && *term != ':') { @@ -245,7 +250,7 @@ int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb, if (anchor > lines) anchor = lines + 1; - if (*arg == ':') { + if (*arg == ':' || (*arg == '^' && *(arg + 1) == ':')) { arg = parse_range_funcname(arg, nth_line_cb, cb_data, lines, anchor, begin, end, path); if (!arg || *arg) return -1; @@ -270,7 +275,7 @@ int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb, const char *skip_range_arg(const char *arg) { - if (*arg == ':') + if (*arg == ':' || (*arg == '^' && *(arg + 1) == ':')) return parse_range_funcname(arg, NULL, NULL, 0, 0, NULL, NULL, NULL); arg = parse_loc(arg, NULL, NULL, 0, -1, NULL); diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index 4f7d6baef5..dabe89d91a 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -398,6 +398,23 @@ test_expect_success 'blame -L :RE (relative: end-of-file)' ' test_must_fail $PROG -L, -L:main hello.c ' +test_expect_success 'blame -L ^:RE (absolute)' ' + check_count -f hello.c -L3,3 -L^:ma.. F 4 G 1 +' + +test_expect_success 'blame -L ^:RE (absolute: no preceding range)' ' + check_count -f hello.c -L^:ma.. F 4 G 1 +' + +test_expect_success 'blame -L ^:RE (absolute: not found)' ' + test_must_fail $PROG -L4,4 -L^:tambourine hello.c +' + +test_expect_success 'blame -L ^:RE (absolute: end-of-file)' ' + n=$(printf "%d" $(wc -l Date: Tue, 6 Aug 2013 09:59:48 -0400 Subject: [PATCH 15/16] t8001/t8002: blame: add tests of -L line numbers less than 1 git-blame -L is documented as accepting 1-based line numbers. When handed a line number less than 1, -L's behavior is undocumented and undefined; it's also nonsensical and should be rejected but is nevertheless accepted. Demonstrate this shortcoming. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/annotate-tests.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index dabe89d91a..376b042f2f 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -185,6 +185,18 @@ test_expect_success 'blame -L Y,X (undocumented)' ' check_count -L6,3 B 1 B1 1 B2 1 D 1 ' +test_expect_failure 'blame -L -X' ' + test_must_fail $PROG -L-1 file +' + +test_expect_failure 'blame -L 0' ' + test_must_fail $PROG -L0 file +' + +test_expect_failure 'blame -L ,0' ' + test_must_fail $PROG -L,0 file +' + test_expect_success 'blame -L ,+0' ' test_must_fail $PROG -L,+0 file ' From 5ce922a014f78684a96c3d03a51decf0d21fa58d Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Tue, 6 Aug 2013 09:59:49 -0400 Subject: [PATCH 16/16] line-range: reject -L line numbers less than 1 Since inception, git-blame -L has been documented as accepting 1-based line numbers. When handed a line number less than 1, -L's behavior is undocumented and undefined; it's also nonsensical and should be diagnosed as an error. Do so. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- line-range.c | 5 ++++- t/annotate-tests.sh | 18 +++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/line-range.c b/line-range.c index ede0c6c98f..de4e32f942 100644 --- a/line-range.c +++ b/line-range.c @@ -54,8 +54,11 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line, } num = strtol(spec, &term, 10); if (term != spec) { - if (ret) + if (ret) { + if (num <= 0) + die("-L invalid line number: %ld", num); *ret = num; + } return term; } diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index 376b042f2f..99caa42f5c 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -185,15 +185,15 @@ test_expect_success 'blame -L Y,X (undocumented)' ' check_count -L6,3 B 1 B1 1 B2 1 D 1 ' -test_expect_failure 'blame -L -X' ' +test_expect_success 'blame -L -X' ' test_must_fail $PROG -L-1 file ' -test_expect_failure 'blame -L 0' ' +test_expect_success 'blame -L 0' ' test_must_fail $PROG -L0 file ' -test_expect_failure 'blame -L ,0' ' +test_expect_success 'blame -L ,0' ' test_must_fail $PROG -L,0 file ' @@ -447,8 +447,8 @@ test_expect_success 'blame empty' ' check_count -h HEAD^^ -f incremental ' -test_expect_success 'blame -L 0 empty (undocumented)' ' - check_count -h HEAD^^ -f incremental -L0 +test_expect_success 'blame -L 0 empty' ' + test_must_fail $PROG -L0 incremental HEAD^^ ' test_expect_success 'blame -L 1 empty' ' @@ -463,8 +463,8 @@ test_expect_success 'blame half' ' check_count -h HEAD^ -f incremental I 1 ' -test_expect_success 'blame -L 0 half (undocumented)' ' - check_count -h HEAD^ -f incremental -L0 I 1 +test_expect_success 'blame -L 0 half' ' + test_must_fail $PROG -L0 incremental HEAD^ ' test_expect_success 'blame -L 1 half' ' @@ -483,8 +483,8 @@ test_expect_success 'blame full' ' check_count -f incremental I 1 ' -test_expect_success 'blame -L 0 full (undocumented)' ' - check_count -f incremental -L0 I 1 +test_expect_success 'blame -L 0 full' ' + test_must_fail $PROG -L0 incremental ' test_expect_success 'blame -L 1 full' '