diff mbox series

[1/1] git-clean.txt: specify core.excludesFile variable is used

Message ID f8bc322cc5727d9cf45037f4231f88956f07b7f3.1551868745.git.liu.denton@gmail.com (mailing list archive)
State New, archived
Headers show
Series update git-clean.txt | expand

Commit Message

Denton Liu March 6, 2019, 10:42 a.m. UTC
In the git-clean documentation, -x and -e documented .gitignore,
$GIT_DIR/info/excludes but neglected to mention the file pointed to by
core.excludesFile.

Explicitly mention that variable so that the list is exhaustive.

Reported-by: Robert P. J. Day <rpjday@crashcourse.ca>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-clean.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Martin Ågren March 6, 2019, 8:22 p.m. UTC | #1
On Wed, 6 Mar 2019 at 12:17, Denton Liu <liu.denton@gmail.com> wrote:
>
> In the git-clean documentation, -x and -e documented .gitignore,
> $GIT_DIR/info/excludes but neglected to mention the file pointed to by
> core.excludesFile.

Nit: I suppose it doesn't so much document it, as mention it. So

  In the git-clean documentation, we mention .gitignore and
  $GIT_DIR/info/excludes, but neglect to mention the file pointed to by
  core.excludesFile.

perhaps.

> Explicitly mention that variable so that the list is exhaustive.

>  -e <pattern>::
>  --exclude=<pattern>::
> -       In addition to those found in .gitignore (per directory) and
> -       $GIT_DIR/info/exclude, also consider these patterns to be in the
> +       In addition to those found in .gitignore (per directory),
> +       $GIT_DIR/info/exclude, and the `core.excludesFile` variable, also
> +       consider these patterns to be in the
>         set of the ignore rules in effect.

The commit message correctly phrases it as "the file pointed to by",
whereas this could give the impression that the config variable is
supposed to provide patterns, not a filename. But if the choice is
between creating a longer, more language-lawyer-correct phrasing and a
shorter one that everyone will understand, I'll choose the latter any
day.

But on the topic of preferring shorter, I sort of wonder if we really
need to provide all of those filenames here. The point we're trying to
make is that we consider another source. So something like this would be
just as technically correct, I think:

  Use the given exclude pattern in addition to those found in .gitignore
  and similar files (see linkgit:gitignore[5]).

This also places the interesting (IMHO) part of the sentence at the
front, rather than at the end.

From gitignore(5), I get the impression that patterns provided using
`--exclude` take precedence over those found in those files we're
listing. Whether or not that is the case here might perhaps be more
interesting than the exact list of files. Does that make sense?

>  -x::
>         Don't use the standard ignore rules read from .gitignore (per
> -       directory) and $GIT_DIR/info/exclude, but do still use the ignore
> +       directory), $GIT_DIR/info/exclude, and the `core.excludesFile`
> +       variable, but do still use the ignore
>         rules given with `-e` options.  This allows removing all untracked
>         files, including build products.  This can be used (possibly in
>         conjunction with 'git reset') to create a pristine

Nit: Not new in this patch, but I think you could add a few `backticks`
while you're here to render things like `.gitignore` and
`$GIT_DIR/info/exclude/` in monospace.

Martin
Junio C Hamano March 7, 2019, 12:38 a.m. UTC | #2
Martin Ågren <martin.agren@gmail.com> writes:

> But on the topic of preferring shorter, I sort of wonder if we really
> need to provide all of those filenames here. The point we're trying to
> make is that we consider another source. So something like this would be
> just as technically correct, I think:
>
>   Use the given exclude pattern in addition to those found in .gitignore
>   and similar files (see linkgit:gitignore[5]).

Yes.  Unless we devise some mechanism to prevent these text that
list the places exclusion list are read from spread across the
documentation set, saying "found in usual places (see $there)" and
making sure we have the authoritative single place up-to-date is a
good approach.
diff mbox series

Patch

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index 03056dad0d..c6023b742c 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -55,13 +55,15 @@  OPTIONS
 
 -e <pattern>::
 --exclude=<pattern>::
-	In addition to those found in .gitignore (per directory) and
-	$GIT_DIR/info/exclude, also consider these patterns to be in the
+	In addition to those found in .gitignore (per directory),
+	$GIT_DIR/info/exclude, and the `core.excludesFile` variable, also
+	consider these patterns to be in the
 	set of the ignore rules in effect.
 
 -x::
 	Don't use the standard ignore rules read from .gitignore (per
-	directory) and $GIT_DIR/info/exclude, but do still use the ignore
+	directory), $GIT_DIR/info/exclude, and the `core.excludesFile`
+	variable, but do still use the ignore
 	rules given with `-e` options.  This allows removing all untracked
 	files, including build products.  This can be used (possibly in
 	conjunction with 'git reset') to create a pristine