{"thread":{"id":"23683","subject":"[PATCH v2] blame: add a range option to -L","startedAt":"2010-05-03T18:06:35Z","lastAt":"2010-05-04T18:11:47Z","messageCount":5,"participants":["Bill Pemberton","Michael Witten","Junio C Hamano","Jakub Narebski","Matthieu Moy"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"140840","messageId":"1272909995-3198-1-git-send-email-wfp5p@virginia.edu","threadId":"23683","inReplyTo":null,"subject":"[PATCH v2] blame: add a range option to -L","fromName":"Bill Pemberton","fromEmail":"wfp5p@virginia.edu","sentAt":"2010-05-03T18:06:35Z","receivedAt":"2010-05-03T18:06:35Z","isPatch":true,"sender":{"key":"wfp5p@virginia.edu","avatar":null},"body":"In addition to , you can now use
%\nto specify how many lines around
that you want to see.\nFor example: -L 20%5 would show lines 15 through 25\n\nSigned-off-by: Bill Pemberton \n---\n\nThis is like the previous patch to create a range option to -L in git-blame.\nHowever, this one uses -L%\n\nI chose to use % since it's on a standard keyboard.\n\n\n Documentation/blame-options.txt | 4 ++++\n Documentation/git-blame.txt | 8 ++++++++\n builtin/blame.c | 18 +++++++++++++++---\n 3 files changed, 27 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt\nindex d820569..f65e69c 100644\n--- a/Documentation/blame-options.txt\n+++ b/Documentation/blame-options.txt\n@@ -32,6 +32,10 @@ This is only valid for and will specify a number\n of lines before or after the line given by .\n +\n \n+-L
%::\n+\tThis works like , with the annotated range\n+\tcentered on
and showing lines around it.\n+\n -l::\n \tShow long rev (Default: off).\n \ndiff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt\nindex a27f439..73f6b83 100644\n--- a/Documentation/git-blame.txt\n+++ b/Documentation/git-blame.txt\n@@ -110,6 +110,14 @@ line 40):\n \tgit blame -L 40,60 foo\n \tgit blame -L 40,+21 foo\n \n+A range of lines around a particular line can be shown by using '%'\n+instead of ','. If you wanted to see line 20 along with the 5\n+lines around it:\n+\n+ git blame -L 20%5 foo\n+\n+\n+\n Also you can use a regular expression to specify the line range:\n \n \tgit blame -L '/^sub hello {/,/^}$/' foo\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex fc15863..eabc292 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1887,6 +1887,9 @@ static const char *parse_loc(const char *spec,\n \t/* Allow \"-L ,+20\" to mean starting at \n \t * for 20 lines, or \"-L ,-5\" for 5 lines ending at\n \t * .\n+\t *\n+\t * In addition \"-L %5\" means starting at\n+\t * -5 and ending at +5\n \t */\n \tif (1 < begin && (spec[0] == '+' || spec[0] == '-')) {\n \t\tnum = strtol(spec + 1, &term, 10);\n@@ -1958,10 +1961,19 @@ static void prepare_blame_range(struct scoreboard *sb,\n \tconst char *term;\n \n \tterm = parse_loc(bottomtop, sb, lno, 1, bottom);\n-\tif (*term == ',') {\n+\tif (*term == ',')\n+\t\tterm = parse_loc(term + 1, sb, lno, *bottom + 1, top);\n+\telse if (*term == '%') {\n+\t\tlong x;\n+\t\t/* ignore + or - if it's there */\n+\t\tif ((*(term+1) == '+') || (*(term+1) == '-'))\n+\t\t\tterm++;\n \t\tterm = parse_loc(term + 1, sb, lno, *bottom + 1, top);\n-\t\tif (*term)\n-\t\t\tusage(blame_usage);\n+\t\tx = *top;\n+\t\t*top = *bottom - x;\n+\t\t*bottom += x;\n+\t\tif (*bottom < 1)\n+\t\t\t*bottom = 1;\n \t}\n \tif (*term)\n \t\tusage(blame_usage);\n-- \n1.7.1\n"},{"id":"140841","messageId":"u2ib4087cc51005031123z9be44d50ra529f082a9eca3c2@mail.gmail.com","threadId":"23683","inReplyTo":"1272909995-3198-1-git-send-email-wfp5p@virginia.edu","subject":"Re: [PATCH v2] blame: add a range option to -L","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2010-05-03T18:23:53Z","receivedAt":"2010-05-03T18:23:53Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"On Mon, May 3, 2010 at 13:06, Bill Pemberton wrote:\n> +-L
%::\n> +       This works like , with the annotated range\n> +       centered on
and showing lines around it.\n\nI think the baggage of '%' might trip people up unless you add\nsomething like: \"Here, % symbolizes the notion of context above and\nbelow the center line\".\n"},{"id":"140925","messageId":"7vd3xbmv4w.fsf@alter.siamese.dyndns.org","threadId":"23683","inReplyTo":"1272909995-3198-1-git-send-email-wfp5p@virginia.edu","subject":"Re: [PATCH v2] blame: add a range option to -L","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-05-04T17:31:11Z","receivedAt":"2010-05-04T17:31:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bill Pemberton writes:\n\n> In addition to , you can now use
%\n> to specify how many lines around
that you want to see.\n> For example: -L 20%5 would show lines 15 through 25\n>\n> Signed-off-by: Bill Pemberton \n> ---\n\nPlease retitle, as (1) -L has always been about \"range\", and (2) what you\nare adding now is a \"radius\" option ;-)\n\n> +-L
%::\n> +\tThis works like , with the annotated range\n> +\tcentered on
and showing lines around it.\n\nI am not sure how \"like ,\" in this sentence helps the readers.\nIf you bring up the similarity, shouldn't you at least be saying that it\nis an shorthand to give \" lines before
\" as , and\n\" lines after
\" as , or somesuch?\n\n> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt\n> index a27f439..73f6b83 100644\n> --- a/Documentation/git-blame.txt\n> +++ b/Documentation/git-blame.txt\n> @@ -110,6 +110,14 @@ line 40):\n> \tgit blame -L 40,60 foo\n> \tgit blame -L 40,+21 foo\n> \n> +A range of lines around a particular line can be shown by using '%'\n> +instead of ','. If you wanted to see line 20 along with the 5\n> +lines around it:\n> +\n> + git blame -L 20%5 foo\n> +\n> +\n> +\n\nWhy this many blank lines around the example?\n\nI see this at the beginning of parse_loc() in builtin/blame.c:\n\n\t/* Allow \"-L ,+20\" to mean starting at \n\t * for 20 lines, or \"-L ,-5\" for 5 lines ending at\n\t * .\n\t */\n\nwhich means that it is not \"-L ,\" to begin with. I wonder if\nit makes the interface more consistent to rewrite the above comment like\nthis:\n\n\t/*\n\t * Allow \"-L ,+20\" to mean starting at \n\t * for 20 lines; \"-L ,-5\" for 5 lines ending at\n\t * ; and \"-L ,+-5\" for 5 lines around\n * .\n\t */\n\nand the match the code.\n"},{"id":"140926","messageId":"m3bpcvftjk.fsf@localhost.localdomain","threadId":"23683","inReplyTo":"7vd3xbmv4w.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] blame: add a range option to -L","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-05-04T17:48:31Z","receivedAt":"2010-05-04T17:48:31Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano writes:\n\n> I wonder if it makes the interface more consistent to rewrite the\n> above comment like this:\n> \n> \t/*\n> \t * Allow \"-L ,+20\" to mean starting at \n> \t * for 20 lines; \"-L ,-5\" for 5 lines ending at\n> \t * ; and \"-L ,+-5\" for 5 lines around\n> * .\n> \t */\n> \n> and the match the code.\n\nI like this.\n\n\nThe other approach would be to use -B / -A / -C [num], -\nconvention from 'grep'... but git-blame uses -C and - for other\nthings.\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"140927","messageId":"vpqk4rj8rks.fsf@bauges.imag.fr","threadId":"23683","inReplyTo":"1272909995-3198-1-git-send-email-wfp5p@virginia.edu","subject":"Re: [PATCH v2] blame: add a range option to -L","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-05-04T18:11:47Z","receivedAt":"2010-05-04T18:11:47Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Bill Pemberton writes:\n\n> \t\tterm = parse_loc(term + 1, sb, lno, *bottom + 1, top);\n> -\t\tif (*term)\n> -\t\t\tusage(blame_usage);\n> +\t\tx = *top;\n\nWhy not use parse_loc(..., &x) if you want the value to end up in x ?\n\n> +\t\t*top = *bottom - x;\n> +\t\t*bottom += x;\n\nThe existing code seems to assume that top >= bottom, but swaps top\nand bottom otherwise:\n\n\tif (bottom && top && top < bottom) {\n\t\tlong tmp;\n\t\ttmp = top; top = bottom; bottom = tmp;\n\t}\n\nSo, I'd write\n\n*top = *bottom + x;\n*bottom -= x;\n\n> +\t\tif (*bottom < 1)\n> +\t\t\t*bottom = 1;\n\nI guess you've assumed that bottom was the small number here,\notherwise, you're checking for overflow, not for actually negative\nnumbers. Either you apply my proposal above or you should\ns/bottom/top/ here, right?\n\n(the existing code already have this a few lines after the call to\nthis functions, it doesn't harm to do it again, but better do it on\nthe right function)\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}