diff mbox series

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

Message ID 20211002213046.725892-2-aclopte@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/2] Documentation/diff-format: state in which cases porcelain status is T | expand

Commit Message

Johannes Altmanninger Oct. 2, 2021, 9:30 p.m. UTC
As reported in [1], T is missing from the description of porcelain
status letters in git-status(1) (whereas T is documented in
git-diff-files(1) and friends). Document T right after M (modified)
because the two are very similar.

A porcelain status containing C (copied) is impossible because "git
status" does not detect copies, only renames. 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].

[1] https://github.com/fish-shell/fish-shell/issues/8311
[2] https://www.reddit.com/r/git/comments/ppc2l9/how_to_get_a_file_with_copied_status/
[3] https://marc.info/?l=git&m=141755095826447&w=2

Signed-off-by: Johannes Altmanninger <aclopte@gmail.com>
---
 Documentation/git-status.txt | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

Comments

Johannes Altmanninger Oct. 2, 2021, 9:33 p.m. UTC | #1
In case anyone is curious and lazy, here is a script that generates all
possible status letters containing T:

#!/bin/sh

set -e

_T() {
	touch file-1.T
	git add file-1.T
	git commit -q -m add
	rm file-1.T
	ln -s /dev/null file-1.T
}

MT() {
	touch file-1MT
	git add file-1MT
	git commit -q -m add
	echo modify > file-1MT
	git add file-1MT
	ln -sf /dev/null file-1MT
}

TM() {
	ln -sf /dev/null file-1TM
	git add file-1TM
	git commit -q -m add
	rm file-1TM
	touch file-1TM
	git add file-1TM
	echo modify > file-1TM
}

TT() {
	touch file-1TT
	git add file-1TT
	git commit -q -m add
	ln -sf /dev/null file-1TT
	git add file-1TT
	rm file-1TT
	touch file-1TT
}

TD() {
	touch file-1TD
	git add file-1TD
	git commit -q -m add
	ln -sf /dev/null file-1TD
	git add file-1TD
	rm file-1TD
}

AT() {
	touch file-1AT
	git add file-1AT
	git commit -q -m add
	ln -sf /dev/null file-1AT
}

RT() {
	touch file-2RT
	git add file-2RT
	git commit -q -m add
	git mv file-2RT new-file-2RT
	ln -sf /dev/null new-file-2RT
}

T_() {
	touch file-1T.
	git add file-1T.
	git commit -q -m add
	ln -sf /dev/null file-1T.
	git add file-1T.
}

cd "$(mktemp -d)"
git init -q
git commit -q --allow-empty -m initial\ commit

for state in _T MT TM TT TD AT RT T_
do
	git reset -q --hard :/initial.commit
	"$state"
	git status --porcelain=2
done
Elijah Newren Oct. 2, 2021, 11:29 p.m. UTC | #2
On Sat, Oct 2, 2021 at 2:38 PM Johannes Altmanninger <aclopte@gmail.com> wrote:
>
> As reported in [1], T is missing from the description of porcelain
> status letters in git-status(1) (whereas T is documented in
> git-diff-files(1) and friends). Document T right after M (modified)
> because the two are very similar.

Good; thanks for sending this in.

> 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:

    $ 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.

> 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.

> [1] https://github.com/fish-shell/fish-shell/issues/8311
> [2] https://www.reddit.com/r/git/comments/ppc2l9/how_to_get_a_file_with_copied_status/
> [3] https://marc.info/?l=git&m=141755095826447&w=2

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.

> Signed-off-by: Johannes Altmanninger <aclopte@gmail.com>
> ---
>  Documentation/git-status.txt | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
> index 83f38e3198..40f308c6a6 100644
> --- a/Documentation/git-status.txt
> +++ b/Documentation/git-status.txt
> @@ -207,6 +207,7 @@ show tracked paths:
>
>  * ' ' = unmodified
>  * 'M' = modified
> +* 'T' = file type changed (regular file, symbolic link or submodule)
>  * 'A' = added
>  * 'D' = deleted
>  * 'R' = renamed
> @@ -217,14 +218,16 @@ show tracked paths:
>  X          Y     Meaning
>  -------------------------------------------------
>          [AMD]   not updated
> -M        [ MD]   updated in index
> -A        [ MD]   added to index
> +M        [ MTD]  updated in index
> +T        [ MTD]  type changed in index
> +A        [ MTD]  added to index
>  D                deleted from index
> -R        [ MD]   renamed in index
> -C        [ MD]   copied in index
> -[MARC]           index and work tree matches
> -[ MARC]     M    work tree changed since index
> -[ MARC]     D    deleted in work tree
> +R        [ MTD]  renamed in index
> +C        [ MTD]  copied in index
> +[MTARC]          index and work tree matches
> +[ MTARC]    M    work tree changed since index
> +[ MTARC]    T    type changed in work tree since index
> +[ MTARC]    D    deleted in work tree

Looks good up to here.

>  [ 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?

Thanks for the contribution!
Johannes Altmanninger Oct. 4, 2021, 6:57 p.m. UTC | #3
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).

> 
>     $ 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.

> 
> > 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.

> 
> 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

> 
> >  [ 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
diff mbox series

Patch

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 83f38e3198..40f308c6a6 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -207,6 +207,7 @@  show tracked paths:
 
 * ' ' = unmodified
 * 'M' = modified
+* 'T' = file type changed (regular file, symbolic link or submodule)
 * 'A' = added
 * 'D' = deleted
 * 'R' = renamed
@@ -217,14 +218,16 @@  show tracked paths:
 X          Y     Meaning
 -------------------------------------------------
 	 [AMD]   not updated
-M        [ MD]   updated in index
-A        [ MD]   added to index
+M        [ MTD]  updated in index
+T        [ MTD]  type changed in index
+A        [ MTD]  added to index
 D                deleted from index
-R        [ MD]   renamed in index
-C        [ MD]   copied in index
-[MARC]           index and work tree matches
-[ MARC]     M    work tree changed since index
-[ MARC]     D    deleted in work tree
+R        [ MTD]  renamed in index
+C        [ MTD]  copied in index
+[MTARC]          index and work tree matches
+[ MTARC]    M    work tree changed since index
+[ MTARC]    T    type changed in work tree since index
+[ MTARC]    D    deleted in work tree
 [ D]        R    renamed in work tree
 [ D]        C    copied in work tree
 -------------------------------------------------