[PATCH] blame: add a range option to -L

[PATCH] blame: add a range option to -L

[Bill Pemberton]

In addition to + and - you can now use r to specify how many lines
around <start> that you want to see.  For example: -L 20,r5 would show
lines 15 through 25

Signed-off-by: Bill Pemberton <wfp5p@virginia.edu>
---

I've been using a perl wrapper script to get this functionality.
In case others may find this a useful feature, I went ahead and
added it to blame.c


 Documentation/blame-options.txt |    6 ++++++
 Documentation/git-blame.txt     |    6 ++++++
 builtin/blame.c                 |   19 +++++++++++++++++--
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index d820569..acbbc91 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -32,6 +32,12 @@ This is only valid for <end> and will specify a number
 of lines before or after the line given by <start>.
 +
 
+- rrange
++
+This is only valid for <end> and will specify a number
+of lines before and after the line given by <start>.
++
+
 -l::
 	Show long rev (Default: off).
 
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index a27f439..7a9b99a 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -110,6 +110,12 @@ line 40):
 	git blame -L 40,60 foo
 	git blame -L 40,+21 foo
 
+A range of lines around a particular line can be shown by using 'r'
+before the second number.  If you wanted to see line 20 along with the
+5 lines around it:
+
+  	git blame -L 20,r5 foo
+
 Also you can use a regular expression to specify the line range:
 
 	git blame -L '/^sub hello {/,/^}$/' foo
diff --git a/builtin/blame.c b/builtin/blame.c
index fc15863..03a8948 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1887,13 +1887,18 @@ static const char *parse_loc(const char *spec,
 	/* Allow "-L <something>,+20" to mean starting at <something>
 	 * for 20 lines, or "-L <something>,-5" for 5 lines ending at
 	 * <something>.
+	 * In addition "-L <something>,r5" means starting at
+	 * <something>-5 and ending at <something>+5
 	 */
-	if (1 < begin && (spec[0] == '+' || spec[0] == '-')) {
+	if (1 < begin &&
+	    (spec[0] == '+' || spec[0] == '-' || spec[0] == 'r')) {
 		num = strtol(spec + 1, &term, 10);
 		if (term != spec + 1) {
 			if (spec[0] == '-')
 				num = 0 - num;
-			if (0 < num)
+			if (spec[0] == 'r')
+				*ret = 0 - num;
+			else if (0 < num)
 				*ret = begin + num - 2;
 			else if (!num)
 				*ret = begin;
@@ -1960,6 +1965,16 @@ static void prepare_blame_range(struct scoreboard *sb,
 	term = parse_loc(bottomtop, sb, lno, 1, bottom);
 	if (*term == ',') {
 		term = parse_loc(term + 1, sb, lno, *bottom + 1, top);
+
+		/* if top is negative then top is actually a range
+		   that was specified with the r option */
+		if (*top < 1) {
+		    long x = *top;
+		    *top = *bottom - x;
+		    *bottom += x;
+		    if (*bottom < 1)
+			*bottom = 1;
+		}
 		if (*term)
 			usage(blame_usage);
 	}
-- 
1.7.0.6

Re: [PATCH] blame: add a range option to -L

[Jonathan Nieder]

Bill Pemberton wrote:

> In addition to + and - you can now use r to specify how many lines
> around <start> that you want to see.  For example: -L 20,r5 would show
> lines 15 through 25

From too much exposure to subversion, the r suggests “revision”.
But ± is too hard to type, so I guess r will have to do.

> +++ b/Documentation/blame-options.txt
> @@ -32,6 +32,12 @@ This is only valid for <end> and will specify a number
>  of lines before or after the line given by <start>.
>  +
>  
> +- rrange

Maybe -rradius?  “range” sounds like an interval with specified endpoints.

> +		/* if top is negative then top is actually a range
> +		   that was specified with the r option */
> +		if (*top < 1) {
> +		    long x = *top;
> +		    *top = *bottom - x;
> +		    *bottom += x;
> +		    if (*bottom < 1)
> +			*bottom = 1;
> +		}

How does this treat -L 5,-10?

Jonathan

Re: [PATCH] blame: add a range option to -L

[Michael Witten]

On Sun, May 2, 2010 at 04:14, Jonathan Nieder <jrnieder@gmail.com> wrote:
>
> From too much exposure to subversion, the r suggests “revision”.
> But ± is too hard to type, so I guess r will have to do.

How about '=' ?

How about '%' (for above and below)?

Re: [PATCH] blame: add a range option to -L

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Bill Pemberton wrote:
>
>> In addition to + and - you can now use r to specify how many lines
>> around <start> that you want to see.  For example: -L 20,r5 would show
>> lines 15 through 25
>
>>From too much exposure to subversion, the r suggests “revision”.
> But ± is too hard to type, so I guess r will have to do.

I am not interested in this patch very much myself, and after thinking
about it, I think my dislike largely comes from this syntax, and not from
the feature itself.  Any non-punctuation letter there looks like a sore
thumb.  Either "-L 20±5" or even "-L 20+/-5" would have looked a _lot_
saner.

>> +++ b/Documentation/blame-options.txt
>> @@ -32,6 +32,12 @@ This is only valid for <end> and will specify a number
>>  of lines before or after the line given by <start>.
>>  +
>>  
>> +- rrange
>
> Maybe -rradius?  “range” sounds like an interval with specified endpoints.

Yes, radius sounds more sensible.  Another alternative would be to call it
"context", as in "grep -C <n>".

Re: [PATCH] blame: add a range option to -L

[Bill Pemberton]

> > In addition to + and - you can now use r to specify how many lines
> > around <start> that you want to see.  For example: -L 20,r5 would show
> > lines 15 through 25
> 
> >From too much exposure to subversion, the r suggests “revision”.
> But ± is too hard to type, so I guess r will have to do.
> 

I wasn't too thrilled with using r either, but I couldn't think of
anything better.


> > +		/* if top is negative then top is actually a range
> > +		   that was specified with the r option */
> > +		if (*top < 1) {
> > +		    long x = *top;
> > +		    *top = *bottom - x;
> > +		    *bottom += x;
> > +		    if (*bottom < 1)
> > +			*bottom = 1;
> > +		}
> 
> How does this treat -L 5,-10?
> 

Yes, you're correct, my code will mess that up.  I thought I had
verified that parse_loc() would never return a number less than 1 as
*top, but looking again I see I didn't do that.  So my patch is no
good.  Sorry folks.

-- 
Bill Pemberton                                 wfp5p@virginia.edu
ITC/Unix Systems                               flash@virginia.edu
University of Virginia

Re: [PATCH] blame: add a range option to -L

[Bill Pemberton]

Junio C Hamano writes:
> 
 
> I am not interested in this patch very much myself, and after thinking
> about it, I think my dislike largely comes from this syntax, and not from
> the feature itself.  Any non-punctuation letter there looks like a sore
> thumb.  Either "-L 20??15" or even "-L 20+/-5" would have looked a _lo=
> t_
> saner.
> 

Ok, I've never been thrilled with the r either, but I couldn't come up
with anything better and I had assumed that you'd want to stick with
the <start>,<end> syntax.

-- 
Bill