[2/4] ls-files: clarify descriptions of file selection options

Commit Message

Elijah Newren Jan. 13, 2023, 4:41 a.m. UTC

From: Elijah Newren <[e-mail]>

The previous descriptions of the file selection options were very easy
to misunderstand.  For example:

  * "Show cached files in the output"
    This could be interpreted as meaning "show files which have been
    modified and git-add'ed, i.e. files which have cached changes
    relative to HEAD".

  * "Show deleted files"
    This could be interpreted as meaning "for each `git rm $FILE` we
    ran, show me $FILE"

  * "Show modified files"
    This could be interpreted as meaning "show files which have been
    modified and git-add'ed" or as "show me files that differ from HEAD"
    or as "show me undeleted files different from HEAD" (given that
    --deleted is a separate option), none of which are correct.

Further, it's not very clear when some options only modify and/or
override other options, as was the case with --ignored, --directory, and
--unmerged (I've seen folks confused by each of them on the mailing
list, sometimes even fellow git developers.)

Tweak these definitions, and the one for --killed, to try to make them
all a bit more clear.  Finally, also clarify early on that duplicate
reports for paths are often expected (both when (a) there are multiple
entries for the file in the index -- i.e. when there are conflicts, and
also (b) when the user specifies options that might pick the same file
multiple times, such as `git ls-files --cached --deleted --modified`
when there is a file with an unstaged deletion).

Signed-off-by: Elijah Newren <[e-mail]>
---
 Documentation/git-ls-files.txt | 37 ++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 13 deletions(-)

Comments

ZheNing Hu Jan. 14, 2023, 8:21 a.m. UTC | #1

Elijah Newren via GitGitGadget <[e-mail]> 于2023年1月13日周五 12:41写道：
>
> From: Elijah Newren <[e-mail]>
>
> The previous descriptions of the file selection options were very easy
> to misunderstand.  For example:
>
>   * "Show cached files in the output"
>     This could be interpreted as meaning "show files which have been
>     modified and git-add'ed, i.e. files which have cached changes
>     relative to HEAD".
>
>   * "Show deleted files"
>     This could be interpreted as meaning "for each `git rm $FILE` we
>     ran, show me $FILE"
>
>   * "Show modified files"
>     This could be interpreted as meaning "show files which have been
>     modified and git-add'ed" or as "show me files that differ from HEAD"
>     or as "show me undeleted files different from HEAD" (given that
>     --deleted is a separate option), none of which are correct.
>
> Further, it's not very clear when some options only modify and/or
> override other options, as was the case with --ignored, --directory, and
> --unmerged (I've seen folks confused by each of them on the mailing
> list, sometimes even fellow git developers.)
>
> Tweak these definitions, and the one for --killed, to try to make them
> all a bit more clear.  Finally, also clarify early on that duplicate
> reports for paths are often expected (both when (a) there are multiple
> entries for the file in the index -- i.e. when there are conflicts, and
> also (b) when the user specifies options that might pick the same file
> multiple times, such as `git ls-files --cached --deleted --modified`
> when there is a file with an unstaged deletion).
>
> Signed-off-by: Elijah Newren <[e-mail]>
> ---
>  Documentation/git-ls-files.txt | 37 ++++++++++++++++++++++------------
>  1 file changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> index cb071583f8b..f89ab1bfc98 100644
> --- a/Documentation/git-ls-files.txt
> +++ b/Documentation/git-ls-files.txt
> @@ -29,21 +29,26 @@ This merges the file listing in the index with the actual working
>  directory list, and shows different combinations of the two.
>
>  One or more of the options below may be used to determine the files
> -shown:
> +shown, and each file may be printed multiple times if there are
> +multiple entries in the index or multiple statuses are applicable for
> +the relevant file selection options.
>

`--deduplicate` option can be used to remove deduped output.

>  OPTIONS
>  -------
>  -c::
>  --cached::
> -       Show cached files in the output (default)
> +       Show all files cached in Git's index, i.e. all tracked files.
> +       (This is the default if no -c/-s/-d/-o/-u/-k/-m/--resolve-undo
> +       options are specified.)
>
>  -d::
>  --deleted::
> -       Show deleted files in the output
> +       Show files with an unstaged deletion
>

This is a nice fix: make it clear to the user that only files in the
working tree are deleted, not in the index.

>  -m::
>  --modified::
> -       Show modified files in the output
> +       Show files with an unstaged modification (note that an unstaged
> +       deletion also counts as an unstaged modification)
>

Good to mention that deleted files are also modified, otherwise no one
looking at the documentation would know that.

>  -o::
>  --others::
> @@ -51,11 +56,14 @@ OPTIONS
>
>  -i::
>  --ignored::
> -       Show only ignored files in the output. When showing files in the
> -       index, print only those matched by an exclude pattern. When
> -       showing "other" files, show only those matched by an exclude
> -       pattern. Standard ignore rules are not automatically activated,
> -       therefore at least one of the `--exclude*` options is required.
> +       Show only ignored files in the output.  Must be used with
> +       either an explicit '-c' or '-o'.  When showing files in the
> +       index (i.e. when used with '-c'), print only those files
> +       matching an exclude pattern.  When showing "other" files
> +       (i.e. when used with '-o'), show only those matched by an
> +       exclude pattern.  Standard ignore rules are not automatically
> +       activated, therefore at least one of the `--exclude*` options
> +       is required.
>
>  -s::
>  --stage::
> @@ -64,19 +72,22 @@ OPTIONS
>  --directory::
>         If a whole directory is classified as "other", show just its
>         name (with a trailing slash) and not its whole contents.
> +       Has no effect without -o/--others.
>
>  --no-empty-directory::
>         Do not list empty directories. Has no effect without --directory.
>
>  -u::
>  --unmerged::
> -       Show unmerged files in the output (forces --stage)
> +       Show information about unmerged files in the output, but do
> +       not show any other tracked files (forces --stage, overrides
> +       --cached).
>
>  -k::
>  --killed::
> -       Show files on the filesystem that need to be removed due
> -       to file/directory conflicts for checkout-index to
> -       succeed.
> +       Show untracked files on the filesystem that need to be removed
> +       due to file/directory conflicts for tracked files to be able to
> +       be written to the filesystem.
>
>  --resolve-undo::
>         Show files having resolve-undo information in the index
> --
> gitgitgadget
>

Elijah Newren Jan. 14, 2023, 7:42 p.m. UTC | #2

On Sat, Jan 14, 2023 at 12:21 AM ZheNing Hu <[e-mail]> wrote:
>
> Elijah Newren via GitGitGadget <[e-mail]> 于2023年1月13日周五 12:41写道：
[...]
> > diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> > index cb071583f8b..f89ab1bfc98 100644
> > --- a/Documentation/git-ls-files.txt
> > +++ b/Documentation/git-ls-files.txt
> > @@ -29,21 +29,26 @@ This merges the file listing in the index with the actual working
> >  directory list, and shows different combinations of the two.
> >
> >  One or more of the options below may be used to determine the files
> > -shown:
> > +shown, and each file may be printed multiple times if there are
> > +multiple entries in the index or multiple statuses are applicable for
> > +the relevant file selection options.
> >
>
> `--deduplicate` option can be used to remove deduped output.

Yes, I'm aware.

If you're suggesting adding this text at this point in the document,
it occurred to me already, but I chose not to put it here.  The reason
is that this is a brief synopsis.  The "relevant file selection
options" of this brief synopsis could also be expanded to mention what
they are or what the default selection is or whatever.  But folks can
read on to learn that `deduplicate` can be used to remove duplicate
options.  Likewise, anyone who reads the text about "relevant file
selections" and wants to learn more is inclined to read on to the
other options to find out.

In contrast, no one will be motivated to read on to find out that
files can be printed multiple times if we don't mention it right here.
And they are likely to get confused when it happens, thinking it is a
bug (in fact, I can point out emails from the archives where that has
happened).  Without mentioning the possibility of multiple files at
this point, we have a discoverability problem.

There is no similar discoverability and negative-surprise problem I
can think of by omitting other details, so there is no need to expand
this brief synopsis any further.

The one place we could potentially change thing that might help, is
moving the text about -c being the default from under the -c option
and putting it here.  That's a toss-up to me, but for now I elected to
keep it where it is.

> >  OPTIONS
> >  -------
> >  -c::
> >  --cached::
> > -       Show cached files in the output (default)
> > +       Show all files cached in Git's index, i.e. all tracked files.
> > +       (This is the default if no -c/-s/-d/-o/-u/-k/-m/--resolve-undo
> > +       options are specified.)
> >
> >  -d::
> >  --deleted::
> > -       Show deleted files in the output
> > +       Show files with an unstaged deletion
> >
>
> This is a nice fix: make it clear to the user that only files in the
> working tree are deleted, not in the index.
>
> >  -m::
> >  --modified::
> > -       Show modified files in the output
> > +       Show files with an unstaged modification (note that an unstaged
> > +       deletion also counts as an unstaged modification)
> >
>
> Good to mention that deleted files are also modified, otherwise no one
> looking at the documentation would know that.
>
> >  -o::
> >  --others::
[...]

Thanks for taking a look!

ZheNing Hu Jan. 16, 2023, 5:12 p.m. UTC | #3

Elijah Newren <[e-mail]> 于2023年1月15日周日 03:42写道：
>
> On Sat, Jan 14, 2023 at 12:21 AM ZheNing Hu <[e-mail]> wrote:
> >
> > Elijah Newren via GitGitGadget <[e-mail]> 于2023年1月13日周五 12:41写道：
> [...]
> > > diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> > > index cb071583f8b..f89ab1bfc98 100644
> > > --- a/Documentation/git-ls-files.txt
> > > +++ b/Documentation/git-ls-files.txt
> > > @@ -29,21 +29,26 @@ This merges the file listing in the index with the actual working
> > >  directory list, and shows different combinations of the two.
> > >
> > >  One or more of the options below may be used to determine the files
> > > -shown:
> > > +shown, and each file may be printed multiple times if there are
> > > +multiple entries in the index or multiple statuses are applicable for
> > > +the relevant file selection options.
> > >
> >
> > `--deduplicate` option can be used to remove deduped output.
>
> Yes, I'm aware.
>
> If you're suggesting adding this text at this point in the document,
> it occurred to me already, but I chose not to put it here.  The reason
> is that this is a brief synopsis.  The "relevant file selection
> options" of this brief synopsis could also be expanded to mention what
> they are or what the default selection is or whatever.  But folks can
> read on to learn that `deduplicate` can be used to remove duplicate
> options.  Likewise, anyone who reads the text about "relevant file
> selections" and wants to learn more is inclined to read on to the
> other options to find out.
>
> In contrast, no one will be motivated to read on to find out that
> files can be printed multiple times if we don't mention it right here.
> And they are likely to get confused when it happens, thinking it is a
> bug (in fact, I can point out emails from the archives where that has
> happened).  Without mentioning the possibility of multiple files at
> this point, we have a discoverability problem.
>
> There is no similar discoverability and negative-surprise problem I
> can think of by omitting other details, so there is no need to expand
> this brief synopsis any further.
>

Well, you are right. It may be better to be concise here, telling users too
much will make it difficult to read.

> The one place we could potentially change thing that might help, is
> moving the text about -c being the default from under the -c option
> and putting it here.  That's a toss-up to me, but for now I elected to
> keep it where it is.
>

I think it's fine to do this or not.

> > >  OPTIONS
> > >  -------
> > >  -c::
> > >  --cached::
> > > -       Show cached files in the output (default)
> > > +       Show all files cached in Git's index, i.e. all tracked files.
> > > +       (This is the default if no -c/-s/-d/-o/-u/-k/-m/--resolve-undo
> > > +       options are specified.)
> > >
> > >  -d::
> > >  --deleted::
> > > -       Show deleted files in the output
> > > +       Show files with an unstaged deletion
> > >
> >
> > This is a nice fix: make it clear to the user that only files in the
> > working tree are deleted, not in the index.
> >
> > >  -m::
> > >  --modified::
> > > -       Show modified files in the output
> > > +       Show files with an unstaged modification (note that an unstaged
> > > +       deletion also counts as an unstaged modification)
> > >
> >
> > Good to mention that deleted files are also modified, otherwise no one
> > looking at the documentation would know that.
> >
> > >  -o::
> > >  --others::
> [...]
>
> Thanks for taking a look!

Patch

diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index cb071583f8b..f89ab1bfc98 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -29,21 +29,26 @@  This merges the file listing in the index with the actual working
 directory list, and shows different combinations of the two.
 
 One or more of the options below may be used to determine the files
-shown:
+shown, and each file may be printed multiple times if there are
+multiple entries in the index or multiple statuses are applicable for
+the relevant file selection options.
 
 OPTIONS
 -------
 -c::
 --cached::
-	Show cached files in the output (default)
+	Show all files cached in Git's index, i.e. all tracked files.
+	(This is the default if no -c/-s/-d/-o/-u/-k/-m/--resolve-undo
+	options are specified.)
 
 -d::
 --deleted::
-	Show deleted files in the output
+	Show files with an unstaged deletion
 
 -m::
 --modified::
-	Show modified files in the output
+	Show files with an unstaged modification (note that an unstaged
+	deletion also counts as an unstaged modification)
 
 -o::
 --others::
@@ -51,11 +56,14 @@  OPTIONS
 
 -i::
 --ignored::
-	Show only ignored files in the output. When showing files in the
-	index, print only those matched by an exclude pattern. When
-	showing "other" files, show only those matched by an exclude
-	pattern. Standard ignore rules are not automatically activated,
-	therefore at least one of the `--exclude*` options is required.
+	Show only ignored files in the output.  Must be used with
+	either an explicit '-c' or '-o'.  When showing files in the
+	index (i.e. when used with '-c'), print only those files
+	matching an exclude pattern.  When showing "other" files
+	(i.e. when used with '-o'), show only those matched by an
+	exclude pattern.  Standard ignore rules are not automatically
+	activated, therefore at least one of the `--exclude*` options
+	is required.
 
 -s::
 --stage::
@@ -64,19 +72,22 @@  OPTIONS
 --directory::
 	If a whole directory is classified as "other", show just its
 	name (with a trailing slash) and not its whole contents.
+	Has no effect without -o/--others.
 
 --no-empty-directory::
 	Do not list empty directories. Has no effect without --directory.
 
 -u::
 --unmerged::
-	Show unmerged files in the output (forces --stage)
+	Show information about unmerged files in the output, but do
+	not show any other tracked files (forces --stage, overrides
+	--cached).
 
 -k::
 --killed::
-	Show files on the filesystem that need to be removed due
-	to file/directory conflicts for checkout-index to
-	succeed.
+	Show untracked files on the filesystem that need to be removed
+	due to file/directory conflicts for tracked files to be able to
+	be written to the filesystem.
 
 --resolve-undo::
 	Show files having resolve-undo information in the index