From: "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Philippe Blain <levraiphilippeblain@gmail.com>,
Philippe Blain <levraiphilippeblain@gmail.com>
Subject: [PATCH v2 0/8] blame: enable funcname blaming with userdiff driver
Date: Sun, 01 Nov 2020 17:28:39 +0000 [thread overview]
Message-ID: <pull.774.v2.git.1604251727.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.774.git.1603889270.gitgitgadget@gmail.com>
Changes since v1:
* patch 1: tweaked the message and took Junio's suggestion to only list the
stuck form if the option in 'git-log' and 'gitk'.
* patch 2: very minor formatting changes (removed double quotes)
* patch 3: removed the parentheses as suggested by Junio, and did the same
for git-log/gitk.
* patch 4: grammar fix pointed out by Eric
* patch 5: shortened the help text as suggested by Eric
* patch 6: changed the approach by initializing 'sb.path' earlier in
'cmd_blame', along with some of sb's other fields.
* patches 7-8: new cleanup patches following the new patch 6 to simplify
the interface of two functions in 'blame.c'
----------------------------------------------------------------------------
This series fixes a bug in git blame where any userdiff driver configured
via the attributes mechanism is ignored, which more often than not means
thatgit blame -L :<funcname> dies for paths that have a configured userdiff
driver. That's patch 6/6.
Patches 1-5 are documentation improvements around the line-log
functionality.
Patches 7-8 are code clean-up patches following the changes in patch 6.
The fact that this bug has been present ever since git blame learned -L
:<funcname> makes me a little sad. I think that the fact that the diff
attribute (either with a builtin userdiff pattern or with diff.*.xfuncname )
influences way more than just the hunk header, but also features like git
log -L, git blame -L, and the different options that I mention in patch 4/6,
is not well-known enough. I'm not saying the code or the doc is wrong, but I
think it's a visibility issue. Maybe more blog posts about that would
help.... I noticed that the "Git Attributes" chapter in the ProGit book [1]
does not even mention the builtin userdiff patterns, so I'll probably do a
PR there to mention it. Any other ideas for improving discoverability of
this very cool feature ?
[1] https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes
CC: Thomas Rast tr@thomasrast.ch [tr@thomasrast.ch], Eric Sunshine
sunshine@sunshineco.com [sunshine@sunshineco.com], René Scharfe l.s.r@web.de
[l.s.r@web.de]
Philippe Blain (8):
doc: log, gitk: move '-L' description to 'line-range-options.txt'
doc: line-range: improve formatting
blame-options.txt: also mention 'funcname' in '-L' description
doc: add more pointers to gitattributes(5) for userdiff
line-log: mention both modes in 'blame' and 'log' short help
blame: enable funcname blaming with userdiff driver
blame: simplify 'setup_scoreboard' interface
blame: simplify 'setup_blame_bloom_data' interface
Documentation/blame-options.txt | 9 +++++----
Documentation/diff-options.txt | 5 ++++-
Documentation/git-grep.txt | 6 ++++--
Documentation/git-log.txt | 15 +--------------
Documentation/gitk.txt | 20 +-------------------
Documentation/line-range-format.txt | 28 +++++++++++++++-------------
Documentation/line-range-options.txt | 15 +++++++++++++++
blame.c | 16 +++++++---------
blame.h | 4 +---
builtin/blame.c | 11 ++++++-----
builtin/log.c | 4 ++--
t/annotate-tests.sh | 18 ++++++++++++++++++
12 files changed, 79 insertions(+), 72 deletions(-)
create mode 100644 Documentation/line-range-options.txt
base-commit: 1d1c4a875900d69c7f0a31e44c3e370dc80ab1ce
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-774%2Fphil-blain%2Fblame-funcname-userdiff-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-774/phil-blain/blame-funcname-userdiff-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/774
Range-diff vs v1:
1: 96f6f95abc ! 1: b4fa5fb5ae doc: log, gitk: move '-L' description to 'line-range-options.txt'
@@ Metadata
## Commit message ##
doc: log, gitk: move '-L' description to 'line-range-options.txt'
- The description of the '-L' option for `git log` and `gitk` is the
- same, but is repeated in both 'git-log.txt' and 'gitk.txt'.
+ The description of the '-L' option for `git log` and `gitk` is almost
+ the same, but is repeated in both 'git-log.txt' and 'gitk.txt' (the
+ difference being that 'git-log.txt' lists the option with a space
+ after '-L', while 'gitk.txt' lists it as stuck and notes that `gitk`
+ only understands the stuck form).
- Remove the duplication by creating a new file, 'line-range-options.txt',
+ Reduce duplication by creating a new file, 'line-range-options.txt',
and include it in both files.
+ To simplify the presentation, only list the stuck form for both
+ commands, and remove the note about `gitk` only understanding the stuck
+ form.
+
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
## Documentation/git-log.txt ##
@@ Documentation/git-log.txt: produced by `--stat`, etc.
- `--name-only`, `--name-status`, `--check`) are not currently implemented.
-+
-include::line-range-format.txt[]
-+:git-log: 1
+include::line-range-options.txt[]
<revision range>::
@@ Documentation/gitk.txt: linkgit:git-rev-list[1] for a complete list.
-*not* put a space after `-L`.
-+
-include::line-range-format.txt[]
-+:gitk: 1
+include::line-range-options.txt[]
<revision range>::
@@ Documentation/gitk.txt: linkgit:git-rev-list[1] for a complete list.
## Documentation/line-range-options.txt (new) ##
@@
-+ifdef::git-log[]
-+-L <start>,<end>:<file>::
-+-L :<funcname>:<file>::
-+endif::git-log[]
-+ifdef::gitk[]
+-L<start>,<end>:<file>::
+-L:<funcname>:<file>::
-+endif::gitk[]
+
+ Trace the evolution of the line range given by "<start>,<end>"
+ (or the function name regex <funcname>) within the <file>. You may
@@ Documentation/line-range-options.txt (new)
+ (namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,
+ `--name-only`, `--name-status`, `--check`) are not currently implemented.
++
-+ifdef::gitk[]
-+*Note:* gitk (unlike linkgit:git-log[1]) currently only understands
-+this option if you specify it "glued together" with its argument. Do
-+*not* put a space after `-L`.
-+endif::gitk[]
-++
+include::line-range-format.txt[]
2: 7d3fc0a503 ! 2: 6c44b1c576 doc: line-range: improve formatting
@@ Documentation/line-range-format.txt
+
-If ``:<funcname>'' is given in place of <start> and <end>, it is a
-+If "`:<funcname>`" is given in place of '<start>' and '<end>', it is a
++If `:<funcname>` is given in place of '<start>' and '<end>', it is a
regular expression that denotes the range from the first funcname line
-that matches <funcname>, up to the next funcname line. ``:<funcname>''
+that matches '<funcname>', up to the next funcname line. `:<funcname>`
@@ Documentation/line-range-format.txt
file.
## Documentation/line-range-options.txt ##
-@@ Documentation/line-range-options.txt: ifdef::gitk[]
+@@
+ -L<start>,<end>:<file>::
-L:<funcname>:<file>::
- endif::gitk[]
- Trace the evolution of the line range given by "<start>,<end>"
- (or the function name regex <funcname>) within the <file>. You may
-+ Trace the evolution of the line range given by "'<start>,<end>'"
++ Trace the evolution of the line range given by '<start>,<end>'
+ (or the function name regex '<funcname>') within the '<file>'. You may
not give any pathspec limiters. This is currently limited to
a walk starting from a single revision, i.e., you may only
3: f7b64bf330 ! 3: d38f0f7e26 blame-options.txt: also mention 'funcname' in '-L' description
@@ Commit message
'-L :<funcname>' by mentioning it at the beginnning of the description
of the '-L' option.
+ Also, in 'line-range-options.txt', which is used for git-log(1) and
+ gitk(1), do not parenthesize the mention of the ':<funcname>' mode, to
+ place it on equal footing with the '<start>,<end>' mode.
+
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
## Documentation/blame-options.txt ##
@@ Documentation/blame-options.txt
-L :<funcname>::
- Annotate only the given line range. May be specified multiple times.
- Overlapping ranges are allowed.
-+ Annotate only the line range given by '<start>,<end>'
-+ (or by the function name regex '<funcname>').
++ Annotate only the line range given by '<start>,<end>',
++ or by the function name regex '<funcname>'.
+ 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>'.
+
+ ## Documentation/line-range-options.txt ##
+@@
+ -L<start>,<end>:<file>::
+ -L:<funcname>:<file>::
+
+- Trace the evolution of the line range given by '<start>,<end>'
+- (or the function name regex '<funcname>') within the '<file>'. You may
++ Trace the evolution of the line range given by '<start>,<end>',
++ or by the function name regex '<funcname>', within the '<file>'. You may
+ not give any pathspec limiters. This is currently limited to
+ a walk starting from a single revision, i.e., you may only
+ give zero or one positive revision arguments, and
4: da76a10717 ! 4: 9443a681e3 doc: add more pointers to gitattributes(5) for userdiff
@@ Documentation/diff-options.txt: endif::git-format-patch[]
-W::
--function-context::
- Show whole surrounding functions of changes.
-+ Show whole functions as context lines for each changes.
++ Show whole function as context lines for each change.
+ The function names are determined in the same way as
+ `git diff` works out patch hunk headers (see 'Defining a
+ custom hunk-header' in linkgit:gitattributes[5]).
5: 27ef94e9cc ! 5: ae61d27a38 line-log: mention both modes in 'blame' and 'log' short help
@@ builtin/blame.c: int cmd_blame(int argc, const char **argv, const char *prefix)
OPT_CALLBACK_F('M', NULL, &opt, N_("score"), N_("Find line movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback),
- OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")),
+ OPT_STRING_LIST('L', NULL, &range_list, N_("range"),
-+ N_("Process only the given line range (<range>=<start>,<end>) or function (<range>=:<funcname>)")),
++ N_("Process only line range <start>,<end> or function :<funcname>")),
OPT__ABBREV(&abbrev),
OPT_END()
};
@@ builtin/log.c: static void cmd_log_init_finish(int argc, const char **argv, cons
- OPT_CALLBACK('L', NULL, &line_cb, "n,m:file",
- N_("Process line range n,m in file, counting from 1"),
+ OPT_CALLBACK('L', NULL, &line_cb, "range:file",
-+ N_("Trace the evolution of the given line range (<range>=<start>,<end>) or function (<range>=:<funcname>) in <file>"),
++ N_("Trace the evolution of line range <start>,<end> or function :<funcname> in <file>"),
log_line_range_callback),
OPT_END()
};
6: a1e1c977d0 ! 6: c77108e864 blame: enable funcname blaming with userdiff driver
@@ Commit message
funcname, 2013-03-28).
Enable funcname blaming for paths using specific userdiff drivers by
- sending the local variable 'path' to 'parse_range_arg' instead of the
- yet unset 'sb.path'.
+ initializing 'sb.path' earlier in 'cmd_blame', when some of its other
+ fields are initialized, so that it is set when passed to
+ 'parse_range_arg'.
Add a regression test in 'annotate-tests.sh', which is sourced in
t8001-annotate.sh and t8002-blame.sh, leveraging an existing file used
to test the userdiff patterns in t4018-diff-funcname.
+ Also, use 'sb.path' instead of 'path' when constructing the error
+ message at line 1114, for consistency.
+
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
## builtin/blame.c ##
@@ builtin/blame.c: int cmd_blame(int argc, const char **argv, const char *prefix)
- long bottom, top;
- if (parse_range_arg(range_list.items[range_i].string,
- nth_line_cb, &sb, lno, anchor,
-- &bottom, &top, sb.path,
-+ &bottom, &top, path,
- the_repository->index))
- usage(blame_usage);
+ sb.contents_from = contents_from;
+ sb.reverse = reverse;
+ sb.repo = the_repository;
++ sb.path = path;
+ build_ignorelist(&sb, &ignore_revs_file_list, &ignore_rev_list);
+ string_list_clear(&ignore_revs_file_list, 0);
+ string_list_clear(&ignore_rev_list, 0);
+@@ builtin/blame.c: int cmd_blame(int argc, const char **argv, const char *prefix)
if ((!lno && (top || bottom)) || lno < bottom)
+ die(Q_("file %s has only %lu line",
+ "file %s has only %lu lines",
+- lno), path, lno);
++ lno), sb.path, lno);
+ if (bottom < 1)
+ bottom = 1;
+ if (top < 1 || lno < top)
+@@ builtin/blame.c: int cmd_blame(int argc, const char **argv, const char *prefix)
+ string_list_clear(&range_list, 0);
+
+ sb.ent = NULL;
+- sb.path = path;
+
+ if (blame_move_score)
+ sb.move_score = blame_move_score;
## t/annotate-tests.sh ##
@@ t/annotate-tests.sh: test_expect_success 'blame -L ^:RE (absolute: end-of-file)' '
-: ---------- > 7: 3b28696e51 blame: simplify 'setup_scoreboard' interface
-: ---------- > 8: 7aca3274d2 blame: simplify 'setup_blame_bloom_data' interface
--
gitgitgadget
next prev parent reply other threads:[~2020-11-01 17:28 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-28 12:47 [PATCH 0/6] blame: enable funcname blaming with userdiff driver Philippe Blain via GitGitGadget
2020-10-28 12:47 ` [PATCH 1/6] doc: log, gitk: move '-L' description to 'line-range-options.txt' Philippe Blain via GitGitGadget
2020-10-29 20:16 ` Junio C Hamano
2020-10-31 17:18 ` Philippe Blain
2020-10-31 17:29 ` Junio C Hamano
2020-10-28 12:47 ` [PATCH 2/6] doc: line-range: improve formatting Philippe Blain via GitGitGadget
2020-10-28 17:23 ` Eric Sunshine
2020-10-29 20:19 ` Junio C Hamano
2020-10-31 17:20 ` Philippe Blain
2020-10-28 12:47 ` [PATCH 3/6] blame-options.txt: also mention 'funcname' in '-L' description Philippe Blain via GitGitGadget
2020-10-29 20:25 ` Junio C Hamano
2020-10-31 17:22 ` Philippe Blain
2020-10-28 12:47 ` [PATCH 4/6] doc: add more pointers to gitattributes(5) for userdiff Philippe Blain via GitGitGadget
2020-10-28 17:26 ` Eric Sunshine
2020-10-28 12:47 ` [PATCH 5/6] line-log: mention both modes in 'blame' and 'log' short help Philippe Blain via GitGitGadget
2020-10-28 17:33 ` Eric Sunshine
2020-10-29 12:43 ` Philippe Blain
2020-10-28 12:47 ` [PATCH 6/6] blame: enable funcname blaming with userdiff driver Philippe Blain via GitGitGadget
2020-10-29 20:40 ` Junio C Hamano
2020-10-31 18:02 ` Philippe Blain
2020-10-31 18:58 ` Junio C Hamano
2020-11-01 17:28 ` Philippe Blain via GitGitGadget [this message]
2020-11-01 17:28 ` [PATCH v2 1/8] doc: log, gitk: move '-L' description to 'line-range-options.txt' Philippe Blain via GitGitGadget
2020-11-01 17:28 ` [PATCH v2 2/8] doc: line-range: improve formatting Philippe Blain via GitGitGadget
2020-11-01 17:28 ` [PATCH v2 3/8] blame-options.txt: also mention 'funcname' in '-L' description Philippe Blain via GitGitGadget
2020-11-01 17:28 ` [PATCH v2 4/8] doc: add more pointers to gitattributes(5) for userdiff Philippe Blain via GitGitGadget
2020-11-01 17:28 ` [PATCH v2 5/8] line-log: mention both modes in 'blame' and 'log' short help Philippe Blain via GitGitGadget
2020-11-01 17:28 ` [PATCH v2 6/8] blame: enable funcname blaming with userdiff driver Philippe Blain via GitGitGadget
2020-11-01 17:28 ` [PATCH v2 7/8] blame: simplify 'setup_scoreboard' interface Philippe Blain via GitGitGadget
2020-11-01 17:28 ` [PATCH v2 8/8] blame: simplify 'setup_blame_bloom_data' interface Philippe Blain via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.774.v2.git.1604251727.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=levraiphilippeblain@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.