Re: [PATCH] blame: fix unblamable and ignored lines in porcelain mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Toon Claes <toon@xxxxxxxxx> writes:

> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>
>> I'm curious, how would it be different, if they blame down to the same
>> commit? My understanding was "unblamable" and "ignored" are tied to
>> commits.

[snip]

> And now with the `.git-blame-ignore-revs` file:
>
>     $ git -c blame.markUnblamableLines=true -c blame.markIgnoredLines=true blame varint.h --ignore-revs-file .git-blame-ignore-revs -l --porcelain
>     d2c1898571a6a2324593e92163e8754880e0c1fb 1 1 3
>     author Junio C Hamano
>     author-mail <gitster@xxxxxxxxx>
>     author-time 1333493588
>     author-tz -0700
>     committer Junio C Hamano
>     committer-mail <gitster@xxxxxxxxx>
>     committer-time 1333495484
>     committer-tz -0700
>     summary varint: make it available outside the context of pack
>     filename varint.h
>             #ifndef VARINT_H
>     d2c1898571a6a2324593e92163e8754880e0c1fb 2 2
>             #define VARINT_H
>     d2c1898571a6a2324593e92163e8754880e0c1fb 3 3
>
>     d2c1898571a6a2324593e92163e8754880e0c1fb 6 4 2
>             int encode_varint(uintmax_t, unsigned char *);
>     d2c1898571a6a2324593e92163e8754880e0c1fb 7 5
>             uintmax_t decode_varint(const unsigned char **);
>     d2c1898571a6a2324593e92163e8754880e0c1fb 8 6 2
>
>     d2c1898571a6a2324593e92163e8754880e0c1fb 9 7
>             #endif /* VARINT_H */
>
> So every line now blames down to commit
> d2c1898571a6a2324593e92163e8754880e0c1fb. The lines which used to
> blame down to 554544276a604c144df45efcb060c80aa322088c should be marked
> as "ignored", but we only emit the details once for each commit. The
> commit details (author, committer) are only relevant once, but the
> "ignored" info can differ for each line (as you also can see in the
> non-porcelain format).
>

Ah! So if a rev is ignored via the `--ignore-rev[s-file]` flag, then the
parent revision is shown in the blame. It could happen that in porcelain
mode the parent revision is clubbed with previous lines if they share
the same revision. This would skip the 'unblamable' or 'ignored'
information.

This would be solved in '--line-porcelain' since details aren't clubbed.

I agree, it makes the most sense to only do this in 'line-porcelain'
mode.

> We could make the output look something like:
>
>     $ git -c blame.markUnblamableLines=true -c blame.markIgnoredLines=true blame varint.h --ignore-revs-file .git-blame-ignore-revs -l --porcelain
>     d2c1898571a6a2324593e92163e8754880e0c1fb 1 1 3
>     author Junio C Hamano
>     author-mail <gitster@xxxxxxxxx>
>     author-time 1333493588
>     author-tz -0700
>     committer Junio C Hamano
>     committer-mail <gitster@xxxxxxxxx>
>     committer-time 1333495484
>     committer-tz -0700
>     summary varint: make it available outside the context of pack
>     filename varint.h
>             #ifndef VARINT_H
>     d2c1898571a6a2324593e92163e8754880e0c1fb 2 2
>             #define VARINT_H
>     d2c1898571a6a2324593e92163e8754880e0c1fb 3 3
>
>     d2c1898571a6a2324593e92163e8754880e0c1fb 6 4 2
>     ignored
>             int encode_varint(uintmax_t, unsigned char *);
>     d2c1898571a6a2324593e92163e8754880e0c1fb 7 5
>     ignored
>             uintmax_t decode_varint(const unsigned char **);
>     d2c1898571a6a2324593e92163e8754880e0c1fb 8 6 2
>
>     d2c1898571a6a2324593e92163e8754880e0c1fb 9 7
>             #endif /* VARINT_H */
>
> It feels odd to me only the "ignored" info is emitted and the rest
> of the details isn't. But that might be just me...
>

I'm with you on this, this would also require us to explain this odd
exclusion where only for 'ignored' and 'unblamable' lines we output
details on every commit. But the other way is also an exclusion, where
we would say that 'ignored' and 'unblamable' lines are only shown in
'--line-porcelain'.

But the latter can be extended into the former in the future but not the
other way around. So I would say it makes more sense to restrict it to
'--line-porcelain' in that sense.

> --
> Toon

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux