git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/6] blame: enable funcname blaming with userdiff driver
@ 2020-10-28 12:47 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
                   ` (6 more replies)
  0 siblings, 7 replies; 30+ messages in thread
From: Philippe Blain via GitGitGadget @ 2020-10-28 12:47 UTC (permalink / raw)
  To: git; +Cc: Thomas Rast, Eric Sunshine, René Scharfe, Philippe Blain

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.

Notes (I can't add in-patch commentaries using GitGitGadget (yet, I hope 1
[https://github.com/gitgitgadget/gitgitgadget/issues/173]), so I'm providing
what would have been an in-patch commentary on patch 6 below):

 * In patch 6, I considered fixing that bug in a different way by
   initializing sb.path inside blame.c::setup_scoreboard. This function
   already receives path as its second argument, so changing that would not
   impact the API. Probably Thomas thought that sb.path was already
   initialized in sb when he modified builtin/blame.c::prepare_blame_range 
   to also send sb.path to line-range.c::parse_range_arg in 13b8f68c1f (log
   -L: :pattern:file syntax to find by funcname, 2013-03-28). 
   
   Initializing the path in setup_scoreboard would mean we could also
   simplify the API of blame.c::setup_blame_bloom_data since it would not
   need to receive path separately and so its second argument could be
   removed. I went for the simpler alternative of just sending path to 
   parse_range_arg instead of sb.path since it felt simpler, but if we feel
   it would be better to actually initialize sb.path in setup_scoreboard,
   I'll gladly tweak that for v2.
   
   
 * 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 2
   [https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes] 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 ?

Philippe Blain (6):
  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

 Documentation/blame-options.txt      |  9 +++++----
 Documentation/diff-options.txt       |  5 ++++-
 Documentation/git-grep.txt           |  6 ++++--
 Documentation/git-log.txt            | 16 ++--------------
 Documentation/gitk.txt               | 21 ++-------------------
 Documentation/line-range-format.txt  | 28 +++++++++++++++-------------
 Documentation/line-range-options.txt | 27 +++++++++++++++++++++++++++
 builtin/blame.c                      |  5 +++--
 builtin/log.c                        |  4 ++--
 t/annotate-tests.sh                  | 18 ++++++++++++++++++
 10 files changed, 82 insertions(+), 57 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-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-774/phil-blain/blame-funcname-userdiff-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/774
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2020-11-01 17:29 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH v2 0/8] " Philippe Blain via GitGitGadget
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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).