Refactor parse_loc

We want to use the same style of -L n,m argument for 'git log -L' as for git-blame. Refactor the argument parsing of the range arguments from builtin/blame.c to the (new) file that will hold the 'git log -L' logic. To accommodate different data structures in blame and log -L, the file contents are abstracted away; parse_range_arg takes a callback that it uses to get the contents of a line of the (notional) file. The new test is for a case that made me pause during debugging: the 'blame -L with invalid end' test was the only one that noticed an outright failure to parse the end *at all*. So make a more explicit test for that. Signed-off-by: Bo Yang <struggleyb.nku@gmail.com> Signed-off-by: Thomas Rast <trast@student.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Bo Yang committed Mar 28, 2013 at 17:47 UTC 25ed3412f86b136efb17689b6712d9e546cac388
7 files changed +151 -109
Documentation/blame-options.txt
+1 -18
@@ -13,24 +13,7 @@
13 Annotate only the given line range. <start> and <end> can take
14 one of these forms:
15
16 - - number
17 -+
18 -If <start> or <end> is a number, it specifies an
19 -absolute line number (lines count from 1).
20 -+
21 -
22 -- /regex/
23 -+
24 -This form will use the first line matching the given
25 -POSIX regex. If <end> is a regex, it will search
26 -starting at the line given by <start>.
27 -+
28 -
29 -- +offset or -offset
30 -+
31 -This is only valid for <end> and will specify a number
32 -of lines before or after the line given by <start>.
33 -+
16 +include::line-range-format.txt[]
17
18 -l::
19 Show long rev (Default: off).
Documentation/line-range-format.txt new
+18
@@ -0,0 +1,18 @@
1 +- number
2 ++
3 +If <start> or <end> is a number, it specifies an
4 +absolute line number (lines count from 1).
5 ++
6 +
7 +- /regex/
8 ++
9 +This form will use the first line matching the given
10 +POSIX regex. If <end> is a regex, it will search
11 +starting at the line given by <start>.
12 ++
13 +
14 +- +offset or -offset
15 ++
16 +This is only valid for <end> and will specify a number
17 +of lines before or after the line given by <start>.
18 ++
Makefile
+2
@@ -683,6 +683,7 @@ LIB_H += help.h
683 LIB_H += http.h
684 LIB_H += kwset.h
685 LIB_H += levenshtein.h
686 +LIB_H += line-range.h
687 LIB_H += list-objects.h
688 LIB_H += ll-merge.h
689 LIB_H += log-tree.h
@@ -798,6 +799,7 @@ LIB_OBJS += hex.o
799 LIB_OBJS += ident.o
800 LIB_OBJS += kwset.o
801 LIB_OBJS += levenshtein.o
802 +LIB_OBJS += line-range.o
803 LIB_OBJS += list-objects.o
804 LIB_OBJS += ll-merge.o
805 LIB_OBJS += lockfile.o
builtin/blame.c
+8 -91
@@ -21,6 +21,7 @@
21 #include "parse-options.h"
22 #include "utf8.h"
23 #include "userdiff.h"
24 +#include "line-range.h"
25
26 static char blame_usage[] = N_("git blame [options] [rev-opts] [rev] [--] file");
27
@@ -566,11 +567,16 @@ static void dup_entry(struct blame_entry *dst, struct blame_entry *src)
567 dst->score = 0;
568 }
569
569 -static const char *nth_line(struct scoreboard *sb, int lno)
570 +static const char *nth_line(struct scoreboard *sb, long lno)
571 {
572 return sb->final_buf + sb->lineno[lno];
573 }
574
575 +static const char *nth_line_cb(void *data, long lno)
576 +{
577 + return nth_line((struct scoreboard *)data, lno);
578 +}
579 +
580 /*
581 * It is known that lines between tlno to same came from parent, and e
582 * has an overlap with that range. it also is known that parent's
@@ -1926,83 +1932,6 @@ static const char *add_prefix(const char *prefix, const char *path)
1932 return prefix_path(prefix, prefix ? strlen(prefix) : 0, path);
1933 }
1934
1929 -/*
1930 - * Parsing of (comma separated) one item in the -L option
1931 - */
1932 -static const char *parse_loc(const char *spec,
1933 - struct scoreboard *sb, long lno,
1934 - long begin, long *ret)
1935 -{
1936 - char *term;
1937 - const char *line;
1938 - long num;
1939 - int reg_error;
1940 - regex_t regexp;
1941 - regmatch_t match[1];
1942 -
1943 - /* Allow "-L <something>,+20" to mean starting at <something>
1944 - * for 20 lines, or "-L <something>,-5" for 5 lines ending at
1945 - * <something>.
1946 - */
1947 - if (1 < begin && (spec[0] == '+' || spec[0] == '-')) {
1948 - num = strtol(spec + 1, &term, 10);
1949 - if (term != spec + 1) {
1950 - if (spec[0] == '-')
1951 - num = 0 - num;
1952 - if (0 < num)
1953 - *ret = begin + num - 2;
1954 - else if (!num)
1955 - *ret = begin;
1956 - else
1957 - *ret = begin + num;
1958 - return term;
1959 - }
1960 - return spec;
1961 - }
1962 - num = strtol(spec, &term, 10);
1963 - if (term != spec) {
1964 - *ret = num;
1965 - return term;
1966 - }
1967 - if (spec[0] != '/')
1968 - return spec;
1969 -
1970 - /* it could be a regexp of form /.../ */
1971 - for (term = (char *) spec + 1; *term && *term != '/'; term++) {
1972 - if (*term == '\\')
1973 - term++;
1974 - }
1975 - if (*term != '/')
1976 - return spec;
1977 -
1978 - /* try [spec+1 .. term-1] as regexp */
1979 - *term = 0;
1980 - begin--; /* input is in human terms */
1981 - line = nth_line(sb, begin);
1982 -
1983 - if (!(reg_error = regcomp(&regexp, spec + 1, REG_NEWLINE)) &&
1984 - !(reg_error = regexec(&regexp, line, 1, match, 0))) {
1985 - const char *cp = line + match[0].rm_so;
1986 - const char *nline;
1987 -
1988 - while (begin++ < lno) {
1989 - nline = nth_line(sb, begin);
1990 - if (line <= cp && cp < nline)
1991 - break;
1992 - line = nline;
1993 - }
1994 - *ret = begin;
1995 - regfree(&regexp);
1996 - *term++ = '/';
1997 - return term;
1998 - }
1999 - else {
2000 - char errbuf[1024];
2001 - regerror(reg_error, &regexp, errbuf, 1024);
2002 - die("-L parameter '%s': %s", spec + 1, errbuf);
2003 - }
2004 -}
2005 -
1935 /*
1936 * Parsing of -L option
1937 */
@@ -2011,15 +1940,7 @@ static void prepare_blame_range(struct scoreboard *sb,
1940 long lno,
1941 long *bottom, long *top)
1942 {
2014 - const char *term;
2015 -
2016 - term = parse_loc(bottomtop, sb, lno, 1, bottom);
2017 - if (*term == ',') {
2018 - term = parse_loc(term + 1, sb, lno, *bottom + 1, top);
2019 - if (*term)
2020 - usage(blame_usage);
2021 - }
2022 - if (*term)
1943 + if (parse_range_arg(bottomtop, nth_line_cb, sb, lno, bottom, top))
1944 usage(blame_usage);
1945 }
1946
@@ -2569,10 +2490,6 @@ parse_done:
2490 bottom = top = 0;
2491 if (bottomtop)
2492 prepare_blame_range(&sb, bottomtop, lno, &bottom, &top);
2572 - if (bottom && top && top < bottom) {
2573 - long tmp;
2574 - tmp = top; top = bottom; bottom = tmp;
2575 - }
2493 if (bottom < 1)
2494 bottom = 1;
2495 if (top < 1)
line-range.c new
+92
@@ -0,0 +1,92 @@
1 +#include "git-compat-util.h"
2 +#include "line-range.h"
3 +
4 +/*
5 + * Parse one item in the -L option
6 + */
7 +static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,
8 + void *data, long lines, long begin, long *ret)
9 +{
10 + char *term;
11 + const char *line;
12 + long num;
13 + int reg_error;
14 + regex_t regexp;
15 + regmatch_t match[1];
16 +
17 + /* Allow "-L <something>,+20" to mean starting at <something>
18 + * for 20 lines, or "-L <something>,-5" for 5 lines ending at
19 + * <something>.
20 + */
21 + if (1 < begin && (spec[0] == '+' || spec[0] == '-')) {
22 + num = strtol(spec + 1, &term, 10);
23 + if (term != spec + 1) {
24 + if (spec[0] == '-')
25 + num = 0 - num;
26 + if (0 < num)
27 + *ret = begin + num - 2;
28 + else if (!num)
29 + *ret = begin;
30 + else
31 + *ret = begin + num;
32 + return term;
33 + }
34 + return spec;
35 + }
36 + num = strtol(spec, &term, 10);
37 + if (term != spec) {
38 + *ret = num;
39 + return term;
40 + }
41 + if (spec[0] != '/')
42 + return spec;
43 +
44 + /* it could be a regexp of form /.../ */
45 + for (term = (char *) spec + 1; *term && *term != '/'; term++) {
46 + if (*term == '\\')
47 + term++;
48 + }
49 + if (*term != '/')
50 + return spec;
51 +
52 + /* try [spec+1 .. term-1] as regexp */
53 + *term = 0;
54 + begin--; /* input is in human terms */
55 + line = nth_line(data, begin);
56 +
57 + if (!(reg_error = regcomp(&regexp, spec + 1, REG_NEWLINE)) &&
58 + !(reg_error = regexec(&regexp, line, 1, match, 0))) {
59 + const char *cp = line + match[0].rm_so;
60 + const char *nline;
61 +
62 + while (begin++ < lines) {
63 + nline = nth_line(data, begin);
64 + if (line <= cp && cp < nline)
65 + break;
66 + line = nline;
67 + }
68 + *ret = begin;
69 + regfree(&regexp);
70 + *term++ = '/';
71 + return term;
72 + }
73 + else {
74 + char errbuf[1024];
75 + regerror(reg_error, &regexp, errbuf, 1024);
76 + die("-L parameter '%s': %s", spec + 1, errbuf);
77 + }
78 +}
79 +
80 +int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb,
81 + void *cb_data, long lines, long *begin, long *end)
82 +{
83 + arg = parse_loc(arg, nth_line_cb, cb_data, lines, 1, begin);
84 +
85 + if (*arg == ',')
86 + arg = parse_loc(arg + 1, nth_line_cb, cb_data, lines, *begin + 1, end);
87 +
88 + if (*arg)
89 + return -1;
90 +
91 + return 0;
92 +}
line-range.h new
+24
@@ -0,0 +1,24 @@
1 +#ifndef LINE_RANGE_H
2 +#define LINE_RANGE_H
3 +
4 +/*
5 + * Parse one item in an -L begin,end option w.r.t. the notional file
6 + * object 'cb_data' consisting of 'lines' lines.
7 + *
8 + * The 'nth_line_cb' callback is used to determine the start of the
9 + * line 'lno' inside the 'cb_data'. The caller is expected to already
10 + * have a suitable map at hand to make this a constant-time lookup.
11 + *
12 + * Returns 0 in case of success and -1 if there was an error. The
13 + * actual range is stored in *begin and *end. The counting starts
14 + * at 1! In case of error, the caller should show usage message.
15 + */
16 +
17 +typedef const char *(*nth_line_fn_t)(void *data, long lno);
18 +
19 +extern int parse_range_arg(const char *arg,
20 + nth_line_fn_t nth_line_cb,
21 + void *cb_data, long lines,
22 + long *begin, long *end);
23 +
24 +#endif /* LINE_RANGE_H */
t/t8003-blame-corner-cases.sh
+6
@@ -175,6 +175,12 @@ test_expect_success 'blame -L with invalid end' '
175 grep "has only 2 lines" errors
176 '
177
178 +test_expect_success 'blame parses <end> part of -L' '
179 + git blame -L1,1 tres >out &&
180 + cat out &&
181 + test $(wc -l < out) -eq 1
182 +'
183 +
184 test_expect_success 'indent of line numbers, nine lines' '
185 git blame nine_lines >actual &&
186 test $(grep -c " " actual) = 0