Re: [PATCH 2/2] Documentation/git-status: document porcelain status T (typechange)

From: Johannes Altmanninger <aclopte@gmail.com>
Date: Oct 4, 2021, 18:57 UTC
Message-ID: <20211004185741.rktidu2ptn4v5kll@gmail.com>
In-Reply-To: <CABPp-BFqJi0q0844NKfU8K5HRpAJ7yP2OkyBUiFQSAw-ZjNoyw@mail.gmail.com>

Thanks for the review, I'll send v2 which addresses comments and makes some minor hanges to the commit messages.

On Sat, Oct 02, 2021 at 04:29:52PM -0700, Elijah Newren wrote:

> > A porcelain status containing C (copied) is impossible because "git
> > status" does not detect copies, only renames.
> 
> This is no true (since ~2018); here's an example:

Ah my bad. Turns out skimming git-status(1) was not enough. I added patch 4/4 to make that easier (or at least document that it *is* possible).

Show 11 quoted lines

> 
>     $ cp -a README.md README-copy
>     $ echo garbage >>README.md
>     $ git add README-copy README.md
>     $ git -c status.renames=copy status --porcelain
>     C  README.md -> README-copy
>     M  README.md
> 
> You can also use diff.renames instead of status.renames, since the
> latter defaults to the former.  Note that you do need to both modify
> and stage README.md in the example above to see the copy status.

Wow yeah, this is quite a special case.

Show 15 quoted lines

> 
> > I was going to delete
> > mentions of C from git-status.txt because it keeps confusing users [2]
> > but a discussion from 2014 suggests that "git status" should re-learn
> > to detect copies, which was disabled in 2005 for (obsolete) performance
> > reasons [3].
> 
> That thread you refer to suggests it was turned off because copy
> detection meant the equivalent of find-copies-harder, and that thread
> also provided numbers showing find-copies-harder would still be very
> painful performance-wise.  Perhaps the "obsolete performance reasons"
> was meant to imply that basic copy detection is cheaper since it does
> something different today than what status's copy detection did back
> then, but summarizing this as "obsolete performance reasons" feels
> misleading to me.

Right, I should have acknowledged the change to copy detection.

Show 9 quoted lines

> 
> Links to lore.kernel.org would be much preferred to marc.info links;
> here that would be
> https://lore.kernel.org/git/20141202200910.GB23461@peff.net/.
> 
> The lore.kernel.org links provide an interface to easily search for
> other mailing list messages, and use the Message-ID in the URL which
> makes it easier for people to find the message in other locations,
> etc.

got it

Show 19 quoted lines

> 
> >  [ D]        R    renamed in work tree
> >  [ D]        C    copied in work tree
> 
> This wasn't something you added; it appears these two came from commit
> 176ea74793 ("wt-status.c: handle worktree renames", 2017-12-27).
> However, I don't think the 'D' part of these examples is possible.  If
> a file is deleted in the index relative to HEAD (what the 'D' means),
> then comparing the index to the worktree means the file didn't even
> exist in the index.  Thus no delete pair for that file will be passed
> to diffcore-rename, and without a delete pair for some file, there is
> nothing for the add pairs to be combined with to create a rename or
> copy pair.  So these lines are misleading and should only have a space
> in the first column rather than either a space or 'D'.
> 
> That said, of course, since this wasn't caused by your patch, you are
> under no obligation to fix.  It would go nicely with your series,
> though.  Would you like to add another patch to your series to fix
> that, or would you rather that I contributed such a patch separately?

Ok I've added patch 1/4