From: Bo Yang <struggleyb....@gmail.com>
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....@gmail.com>
Signed-off-by: Thomas Rast <tr...@student.ethz.ch>
---
Documentation/blame-options.txt | 19 +------
Documentation/line-range-format.txt | 18 +++++++
Makefile | 2 +
builtin/blame.c | 99 +++----------------------------------
line-range.c | 92 ++++++++++++++++++++++++++++++++++
line-range.h | 24 +++++++++
t/t8003-blame-corner-cases.sh | 6 +++
7 files changed, 151 insertions(+), 109 deletions(-)
create mode 100644 Documentation/line-range-format.txt
create mode 100644 line-range.c
create mode 100644 line-range.h
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index b0d31df..6998d9f 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -13,24 +13,7 @@
Annotate only the given line range. <start> and <end> can take
one of these forms:
- - number
-+
-If <start> or <end> is a number, it specifies an
-absolute line number (lines count from 1).
-+
-
-- /regex/
-+
-This form will use the first line matching the given
-POSIX regex. If <end> is a regex, it will search
-starting at the line given by <start>.
-+
-
-- +offset or -offset
-+
-This is only valid for <end> and will specify a number
-of lines before or after the line given by <start>.
-+
+include::line-range-format.txt[]
-l::
Show long rev (Default: off).
diff --git a/Documentation/line-range-format.txt
b/Documentation/line-range-format.txt
new file mode 100644
index 0000000..265bc23
--- /dev/null
+++ b/Documentation/line-range-format.txt
@@ -0,0 +1,18 @@
+- number
++
+If <start> or <end> is a number, it specifies an
+absolute line number (lines count from 1).
++
+
+- /regex/
++
+This form will use the first line matching the given
+POSIX regex. If <end> is a regex, it will search
+starting at the line given by <start>.
++
+
+- +offset or -offset
++
+This is only valid for <end> and will specify a number
+of lines before or after the line given by <start>.
++
diff --git a/Makefile b/Makefile
index 598d631..e83f64b 100644
--- a/Makefile
+++ b/Makefile
@@ -667,6 +667,7 @@ LIB_H += help.h
LIB_H += http.h
LIB_H += kwset.h
LIB_H += levenshtein.h
+LIB_H += line-range.h
LIB_H += list-objects.h
LIB_H += ll-merge.h
LIB_H += log-tree.h
@@ -795,6 +796,7 @@ LIB_OBJS += hex.o
LIB_OBJS += ident.o
LIB_OBJS += kwset.o
LIB_OBJS += levenshtein.o
+LIB_OBJS += line-range.o
LIB_OBJS += list-objects.o
LIB_OBJS += ll-merge.o
LIB_OBJS += lockfile.o
diff --git a/builtin/blame.c b/builtin/blame.c
index 86100e9..20eb439 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -21,6 +21,7 @@
#include "parse-options.h"
#include "utf8.h"
#include "userdiff.h"
+#include "line-range.h"
static char blame_usage[] = N_("git blame [options] [rev-opts] [rev] [--]
file");
@@ -566,11 +567,16 @@ static void dup_entry(struct blame_entry *dst, struct
blame_entry *src)
dst->score = 0;
}
-static const char *nth_line(struct scoreboard *sb, int lno)
+static const char *nth_line(struct scoreboard *sb, long lno)
{
return sb->final_buf + sb->lineno[lno];
}
+static const char *nth_line_cb(void *data, long lno)
+{
+ return nth_line((struct scoreboard *)data, lno);
+}
+
/*
* It is known that lines between tlno to same came from parent, and e
* has an overlap with that range. it also is known that parent's
@@ -1927,83 +1933,6 @@ static const char *add_prefix(const char *prefix, const
char *path)
}
/*
- * Parsing of (comma separated) one item in the -L option
- */
-static const char *parse_loc(const char *spec,
- struct scoreboard *sb, long lno,
- long begin, long *ret)
-{
- char *term;
- const char *line;
- long num;
- int reg_error;
- regex_t regexp;
- regmatch_t match[1];
-
- /* Allow "-L <something>,+20" to mean starting at <something>
- * for 20 lines, or "-L <something>,-5" for 5 lines ending at
- * <something>.
- */
- if (1 < begin && (spec[0] == '+' || spec[0] == '-')) {
- num = strtol(spec + 1, &term, 10);
- if (term != spec + 1) {
- if (spec[0] == '-')
- num = 0 - num;
- if (0 < num)
- *ret = begin + num - 2;
- else if (!num)
- *ret = begin;
- else
- *ret = begin + num;
- return term;
- }
- return spec;
- }
- num = strtol(spec, &term, 10);
- if (term != spec) {
- *ret = num;
- return term;
- }
- if (spec[0] != '/')
- return spec;
-
- /* it could be a regexp of form /.../ */
- for (term = (char *) spec + 1; *term && *term != '/'; term++) {
- if (*term == '\\')
- term++;
- }
- if (*term != '/')
- return spec;
-
- /* try [spec+1 .. term-1] as regexp */
- *term = 0;
- begin--; /* input is in human terms */
- line = nth_line(sb, begin);
-
- if (!(reg_error = regcomp(®exp, spec + 1, REG_NEWLINE)) &&
- !(reg_error = regexec(®exp, line, 1, match, 0))) {
- const char *cp = line + match[0].rm_so;
- const char *nline;
-
- while (begin++ < lno) {
- nline = nth_line(sb, begin);
- if (line <= cp && cp < nline)
- break;
- line = nline;
- }
- *ret = begin;
- regfree(®exp);
- *term++ = '/';
- return term;
- }
- else {
- char errbuf[1024];
- regerror(reg_error, ®exp, errbuf, 1024);
- die("-L parameter '%s': %s", spec + 1, errbuf);
- }
-}
-
-/*
* Parsing of -L option
*/
static void prepare_blame_range(struct scoreboard *sb,
@@ -2011,15 +1940,7 @@ static void prepare_blame_range(struct scoreboard *sb,
long lno,
long *bottom, long *top)
{
- const char *term;
-
- term = parse_loc(bottomtop, sb, lno, 1, bottom);
- if (*term == ',') {
- term = parse_loc(term + 1, sb, lno, *bottom + 1, top);
- if (*term)
- usage(blame_usage);
- }
- if (*term)
+ if (parse_range_arg(bottomtop, nth_line_cb, sb, lno, bottom, top))
usage(blame_usage);
}
@@ -2569,10 +2490,6 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
bottom = top = 0;
if (bottomtop)
prepare_blame_range(&sb, bottomtop, lno, &bottom, &top);
- if (bottom && top && top < bottom) {
- long tmp;
- tmp = top; top = bottom; bottom = tmp;
- }
if (bottom < 1)
bottom = 1;
if (top < 1)
diff --git a/line-range.c b/line-range.c
new file mode 100644
index 0000000..5a226ab
--- /dev/null
+++ b/line-range.c
@@ -0,0 +1,92 @@
+#include "git-compat-util.h"
+#include "line-range.h"
+
+/*
+ * Parse one item in the -L option
+ */
+static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,
+ void *data, long lines, long begin, long *ret)
+{
+ char *term;
+ const char *line;
+ long num;
+ int reg_error;
+ regex_t regexp;
+ regmatch_t match[1];
+
+ /* Allow "-L <something>,+20" to mean starting at <something>
+ * for 20 lines, or "-L <something>,-5" for 5 lines ending at
+ * <something>.
+ */
+ if (1 < begin && (spec[0] == '+' || spec[0] == '-')) {
+ num = strtol(spec + 1, &term, 10);
+ if (term != spec + 1) {
+ if (spec[0] == '-')
+ num = 0 - num;
+ if (0 < num)
+ *ret = begin + num - 2;
+ else if (!num)
+ *ret = begin;
+ else
+ *ret = begin + num;
+ return term;
+ }
+ return spec;
+ }
+ num = strtol(spec, &term, 10);
+ if (term != spec) {
+ *ret = num;
+ return term;
+ }
+ if (spec[0] != '/')
+ return spec;
+
+ /* it could be a regexp of form /.../ */
+ for (term = (char *) spec + 1; *term && *term != '/'; term++) {
+ if (*term == '\\')
+ term++;
+ }
+ if (*term != '/')
+ return spec;
+
+ /* try [spec+1 .. term-1] as regexp */
+ *term = 0;
+ begin--; /* input is in human terms */
+ line = nth_line(data, begin);
+
+ if (!(reg_error = regcomp(®exp, spec + 1, REG_NEWLINE)) &&
+ !(reg_error = regexec(®exp, line, 1, match, 0))) {
+ const char *cp = line + match[0].rm_so;
+ const char *nline;
+
+ while (begin++ < lines) {
+ nline = nth_line(data, begin);
+ if (line <= cp && cp < nline)
+ break;
+ line = nline;
+ }
+ *ret = begin;
+ regfree(®exp);
+ *term++ = '/';
+ return term;
+ }
+ else {
+ char errbuf[1024];
+ regerror(reg_error, ®exp, errbuf, 1024);
+ die("-L parameter '%s': %s", spec + 1, errbuf);
+ }
+}
+
+int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb,
+ void *cb_data, long lines, long *begin, long *end)
+{
+ arg = parse_loc(arg, nth_line_cb, cb_data, lines, 1, begin);
+
+ if (*arg == ',')
+ arg = parse_loc(arg + 1, nth_line_cb, cb_data, lines, *begin +
1, end);
+
+ if (*arg)
+ return -1;
+
+ return 0;
+}
diff --git a/line-range.h b/line-range.h
new file mode 100644
index 0000000..830f25b
--- /dev/null
+++ b/line-range.h
@@ -0,0 +1,24 @@
+#ifndef LINE_RANGE_H
+#define LINE_RANGE_H
+
+/*
+ * Parse one item in an -L begin,end option w.r.t. the notional file
+ * object 'cb_data' consisting of 'lines' lines.
+ *
+ * The 'nth_line_cb' callback is used to determine the start of the
+ * 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.
+ *
+ * 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.
+ */
+
+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,
+ long *begin, long *end);
+
+#endif /* LINE_RANGE_H */
diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
index 230143c..e7cac1d 100755
--- a/t/t8003-blame-corner-cases.sh
+++ b/t/t8003-blame-corner-cases.sh
@@ -175,6 +175,12 @@ test_expect_success 'blame -L with invalid end' '
grep "has only 2 lines" errors
'
+test_expect_success 'blame parses <end> part of -L' '
+ git blame -L1,1 tres >out &&
+ cat out &&
+ test $(wc -l < out) -eq 1
+'
+
test_expect_success 'indent of line numbers, nine lines' '
git blame nine_lines >actual &&
test $(grep -c " " actual) = 0
--
1.8.2.446.g2b4de83
--
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
