[PATCH v2 00/16] blame: accept multiple -L ranges

[PATCH v2 00/16] blame: accept multiple -L ranges

[Eric Sunshine]

This is a re-roll of [1] which teaches git-blame to accept multiple -L
ranges. It is built atop [6] (es/blame-L-more in 'pu').

The series is longer than expected since it includes a few more cleanup
patches beyond those already posted separately [2], [3], [4], [5], [6];
and because it implements relative /RE/ searches requested by Junio [7],
and brings git-log's multiple -L behavior in line with git-blame's.
Despite the length of the series, the patches are mostly small and
simple.


Changes since v1:

* Make /RE/ searches relative to end of previous -L, if any [7]. Ditto
  for :RE searches.

* Introduce ^/RE/ and ^:RE to search from start of file.

* Add (lots of) tests.

* Update documentation.

* Collect ranges via 'struct range_set' rather than (ab)using
  blame.c:coalesce() and add_blame_range().


Quick overview of patches:

1-2: More cleanups akin to [2], [3], [4], [5], [6].

3-7: Implement git-blame multiple -L support; add tests; update
  documentation.

8-14: Make /RE/ and :RE searches relative; introduce ^/RE/ and ^:RE to
  search from start of file; add tests; update documentation.

15-16: Reject bogus -L inputs for line numbers less than 1. This comes
  late in the series because this case becomes much easier to detect
  following patch 8.


[1]: http://thread.gmane.org/gmane.comp.version-control.git/229755
[2]: http://thread.gmane.org/gmane.comp.version-control.git/229917
[3]: http://thread.gmane.org/gmane.comp.version-control.git/230532
[4]: http://git.661346.n2.nabble.com/PATCH-0-6-fix-blame-L-regression-add-tests-tp7592174.html
[5]: http://thread.gmane.org/gmane.comp.version-control.git/231035
[6]: http://thread.gmane.org/gmane.comp.version-control.git/231412
[7]: http://article.gmane.org/gmane.comp.version-control.git/229966


Eric Sunshine (16):
  git-log.txt: place each -L option variation on its own line
  line-range-format.txt: clarify -L:regex usage form
  range-set: publish API for re-use by git-blame -L
  blame: inline one-line function into its lone caller
  blame: accept multiple -L ranges
  t8001/t8002: blame: add tests of multiple -L options
  blame: document multiple -L support
  line-range: teach -L/RE/ to search relative to anchor point
  blame: teach -L/RE/ to search from end of previous -L range
  log: teach -L/RE/ to search from end of previous -L range
  line-range-format.txt: document -L/RE/ relative search
  line-range: teach -L^/RE/ to search from start of file
  line-range: teach -L:RE to search from end of previous -L range
  line-range: teach -L^:RE to search from start of file
  t8001/t8002: blame: add tests of -L line numbers less than 1
  line-range: reject -L line numbers less than 1

 Documentation/blame-options.txt     |  10 +--
 Documentation/git-blame.txt         |  10 ++-
 Documentation/git-log.txt           |   5 +-
 Documentation/line-range-format.txt |  16 +++--
 builtin/blame.c                     |  93 ++++++++++++++------------
 line-log.c                          |  25 ++++---
 line-log.h                          |  12 ++++
 line-range.c                        |  62 +++++++++++++----
 line-range.h                        |   5 +-
 t/annotate-tests.sh                 | 130 ++++++++++++++++++++++++++++++++++--
 t/t4211-line-log.sh                 |   2 +-
 11 files changed, 282 insertions(+), 88 deletions(-)

-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 01/16] git-log.txt: place each -L option variation on its own line

[Eric Sunshine]

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
<start>,<end>:<file>" and "-L :<regex>:<file>", separated by a comma.
This is inconsistent and potentially confusing since the comma
separating them is typeset the same as the comma in "<start>,<end>". Fix
this by placing each variation on its own line.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 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 ac2694d..fe04e06 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 <start>,<end>:<file>, -L :<regex>:<file>::
+-L <start>,<end>:<file>::
+-L :<regex>:<file>::
 
 	Trace the evolution of the line range given by "<start>,<end>"
 	(or the funcname regex <regex>) within the <file>.  You may
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 02/16] line-range-format.txt: clarify -L:regex usage form

[Eric Sunshine]

blame/log documentation describes -L option as:

  -L<start>,<end>
  -L:<regex>

  <start> and <end> 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 <start> or <end>; in fact, it must be -L's lone argument.

Clarify by discussing :<regex> at the same indentation level as "<start>
and <end>...":

  -L<start>,<end>
  -L:<regex>

  <start> and <end> can take one of these forms:

    * number
    * /regex/
    * +offset or -offset

  If :<regex> is given in place of <start> and <end> ...

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 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 4e55b15..489032c 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -15,8 +15,6 @@
 	``-L <start>'' or ``-L <start>,'' spans from <start> to end of file.
 	``-L ,<end>'' spans from start of file to <end>.
 +
-<start> and <end> 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 fe04e06..34097ef 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.
 +
-<start> and <end> can take one of these forms:
-
 include::line-range-format.txt[]
 
 <revision range>::
diff --git a/Documentation/line-range-format.txt b/Documentation/line-range-format.txt
index 3e7ce72..a1b2f4e 100644
--- a/Documentation/line-range-format.txt
+++ b/Documentation/line-range-format.txt
@@ -1,3 +1,5 @@
+<start> and <end> can take one of these forms:
+
 - number
 +
 If <start> or <end> is a number, it specifies an
@@ -15,11 +17,8 @@ starting at the line given by <start>.
 +
 This is only valid for <end> and will specify a number
 of lines before or after the line given by <start>.
-+
 
-- :regex
 +
-If the option's argument is of the form :regex, it denotes the range
+If ``:<regex>'' is given in place of <start> and <end>, it denotes the range
 from the first funcname line that matches <regex>, up to the next
 funcname line.
-+
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 03/16] range-set: publish API for re-use by git-blame -L

[Eric Sunshine]

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 <sunshine@sunshineco.com>
---

Since range-set is generic -- not at all specific to -L line ranges -- a
future patch series may want to publish the entire API, move the
implementation to its own file, and add documentation and full unit-test
coverage, however, such a topic is orthogonal to the present series
which adds multiple -L support to git-blame.

 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 1c3ac8d..bdadf35 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 8bea45f..a9212d8 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.
  *
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 04/16] blame: inline one-line function into its lone caller

[Eric Sunshine]

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 <sunshine@sunshineco.com>
---
 builtin/blame.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index e70b089..9db01b0 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)
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 05/16] blame: accept multiple -L ranges

[Eric Sunshine]

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 <sunshine@sunshineco.com>
---

This patch is a bit less scary when whitespace changes are ignored.

 builtin/blame.c | 79 ++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 47 insertions(+), 32 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 9db01b0..2f4d9e2 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 <file>'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--;
-
-	ent = xcalloc(1, sizeof(*ent));
-	ent->lno = bottom;
-	ent->num_lines = top - bottom;
-	ent->suspect = o;
-	ent->s_lno = bottom;
+	if (lno && !range_list.nr)
+		string_list_append(&range_list, xstrdup("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,
+				    &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;
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 06/16] t8001/t8002: blame: add tests of multiple -L options

[Eric Sunshine]

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/annotate-tests.sh | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index ce5b8ed..77083d9 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[])
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 07/16] blame: document multiple -L support

[Eric Sunshine]

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 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 489032c..0cebc4f 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -11,9 +11,11 @@
 
 -L <start>,<end>::
 -L :<regex>::
-	Annotate only the given line range.  <start> and <end> are optional.
-	``-L <start>'' or ``-L <start>,'' spans from <start> to end of file.
-	``-L ,<end>'' spans from start of file to <end>.
+	Annotate only the given line range. May be specified multiple times.
+	Overlapping ranges are allowed.
++
+<start> and <end> are optional. ``-L <start>'' or ``-L <start>,'' spans from
+<start> to end of file. ``-L ,<end>'' spans from start of file to <end>.
 +
 include::line-range-format.txt[]
 
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index 6cea7f1..f2c85cc 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 <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
+	    [-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
 	    [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>] [--] <file>
 
 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):
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 08/16] line-range: teach -L/RE/ to search relative to anchor point

[Eric Sunshine]

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 <sunshine@sunshineco.com>
---
 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 2f4d9e2..7b084d8 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 bdadf35..38f827b 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 69e8d6b..bbf3c0f 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, &regexp, 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 ae3d012..83ba3c2 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);
 
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 09/16] blame: teach -L/RE/ to search from end of previous -L range

[Eric Sunshine]

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 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 7b084d8..1bf8056 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 77083d9..b963d36 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[])
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 10/16] log: teach -L/RE/ to search from end of previous -L range

[Eric Sunshine]

This is complicated slightly by having to remember the previous -L range
for each file specified via -L<range>: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 <sunshine@sunshineco.com>
---
 line-log.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/line-log.c b/line-log.c
index 38f827b..d40c79d 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;
 }
 
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 11/16] line-range-format.txt: document -L/RE/ relative search

[Eric Sunshine]

Option -L/RE/ of blame/log now searches relative to the previous -L
range, if any. Document this.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 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 a1b2f4e..42d74f7 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 <end> is a regex, it will search
+POSIX regex. If <start> is a regex, it will search from the end of
+the previous `-L` range, if any, otherwise from the start of file.
+If <end> is a regex, it will search
 starting at the line given by <start>.
 +
 
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 12/16] line-range: teach -L^/RE/ to search from start of file

[Eric Sunshine]

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 <start> argument of
-L<start>,<end>. The <end> argument, as usual, is relative to <start>.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 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 42d74f7..cf84417 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 <start> is a regex, it will search from the end of
 the previous `-L` range, if any, otherwise from the start of file.
+If <start> is ``^/regex/'', it will search from the start of file.
 If <end> is a regex, it will search
 starting at the line given by <start>.
 +
diff --git a/line-range.c b/line-range.c
index bbf3c0f..7048489 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 b963d36..5a7d7c7 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 <file) + 1) &&
+	check_count -L$n -L^/$/,+2 A 1 C 1 E 1
+'
+
 test_expect_success 'setup -L :regex' '
 	tr Q "\\t" >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
+'
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 13/16] line-range: teach -L:RE to search from end of previous -L range

[Eric Sunshine]

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 <sunshine@sunshineco.com>
---
 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 cf84417..469d80b 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 <start>.
 +
 If ``:<regex>'' is given in place of <start> and <end>, it denotes the range
 from the first funcname line that matches <regex>, up to the next
-funcname line.
+funcname line. ``:<regex>'' 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 7048489..4bae4bf 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, &regexp);
 	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 5a7d7c7..4f7d6ba 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 b01b3dd..8ba2d51 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
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 14/16] line-range: teach -L^:RE to search from start of file

[Eric Sunshine]

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 <sunshine@sunshineco.com>
---
 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 469d80b..d7f2603 100644
--- a/Documentation/line-range-format.txt
+++ b/Documentation/line-range-format.txt
@@ -26,3 +26,4 @@ If ``:<regex>'' is given in place of <start> and <end>, it denotes the range
 from the first funcname line that matches <regex>, up to the next
 funcname line. ``:<regex>'' searches from the end of the previous `-L` range,
 if any, otherwise from the start of file.
+``^:<regex>'' searches from the start of file.
diff --git a/line-range.c b/line-range.c
index 4bae4bf..ede0c6c 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 4f7d6ba..dabe89d 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 <hello.c)) &&
+	check_count -f hello.c -L$n -L^:ma.. F 4 G 1 H 1
+'
+
 test_expect_success 'setup incremental' '
 	(
 	GIT_AUTHOR_NAME=I &&
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 8ba2d51..7369d3c 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -48,6 +48,7 @@ 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
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 15/16] t8001/t8002: blame: add tests of -L line numbers less than 1

[Eric Sunshine]

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 <sunshine@sunshineco.com>
---
 t/annotate-tests.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index dabe89d..376b042 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
 '
-- 
1.8.4.rc1.409.gbd48715

[PATCH v2 16/16] line-range: reject -L line numbers less than 1

[Eric Sunshine]

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 <sunshine@sunshineco.com>
---
 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 ede0c6c..de4e32f 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 376b042..99caa42 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' '
-- 
1.8.4.rc1.409.gbd48715

Re: [PATCH v2 05/16] blame: accept multiple -L ranges

[Junio C Hamano]

> +	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);

Hmph, I do not see where the need for this decref is coming from.
Did we incref once too many somewhere?

> +	range_set_release(&ranges);
> +	string_list_clear(&range_list, 0);
>  
>  	sb.ent = ent;
>  	sb.path = path;

Re: [PATCH v2 08/16] line-range: teach -L/RE/ to search relative to anchor point

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> 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 <sunshine@sunshineco.com>
> ---
>  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 2f4d9e2..7b084d8 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 bdadf35..38f827b 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,

This one feeds "1" to anchor, which in turn is given to parse_loc as
"-1" and then (after bypassing the <end> part support) its sign
flipped once again to become "begin=1" in parse_loc().  Then we run
regexp search starting from (1-based) 1st line, retaining the
"always scan from the beginning" behaviour in this step in the
series.

OK, looks sensible.

Re: [PATCH v2 09/16] blame: teach -L/RE/ to search from end of previous -L range

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---

With the previous step, what this one does is fairly obvious and
straight-forward.  Looking good ;-)

>  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 7b084d8..1bf8056 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 77083d9..b963d36 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[])

Re: [PATCH v2 05/16] blame: accept multiple -L ranges

[Eric Sunshine]

On Tue, Aug 6, 2013 at 5:33 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> +     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);
>
> Hmph, I do not see where the need for this decref is coming from.
> Did we incref once too many somewhere?

Each constructed blame_entry must own a reference to the suspect.
o->refcnt should equal the number of blame_entries. At construction, a
'struct origin' has refcnt 1. In the original code, which supported
only a single initial range (blame_entry), we had:

  o = get-initial-suspect();  # refcnt already 1
  ent->suspect = o;  # refcnt still 1
  sb.ent = ent;
  assign_blame(&sb);

So, o->refcnt equals the number of blame_entries (1) when
assign_blame() is called.

The new for-loop calls origin_incref() on each iteration since each
blame_entry needs to own a reference to the suspect. Assume that we
have two disjoint -L ranges:

  o = get-initial-suspect();  # refcnt already 1
  foreach range:
    ent = new blame_entry;
    ent->suspect = o;
    origin_incref(o);  # refcnt++
  end
  # for 2 ranges, refcnt incremented twice, so value is 3
  origin_decref(o);  # refcnt = 2
  sb.ent = ent;
  assign_blame(&sb);

Thus, as with the original code, o->refcnt equals the number of
blame_entries (2) when assign_blame() is called.

The same holds for the boundary case when the file is empty and there
is no range. o->refcnt starts at 1, the loop is never entered so no
blame_entries are created, and o->refcnt gets decremented to 0, which
again matches the number of blame_entries (0).

Re: [PATCH v2 05/16] blame: accept multiple -L ranges

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> Each constructed blame_entry must own a reference to the suspect.
> o->refcnt should equal the number of blame_entries. At construction, a
> 'struct origin' has refcnt 1. In the original code, which supported
> only a single initial range (blame_entry), we had:
>
>   o = get-initial-suspect();  # refcnt already 1
>   ent->suspect = o;  # refcnt still 1

Ah, of course.

I forgot that I initialized a new origin with refcnt 1 exactly for
this.  As you use it once for each range, you would need to
compensate for it.

Thanks.