All of lore.kernel.org
 help / color / mirror / Atom feed
From: Philip Oakley <philipoakley@iee.email>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Sibi Siddharthan <sibisiddharthan.github@gmail.com>
Subject: Re: [PATCH 0/4] Selectively show only blamed limes
Date: Wed, 27 May 2020 11:24:54 +0100	[thread overview]
Message-ID: <0da84168-3d81-fb14-32ad-010ef2436958@iee.email> (raw)
In-Reply-To: <20200527072318.GA4006199@coredump.intra.peff.net>

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)

  reply	other threads:[~2020-05-27 10:24 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-25 21:57 [PATCH 0/4] Selectively show only blamed limes Philip Oakley
2020-05-25 21:57 ` [PATCH 1/4] doc: blame: show the boundary commit '^' caret mark Philip Oakley
2020-05-25 21:57 ` [PATCH 2/4] blame: add option to show only blamed commits `--blame-only` Philip Oakley
2020-05-25 21:57 ` [PATCH 3/4] blame: do not show boundary commits, only those blamed Philip Oakley
2020-05-25 21:57 ` [PATCH 4/4] blame: test the -b option, use blank oid for boundary commits Philip Oakley
2020-05-27  7:30   ` Jeff King
2020-05-27 10:52     ` Philip Oakley
2020-05-27  7:23 ` [PATCH 0/4] Selectively show only blamed limes Jeff King
2020-05-27 10:24   ` Philip Oakley [this message]
2020-05-28 16:45     ` Jeff King
2020-05-28 18:30       ` Junio C Hamano
2020-05-30 16:18         ` Philip Oakley
2020-05-31 16:02       ` Junio C Hamano
2020-05-31 16:28         ` Junio C Hamano

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=0da84168-3d81-fb14-32ad-010ef2436958@iee.email \
    --to=philipoakley@iee.email \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=sibisiddharthan.github@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.