git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/4] clarify ls-files docs
@ 2023-01-13  4:41 Elijah Newren via GitGitGadget
  2023-01-13  4:41 ` [PATCH 1/4] ls-files: add missing documentation for --resolve-undo option Elijah Newren via GitGitGadget
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Elijah Newren via GitGitGadget @ 2023-01-13  4:41 UTC (permalink / raw)
  To: git; +Cc: ZheNing Hu, Elijah Newren

"git ls-files -t" was marked as semi-deprecated back in 5bc0e247c4
("Document ls-files -t as semi-obsolete.", 2010-07-28); quoting that commit
message:

    git ls-files -t" is [...] badly documented, hence we point the
    users to superior alternatives.

    The feature is marked as "semi-obsolete" but not "scheduled for removal"
    since it's a plumbing command, scripts might use it, and Git testsuite
    already uses it to test the state of the index.


Looking over the manual, "git ls-files -t" is very easy to
misunderstand...as are several things besides the "-t" option within that
manual page. I recall a number of discussions over the years on the mailing
list about various confusing aspects of ls-files, and I think a number of
those misunderstandings could have been avoided with a few small
clarifications. This series sets about to do that, as well as remove the
"semi-deprecated" notice on git ls-files -t. That particular command is
rather integral to sparse-checkout usage (and doesn't seem to confuse in
that case), and I think the improvements avoid the original problems.
However, I have kept the comments about git status --porcelain and such
being likely better candidates for a number of things that git ls-files -t
might have historically been used for.

Series spurred by
https://lore.kernel.org/git/CABPp-BGsD=6PiJtnsuYPsiZJ1rm2X8yTeu-YeP4q5uu5UDw2og@mail.gmail.com/

Elijah Newren (4):
  ls-files: add missing documentation for --resolve-undo option
  ls-files: clarify descriptions of file selection options
  ls-files: clarify descriptions of status tags for -t
  ls-files: guide folks to --exclude-standard over other --exclude*
    options

 Documentation/git-ls-files.txt | 79 ++++++++++++++++++++++------------
 1 file changed, 52 insertions(+), 27 deletions(-)


base-commit: 2b4f5a4e4bb102ac8d967cea653ed753b608193c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1463%2Fnewren%2Fls-files-docs-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1463/newren/ls-files-docs-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1463
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] ls-files: add missing documentation for --resolve-undo option
  2023-01-13  4:41 [PATCH 0/4] clarify ls-files docs Elijah Newren via GitGitGadget
@ 2023-01-13  4:41 ` Elijah Newren via GitGitGadget
  2023-01-14  8:07   ` ZheNing Hu
  2023-01-13  4:41 ` [PATCH 2/4] ls-files: clarify descriptions of file selection options Elijah Newren via GitGitGadget
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Elijah Newren via GitGitGadget @ 2023-01-13  4:41 UTC (permalink / raw)
  To: git; +Cc: ZheNing Hu, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

ls-files' --resolve-undo option has existed ever since 9d9a2f4aba
("resolve-undo: basic tests", 2009-12-25), but was never documented.
However, the option has been referred to in the ls-files manual itself
ever since ce74de931d ("ls-files: introduce "--format" option",
2022-07-23), making its omission a bit jarring.  Document this option.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-ls-files.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index 440043cdb8e..cb071583f8b 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -12,6 +12,7 @@ SYNOPSIS
 'git ls-files' [-z] [-t] [-v] [-f]
 		[-c|--cached] [-d|--deleted] [-o|--others] [-i|--ignored]
 		[-s|--stage] [-u|--unmerged] [-k|--killed] [-m|--modified]
+		[--resolve-undo]
 		[--directory [--no-empty-directory]] [--eol]
 		[--deduplicate]
 		[-x <pattern>|--exclude=<pattern>]
@@ -77,6 +78,13 @@ OPTIONS
 	to file/directory conflicts for checkout-index to
 	succeed.
 
+--resolve-undo::
+	Show files having resolve-undo information in the index
+	together with their resolve-undo information.  (resolve-undo
+	information is what is used to implement "git checkout -m
+	$PATH", i.e. to recreate merge conflicts that were
+	accidentally resolved)
+
 -z::
 	\0 line termination on output and do not quote filenames.
 	See OUTPUT below for more information.
@@ -136,6 +144,7 @@ a space) at the start of each line:
 	C::	modified/changed
 	K::	to be killed
 	?::	other
+	U::     resolve-undo
 --
 
 -v::
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/4] ls-files: clarify descriptions of file selection options
  2023-01-13  4:41 [PATCH 0/4] clarify ls-files docs Elijah Newren via GitGitGadget
  2023-01-13  4:41 ` [PATCH 1/4] ls-files: add missing documentation for --resolve-undo option Elijah Newren via GitGitGadget
@ 2023-01-13  4:41 ` Elijah Newren via GitGitGadget
  2023-01-14  8:21   ` ZheNing Hu
  2023-01-13  4:41 ` [PATCH 3/4] ls-files: clarify descriptions of status tags for -t Elijah Newren via GitGitGadget
  2023-01-13  4:41 ` [PATCH 4/4] ls-files: guide folks to --exclude-standard over other --exclude* options Elijah Newren via GitGitGadget
  3 siblings, 1 reply; 12+ messages in thread
From: Elijah Newren via GitGitGadget @ 2023-01-13  4:41 UTC (permalink / raw)
  To: git; +Cc: ZheNing Hu, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

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 <newren@gmail.com>
---
 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.
 
 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
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/4] ls-files: clarify descriptions of status tags for -t
  2023-01-13  4:41 [PATCH 0/4] clarify ls-files docs Elijah Newren via GitGitGadget
  2023-01-13  4:41 ` [PATCH 1/4] ls-files: add missing documentation for --resolve-undo option Elijah Newren via GitGitGadget
  2023-01-13  4:41 ` [PATCH 2/4] ls-files: clarify descriptions of file selection options Elijah Newren via GitGitGadget
@ 2023-01-13  4:41 ` Elijah Newren via GitGitGadget
  2023-01-14  8:26   ` ZheNing Hu
  2023-01-13  4:41 ` [PATCH 4/4] ls-files: guide folks to --exclude-standard over other --exclude* options Elijah Newren via GitGitGadget
  3 siblings, 1 reply; 12+ messages in thread
From: Elijah Newren via GitGitGadget @ 2023-01-13  4:41 UTC (permalink / raw)
  To: git; +Cc: ZheNing Hu, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Much like the file selection options we tweaked in the last commit, the
status tags printed with -t had descriptions that were easy to
misunderstand, and for many of the same reasons.  Clarify them.

Also, while at it, remove the "semi-deprecated" comment for "git
ls-files -t".  The -t option was marked as semi-deprecated in 5bc0e247c4
("Document ls-files -t as semi-obsolete.", 2010-07-28) because:

    "git ls-files -t" is [...] badly documented, hence we point the
    users to superior alternatives.
    The feature is marked as "semi-obsolete" but not "scheduled for removal"
    since it's a plumbing command, scripts might use it, and Git testsuite
    already uses it to test the state of the index.

Marking it as obsolete because it was easily misunderstood, which I
think was primarily due to documentation problems, is one strategy, but
I think fixing the documentation is a better option.  Especially since
in the intervening time, "git ls-files -t" has become heavily used by
sparse-checkout users where the same confusion just doesn't apply.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-ls-files.txt | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index f89ab1bfc98..3886d58d178 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -137,25 +137,27 @@ OPTIONS
 	with `-s` or `-u` options does not make any sense.
 
 -t::
-	This feature is semi-deprecated. For scripting purpose,
-	linkgit:git-status[1] `--porcelain` and
+	Show status tags together with filenames.  Note that for
+	scripting purposes, linkgit:git-status[1] `--porcelain` and
 	linkgit:git-diff-files[1] `--name-status` are almost always
 	superior alternatives, and users should look at
 	linkgit:git-status[1] `--short` or linkgit:git-diff[1]
 	`--name-status` for more user-friendly alternatives.
 +
 --
-This option identifies the file status with the following tags (followed by
-a space) at the start of each line:
-
-	H::	cached
-	S::	skip-worktree
-	M::	unmerged
-	R::	removed/deleted
-	C::	modified/changed
-	K::	to be killed
-	?::	other
-	U::     resolve-undo
+This option provides a reason for showing each filename, in the form
+of a status tag (which is followed by a space and then the filename).
+The status tags are all single characters from the following list:
+
+	H::	tracked file that is not either unmerged or skip-worktree
+	S::	tracked file that is skip-worktree
+	M::	tracked file that is unmerged
+	R::	tracked file with unstaged removal/deletion
+	C::	tracked file with unstaged modification/change
+	K::	untracked paths which are part of file/directory conflicts
+		which prevent checking out tracked files
+	?::	untracked file
+	U::     file with resolve-undo information
 --
 
 -v::
-- 
gitgitgadget


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/4] ls-files: guide folks to --exclude-standard over other --exclude* options
  2023-01-13  4:41 [PATCH 0/4] clarify ls-files docs Elijah Newren via GitGitGadget
                   ` (2 preceding siblings ...)
  2023-01-13  4:41 ` [PATCH 3/4] ls-files: clarify descriptions of status tags for -t Elijah Newren via GitGitGadget
@ 2023-01-13  4:41 ` Elijah Newren via GitGitGadget
  3 siblings, 0 replies; 12+ messages in thread
From: Elijah Newren via GitGitGadget @ 2023-01-13  4:41 UTC (permalink / raw)
  To: git; +Cc: ZheNing Hu, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 Documentation/git-ls-files.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index 3886d58d178..1abdd3c21c5 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -119,7 +119,8 @@ OPTIONS
 
 --exclude-per-directory=<file>::
 	Read additional exclude patterns that apply only to the
-	directory and its subdirectories in <file>.
+	directory and its subdirectories in <file>.  Deprecated; use
+	--exclude-standard instead.
 
 --exclude-standard::
 	Add the standard Git exclusions: .git/info/exclude, .gitignore
@@ -291,7 +292,9 @@ traversing the directory tree and finding files to show when the
 flags --others or --ignored are specified.  linkgit:gitignore[5]
 specifies the format of exclude patterns.
 
-These exclude patterns come from these places, in order:
+Generally, you should just use --exclude-standard, but for historical
+reasons the exclude patterns can be specified from the following
+places, in order:
 
   1. The command-line flag --exclude=<pattern> specifies a
      single pattern.  Patterns are ordered in the same order
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] ls-files: add missing documentation for --resolve-undo option
  2023-01-13  4:41 ` [PATCH 1/4] ls-files: add missing documentation for --resolve-undo option Elijah Newren via GitGitGadget
@ 2023-01-14  8:07   ` ZheNing Hu
  0 siblings, 0 replies; 12+ messages in thread
From: ZheNing Hu @ 2023-01-14  8:07 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren

Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> 于2023年1月13日周五 12:41写道:

>
> From: Elijah Newren <newren@gmail.com>
>
> ls-files' --resolve-undo option has existed ever since 9d9a2f4aba
> ("resolve-undo: basic tests", 2009-12-25), but was never documented.
> However, the option has been referred to in the ls-files manual itself
> ever since ce74de931d ("ls-files: introduce "--format" option",
> 2022-07-23), making its omission a bit jarring.  Document this option.
>

I checked this should be the only option that git ls-files forgot to
document, thanks.

> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  Documentation/git-ls-files.txt | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> index 440043cdb8e..cb071583f8b 100644
> --- a/Documentation/git-ls-files.txt
> +++ b/Documentation/git-ls-files.txt
> @@ -12,6 +12,7 @@ SYNOPSIS
>  'git ls-files' [-z] [-t] [-v] [-f]
>                 [-c|--cached] [-d|--deleted] [-o|--others] [-i|--ignored]
>                 [-s|--stage] [-u|--unmerged] [-k|--killed] [-m|--modified]
> +               [--resolve-undo]
>                 [--directory [--no-empty-directory]] [--eol]
>                 [--deduplicate]
>                 [-x <pattern>|--exclude=<pattern>]
> @@ -77,6 +78,13 @@ OPTIONS
>         to file/directory conflicts for checkout-index to
>         succeed.
>
> +--resolve-undo::
> +       Show files having resolve-undo information in the index
> +       together with their resolve-undo information.  (resolve-undo
> +       information is what is used to implement "git checkout -m
> +       $PATH", i.e. to recreate merge conflicts that were
> +       accidentally resolved)
> +
>  -z::
>         \0 line termination on output and do not quote filenames.
>         See OUTPUT below for more information.
> @@ -136,6 +144,7 @@ a space) at the start of each line:
>         C::     modified/changed
>         K::     to be killed
>         ?::     other
> +       U::     resolve-undo
>  --
>
>  -v::
> --
> gitgitgadget
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] ls-files: clarify descriptions of file selection options
  2023-01-13  4:41 ` [PATCH 2/4] ls-files: clarify descriptions of file selection options Elijah Newren via GitGitGadget
@ 2023-01-14  8:21   ` ZheNing Hu
  2023-01-14 19:42     ` Elijah Newren
  0 siblings, 1 reply; 12+ messages in thread
From: ZheNing Hu @ 2023-01-14  8:21 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren

Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> 于2023年1月13日周五 12:41写道:
>
> From: Elijah Newren <newren@gmail.com>
>
> 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 <newren@gmail.com>
> ---
>  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
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] ls-files: clarify descriptions of status tags for -t
  2023-01-13  4:41 ` [PATCH 3/4] ls-files: clarify descriptions of status tags for -t Elijah Newren via GitGitGadget
@ 2023-01-14  8:26   ` ZheNing Hu
  2023-01-14 20:26     ` Elijah Newren
  0 siblings, 1 reply; 12+ messages in thread
From: ZheNing Hu @ 2023-01-14  8:26 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren

Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> 于2023年1月13日周五 12:41写道:
>
> From: Elijah Newren <newren@gmail.com>
>
> Much like the file selection options we tweaked in the last commit, the
> status tags printed with -t had descriptions that were easy to
> misunderstand, and for many of the same reasons.  Clarify them.
>
> Also, while at it, remove the "semi-deprecated" comment for "git
> ls-files -t".  The -t option was marked as semi-deprecated in 5bc0e247c4
> ("Document ls-files -t as semi-obsolete.", 2010-07-28) because:
>
>     "git ls-files -t" is [...] badly documented, hence we point the
>     users to superior alternatives.
>     The feature is marked as "semi-obsolete" but not "scheduled for removal"
>     since it's a plumbing command, scripts might use it, and Git testsuite
>     already uses it to test the state of the index.
>
> Marking it as obsolete because it was easily misunderstood, which I
> think was primarily due to documentation problems, is one strategy, but
> I think fixing the documentation is a better option.  Especially since
> in the intervening time, "git ls-files -t" has become heavily used by
> sparse-checkout users where the same confusion just doesn't apply.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  Documentation/git-ls-files.txt | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> index f89ab1bfc98..3886d58d178 100644
> --- a/Documentation/git-ls-files.txt
> +++ b/Documentation/git-ls-files.txt
> @@ -137,25 +137,27 @@ OPTIONS
>         with `-s` or `-u` options does not make any sense.
>
>  -t::
> -       This feature is semi-deprecated. For scripting purpose,
> -       linkgit:git-status[1] `--porcelain` and
> +       Show status tags together with filenames.  Note that for
> +       scripting purposes, linkgit:git-status[1] `--porcelain` and
>         linkgit:git-diff-files[1] `--name-status` are almost always
>         superior alternatives, and users should look at
>         linkgit:git-status[1] `--short` or linkgit:git-diff[1]
>         `--name-status` for more user-friendly alternatives.
>  +
>  --
> -This option identifies the file status with the following tags (followed by
> -a space) at the start of each line:
> -
> -       H::     cached
> -       S::     skip-worktree
> -       M::     unmerged
> -       R::     removed/deleted
> -       C::     modified/changed
> -       K::     to be killed
> -       ?::     other
> -       U::     resolve-undo
> +This option provides a reason for showing each filename, in the form
> +of a status tag (which is followed by a space and then the filename).
> +The status tags are all single characters from the following list:
> +
> +       H::     tracked file that is not either unmerged or skip-worktree
> +       S::     tracked file that is skip-worktree
> +       M::     tracked file that is unmerged
> +       R::     tracked file with unstaged removal/deletion
> +       C::     tracked file with unstaged modification/change
> +       K::     untracked paths which are part of file/directory conflicts
> +               which prevent checking out tracked files
> +       ?::     untracked file
> +       U::     file with resolve-undo information
>  --
>

Good to see these tags describe are changed, especially "K" (reader
don't know what is "to be killed")

Maybe we should mention which option will output these tags?
e.g. default -> "H"/"S" ,`--other` -> "?", `--modified` -> "C",
`--killed` -> "K"...

>  -v::
> --
> gitgitgadget
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] ls-files: clarify descriptions of file selection options
  2023-01-14  8:21   ` ZheNing Hu
@ 2023-01-14 19:42     ` Elijah Newren
  2023-01-16 17:12       ` ZheNing Hu
  0 siblings, 1 reply; 12+ messages in thread
From: Elijah Newren @ 2023-01-14 19:42 UTC (permalink / raw)
  To: ZheNing Hu; +Cc: Elijah Newren via GitGitGadget, git

On Sat, Jan 14, 2023 at 12:21 AM ZheNing Hu <adlternative@gmail.com> wrote:
>
> Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> 于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!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] ls-files: clarify descriptions of status tags for -t
  2023-01-14  8:26   ` ZheNing Hu
@ 2023-01-14 20:26     ` Elijah Newren
  2023-01-16 17:21       ` ZheNing Hu
  0 siblings, 1 reply; 12+ messages in thread
From: Elijah Newren @ 2023-01-14 20:26 UTC (permalink / raw)
  To: ZheNing Hu; +Cc: Elijah Newren via GitGitGadget, git

On Sat, Jan 14, 2023 at 12:27 AM ZheNing Hu <adlternative@gmail.com> wrote:
>
> Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> 于2023年1月13日周五 12:41写道:
> >
> > From: Elijah Newren <newren@gmail.com>
> >
> > Much like the file selection options we tweaked in the last commit, the
> > status tags printed with -t had descriptions that were easy to
> > misunderstand, and for many of the same reasons.  Clarify them.
> >
> > Also, while at it, remove the "semi-deprecated" comment for "git
> > ls-files -t".  The -t option was marked as semi-deprecated in 5bc0e247c4
> > ("Document ls-files -t as semi-obsolete.", 2010-07-28) because:
> >
> >     "git ls-files -t" is [...] badly documented, hence we point the
> >     users to superior alternatives.
> >     The feature is marked as "semi-obsolete" but not "scheduled for removal"
> >     since it's a plumbing command, scripts might use it, and Git testsuite
> >     already uses it to test the state of the index.
> >
> > Marking it as obsolete because it was easily misunderstood, which I
> > think was primarily due to documentation problems, is one strategy, but
> > I think fixing the documentation is a better option.  Especially since
> > in the intervening time, "git ls-files -t" has become heavily used by
> > sparse-checkout users where the same confusion just doesn't apply.
> >
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >  Documentation/git-ls-files.txt | 28 +++++++++++++++-------------
> >  1 file changed, 15 insertions(+), 13 deletions(-)
> >
> > diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> > index f89ab1bfc98..3886d58d178 100644
> > --- a/Documentation/git-ls-files.txt
> > +++ b/Documentation/git-ls-files.txt
> > @@ -137,25 +137,27 @@ OPTIONS
> >         with `-s` or `-u` options does not make any sense.
> >
> >  -t::
> > -       This feature is semi-deprecated. For scripting purpose,
> > -       linkgit:git-status[1] `--porcelain` and
> > +       Show status tags together with filenames.  Note that for
> > +       scripting purposes, linkgit:git-status[1] `--porcelain` and
> >         linkgit:git-diff-files[1] `--name-status` are almost always
> >         superior alternatives, and users should look at
> >         linkgit:git-status[1] `--short` or linkgit:git-diff[1]
> >         `--name-status` for more user-friendly alternatives.
> >  +
> >  --
> > -This option identifies the file status with the following tags (followed by
> > -a space) at the start of each line:
> > -
> > -       H::     cached
> > -       S::     skip-worktree
> > -       M::     unmerged
> > -       R::     removed/deleted
> > -       C::     modified/changed
> > -       K::     to be killed
> > -       ?::     other
> > -       U::     resolve-undo
> > +This option provides a reason for showing each filename, in the form
> > +of a status tag (which is followed by a space and then the filename).
> > +The status tags are all single characters from the following list:
> > +
> > +       H::     tracked file that is not either unmerged or skip-worktree
> > +       S::     tracked file that is skip-worktree
> > +       M::     tracked file that is unmerged
> > +       R::     tracked file with unstaged removal/deletion
> > +       C::     tracked file with unstaged modification/change
> > +       K::     untracked paths which are part of file/directory conflicts
> > +               which prevent checking out tracked files
> > +       ?::     untracked file
> > +       U::     file with resolve-undo information
> >  --
> >
>
> Good to see these tags describe are changed, especially "K" (reader
> don't know what is "to be killed")
>
> Maybe we should mention which option will output these tags?
> e.g. default -> "H"/"S" ,`--other` -> "?", `--modified` -> "C",
> `--killed` -> "K"...

We could, but...

  * It's H/S/M, not just H/S that is shown by default.
  * It gets weird because other options aren't added to the default,
so if someone specifies "-m" then suddenly H/S/M go away...unless they
also specify "-c".

Trying to explain all that feels like we're getting close to repeating
the documentation of the individual options.  But maybe we could just
ignore everything around default behavior and find a way to be brief
such as with:

    Note that H, S, and M entries are shown with --cached; R entries
    are shown with --deleted, C entries are shown with --modified, K
    entries are shown with --killed, ? entries are shown with
    --others, and U entries are shown with --resolve-undo.

I'm not sure if I like the documentation better with or without this
added paragraph.  What do others think?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] ls-files: clarify descriptions of file selection options
  2023-01-14 19:42     ` Elijah Newren
@ 2023-01-16 17:12       ` ZheNing Hu
  0 siblings, 0 replies; 12+ messages in thread
From: ZheNing Hu @ 2023-01-16 17:12 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Elijah Newren via GitGitGadget, git

Elijah Newren <newren@gmail.com> 于2023年1月15日周日 03:42写道:
>
> On Sat, Jan 14, 2023 at 12:21 AM ZheNing Hu <adlternative@gmail.com> wrote:
> >
> > Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> 于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!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] ls-files: clarify descriptions of status tags for -t
  2023-01-14 20:26     ` Elijah Newren
@ 2023-01-16 17:21       ` ZheNing Hu
  0 siblings, 0 replies; 12+ messages in thread
From: ZheNing Hu @ 2023-01-16 17:21 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Elijah Newren via GitGitGadget, git

Elijah Newren <newren@gmail.com> 于2023年1月15日周日 04:27写道:
>
> On Sat, Jan 14, 2023 at 12:27 AM ZheNing Hu <adlternative@gmail.com> wrote:
> >
> > Elijah Newren via GitGitGadget <gitgitgadget@gmail.com> 于2023年1月13日周五 12:41写道:
> > >
> > > From: Elijah Newren <newren@gmail.com>
> > >
> > > Much like the file selection options we tweaked in the last commit, the
> > > status tags printed with -t had descriptions that were easy to
> > > misunderstand, and for many of the same reasons.  Clarify them.
> > >
> > > Also, while at it, remove the "semi-deprecated" comment for "git
> > > ls-files -t".  The -t option was marked as semi-deprecated in 5bc0e247c4
> > > ("Document ls-files -t as semi-obsolete.", 2010-07-28) because:
> > >
> > >     "git ls-files -t" is [...] badly documented, hence we point the
> > >     users to superior alternatives.
> > >     The feature is marked as "semi-obsolete" but not "scheduled for removal"
> > >     since it's a plumbing command, scripts might use it, and Git testsuite
> > >     already uses it to test the state of the index.
> > >
> > > Marking it as obsolete because it was easily misunderstood, which I
> > > think was primarily due to documentation problems, is one strategy, but
> > > I think fixing the documentation is a better option.  Especially since
> > > in the intervening time, "git ls-files -t" has become heavily used by
> > > sparse-checkout users where the same confusion just doesn't apply.
> > >
> > > Signed-off-by: Elijah Newren <newren@gmail.com>
> > > ---
> > >  Documentation/git-ls-files.txt | 28 +++++++++++++++-------------
> > >  1 file changed, 15 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> > > index f89ab1bfc98..3886d58d178 100644
> > > --- a/Documentation/git-ls-files.txt
> > > +++ b/Documentation/git-ls-files.txt
> > > @@ -137,25 +137,27 @@ OPTIONS
> > >         with `-s` or `-u` options does not make any sense.
> > >
> > >  -t::
> > > -       This feature is semi-deprecated. For scripting purpose,
> > > -       linkgit:git-status[1] `--porcelain` and
> > > +       Show status tags together with filenames.  Note that for
> > > +       scripting purposes, linkgit:git-status[1] `--porcelain` and
> > >         linkgit:git-diff-files[1] `--name-status` are almost always
> > >         superior alternatives, and users should look at
> > >         linkgit:git-status[1] `--short` or linkgit:git-diff[1]
> > >         `--name-status` for more user-friendly alternatives.
> > >  +
> > >  --
> > > -This option identifies the file status with the following tags (followed by
> > > -a space) at the start of each line:
> > > -
> > > -       H::     cached
> > > -       S::     skip-worktree
> > > -       M::     unmerged
> > > -       R::     removed/deleted
> > > -       C::     modified/changed
> > > -       K::     to be killed
> > > -       ?::     other
> > > -       U::     resolve-undo
> > > +This option provides a reason for showing each filename, in the form
> > > +of a status tag (which is followed by a space and then the filename).
> > > +The status tags are all single characters from the following list:
> > > +
> > > +       H::     tracked file that is not either unmerged or skip-worktree
> > > +       S::     tracked file that is skip-worktree
> > > +       M::     tracked file that is unmerged
> > > +       R::     tracked file with unstaged removal/deletion
> > > +       C::     tracked file with unstaged modification/change
> > > +       K::     untracked paths which are part of file/directory conflicts
> > > +               which prevent checking out tracked files
> > > +       ?::     untracked file
> > > +       U::     file with resolve-undo information
> > >  --
> > >
> >
> > Good to see these tags describe are changed, especially "K" (reader
> > don't know what is "to be killed")
> >
> > Maybe we should mention which option will output these tags?
> > e.g. default -> "H"/"S" ,`--other` -> "?", `--modified` -> "C",
> > `--killed` -> "K"...
>
> We could, but...
>
>   * It's H/S/M, not just H/S that is shown by default.
>   * It gets weird because other options aren't added to the default,
> so if someone specifies "-m" then suddenly H/S/M go away...unless they
> also specify "-c".
>
> Trying to explain all that feels like we're getting close to repeating
> the documentation of the individual options.  But maybe we could just
> ignore everything around default behavior and find a way to be brief
> such as with:
>
>     Note that H, S, and M entries are shown with --cached; R entries
>     are shown with --deleted, C entries are shown with --modified, K
>     entries are shown with --killed, ? entries are shown with
>     --others, and U entries are shown with --resolve-undo.
>

What you mean is that each tag will appear in which commands, rather than
each command will have which tags. I think this segment is pretty good.

> I'm not sure if I like the documentation better with or without this
> added paragraph.  What do others think?

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-01-16 17:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-13  4:41 [PATCH 0/4] clarify ls-files docs Elijah Newren via GitGitGadget
2023-01-13  4:41 ` [PATCH 1/4] ls-files: add missing documentation for --resolve-undo option Elijah Newren via GitGitGadget
2023-01-14  8:07   ` ZheNing Hu
2023-01-13  4:41 ` [PATCH 2/4] ls-files: clarify descriptions of file selection options Elijah Newren via GitGitGadget
2023-01-14  8:21   ` ZheNing Hu
2023-01-14 19:42     ` Elijah Newren
2023-01-16 17:12       ` ZheNing Hu
2023-01-13  4:41 ` [PATCH 3/4] ls-files: clarify descriptions of status tags for -t Elijah Newren via GitGitGadget
2023-01-14  8:26   ` ZheNing Hu
2023-01-14 20:26     ` Elijah Newren
2023-01-16 17:21       ` ZheNing Hu
2023-01-13  4:41 ` [PATCH 4/4] ls-files: guide folks to --exclude-standard over other --exclude* options Elijah Newren via GitGitGadget

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).