| Message ID | 20200525215751.1735-1-philipoakley@iee.email (mailing list archive) |
|---|---|
| Headers | show |
| Series | Selectively show only blamed limes | expand |
On Mon, May 25, 2020 at 10:57:47PM +0100, Philip Oakley wrote: > While `git blame` is able to select interesting line regions of a > file, it's not easy to just display blame lines since a recent date, > especially for large files. I'm not convinced this is a useful thing to do in general. Lines don't always stand on their own, and you'd lack context for deciphering them. So a real example from "git blame -b --since=1.year.ago Makefile", for example (pardon the long lines): 3a94cb31d52 (Johannes Schindelin 2019-07-29 13:08:16 -0700 2734) -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \ but in context it looks like: (Junio C Hamano 2019-05-19 16:46:42 +0900 2730) bin-wrappers/%: wrap-for-bin.sh (Junio C Hamano 2019-05-19 16:46:42 +0900 2731) @mkdir -p bin-wrappers (Junio C Hamano 2019-05-19 16:46:42 +0900 2732) $(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ (Junio C Hamano 2019-05-19 16:46:42 +0900 2733) -e 's|@@BUILD_DIR@@|$(shell pwd)|' \ 3a94cb31d52 (Johannes Schindelin 2019-07-29 13:08:16 -0700 2734) -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \ (Junio C Hamano 2019-05-19 16:46:42 +0900 2735) chmod +x $@ Of course there are cases where it might be useful. But you can already do: git blame --since=1.year.ago Makefile | grep -v ^^ I'm not totally opposed to the feature. I'm just really struggling to see how it would be generally useful. For special cases where you're just counting up lines, you'd be more likely to post-process the result anyway, at which point --line-porcelain is often easier to work with. > Philip Oakley (4): > doc: blame: show the boundary commit '^' caret mark This doc fix seems worthwhile on its own, though. > blame: add option to show only blamed commits `--blame-only` > blame: do not show boundary commits, only those blamed If we do go this direction, these really ought to be a single commit. > blame: test the -b option, use blank oid for boundary commits. This one might be worth doing independently, too. -Peff
Hi Peff On 27/05/2020 08:23, Jeff King wrote: > On Mon, May 25, 2020 at 10:57:47PM +0100, Philip Oakley wrote: > >> While `git blame` is able to select interesting line regions of a >> file, it's not easy to just display blame lines since a recent date, >> especially for large files. > I'm not convinced this is a useful thing to do in general. Lines don't > always stand on their own, and you'd lack context for deciphering them. > So a real example from "git blame -b --since=1.year.ago Makefile", for > example (pardon the long lines): My contention is that there is a lot of effort needed _beforehand_ to reach that point. We've already had to page down 2700 line to get there, and then had to carefully select the differing before and after context lines. The use of the --blame-only (especially with broad date range --since) is to quickly narrow focus with the rather large file to the few areas of concern (probably related why the -b was introduced initially?) > > 3a94cb31d52 (Johannes Schindelin 2019-07-29 13:08:16 -0700 2734) -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \ > > but in context it looks like: > > (Junio C Hamano 2019-05-19 16:46:42 +0900 2730) bin-wrappers/%: wrap-for-bin.sh > (Junio C Hamano 2019-05-19 16:46:42 +0900 2731) @mkdir -p bin-wrappers > (Junio C Hamano 2019-05-19 16:46:42 +0900 2732) $(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ > (Junio C Hamano 2019-05-19 16:46:42 +0900 2733) -e 's|@@BUILD_DIR@@|$(shell pwd)|' \ > 3a94cb31d52 (Johannes Schindelin 2019-07-29 13:08:16 -0700 2734) -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \ > (Junio C Hamano 2019-05-19 16:46:42 +0900 2735) chmod +x $@ > > Of course there are cases where it might be useful. But you can already > do: > > git blame --since=1.year.ago Makefile | grep -v ^^ > > I'm not totally opposed to the feature. I'm just really struggling to > see how it would be generally useful. For special cases where you're > just counting up lines, you'd be more likely to post-process the result > anyway, at which point --line-porcelain is often easier to work with. I'm also thinking that use of Git has expanded well beyond its core Linux VCS roots, so not all users are ready for the grep regex [1]. Hence the expansion of the `-b` to truly filter only the blamed commits. > >> Philip Oakley (4): >> doc: blame: show the boundary commit '^' caret mark > This doc fix seems worthwhile on its own, though. > >> blame: add option to show only blamed commits `--blame-only` >> blame: do not show boundary commits, only those blamed > If we do go this direction, these really ought to be a single commit. > >> blame: test the -b option, use blank oid for boundary commits. > This one might be worth doing independently, too. OK. The final test look a while to come up with, so it ended up last. > > -Peff Thanks Philip [1] https://stackoverflow.com/a/42540014/717355 "Git Blame see changes after a certain date" (longer regex for the same effect)
On Wed, May 27, 2020 at 11:24:54AM +0100, Philip Oakley wrote: > > I'm not convinced this is a useful thing to do in general. Lines don't > > always stand on their own, and you'd lack context for deciphering them. > > So a real example from "git blame -b --since=1.year.ago Makefile", for > > example (pardon the long lines): > > My contention is that there is a lot of effort needed _beforehand_ to > reach that point. We've already had to page down 2700 line to get there, > and then had to carefully select the differing before and after context > lines. I think: git blame -b --since=1.year.ago ... | less "+/^[^ ]" works pretty well to get you to the right spot, with context (and then "/<enter>" within less takes you to the next line). But that's also pretty magical. > The use of the --blame-only (especially with broad date range --since) > is to quickly narrow focus with the rather large file to the few areas > of concern (probably related why the -b was introduced initially?) Perhaps. I think when you get an out-of-context chunk, though, it's tricky to take the next step. I'm still skeptical this will turn out to be all that useful in practice. But it really isn't much code to carry, so I don't have any real objection. -Peff
Jeff King <peff@peff.net> writes: > On Wed, May 27, 2020 at 11:24:54AM +0100, Philip Oakley wrote: > >> > I'm not convinced this is a useful thing to do in general. Lines don't >> > always stand on their own, and you'd lack context for deciphering them. >> > So a real example from "git blame -b --since=1.year.ago Makefile", for >> > example (pardon the long lines): >> >> My contention is that there is a lot of effort needed _beforehand_ to >> reach that point. We've already had to page down 2700 line to get there, >> and then had to carefully select the differing before and after context >> lines. > > I think: > > git blame -b --since=1.year.ago ... | less "+/^[^ ]" > > works pretty well to get you to the right spot, with context (and then > "/<enter>" within less takes you to the next line). Yup. I think this is mostly about knowing how to use the tools that already exist. I am not interested in this new "feature", either. Thanks.
On 28/05/2020 19:30, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > >> On Wed, May 27, 2020 at 11:24:54AM +0100, Philip Oakley wrote: >> >>>> I'm not convinced this is a useful thing to do in general. Lines don't >>>> always stand on their own, and you'd lack context for deciphering them. >>>> So a real example from "git blame -b --since=1.year.ago Makefile", for >>>> example (pardon the long lines): >>> My contention is that there is a lot of effort needed _beforehand_ to >>> reach that point. We've already had to page down 2700 line to get there, >>> and then had to carefully select the differing before and after context >>> lines. >> I think: >> >> git blame -b --since=1.year.ago ... | less "+/^[^ ]" >> >> works pretty well to get you to the right spot, with context (and then >> "/<enter>" within less takes you to the next line). > Yup. I think this is mostly about knowing how to use the tools that > already exist. I am not interested in this new "feature", either. > > Thanks. Is the presumption of knowing the *nix tools something that should documented, or is that an over simplification? I'll trim this series to just the '^' documentation change and the '-b' option test (after v2.27.0) Philip
Jeff King <peff@peff.net> writes: > I think: > > git blame -b --since=1.year.ago ... | less "+/^[^ ]" > > works pretty well to get you to the right spot, with context (and then > "/<enter>" within less takes you to the next line). > > But that's also pretty magical. Instead of teaching each and every tool (like "git blame", "diff", "grep", etc.) a special trick to remove the lines from its output, it may make a lot more sense to add a feature to pagers that lets the users selectively "hide" the matching lines given a pattern. Then when viewing "git blame -b" output, you can first declare "lines that begin with a whitespace are sometimes uninteresting" and then tell your pager "now unshow uninteresting lines", "the same, but show 3 lines at the boundary of runs of uninteresting lines", "now show everything", etc. That's another reason why I do not particularly find the proposed feature interesting.
Junio C Hamano <gitster@pobox.com> writes: > Instead of teaching each and every tool (like "git blame", "diff", > "grep", etc.) a special trick to remove the lines from its output, > it may make a lot more sense to add a feature to pagers that lets > the users selectively "hide" the matching lines given a pattern. > Then when viewing "git blame -b" output, you can first declare > "lines that begin with a whitespace are sometimes uninteresting" and > then tell your pager "now unshow uninteresting lines", "the same, but > show 3 lines at the boundary of runs of uninteresting lines", "now > show everything", etc. I forgot to say what I wanted to touch a bit more. Such a feature in pager is readily reusable for other types of payload. "Lines that begin with a whitespace are sometimes uninteresting" can also be used for output from "diff -u -U<largenumber>", and "lines that do not match '^[^:]*:[0-9]*:' are not interesting" can be used for output from "grep n -C". That would be a lot more sensible than tweaking each and every tool that generates output that sometimes needs to be read in full and sometimes needs to be shown with less interesting parts ellided.