[4/5] doc: commit: unify <pathspec> description
diff mbox series

Message ID 251d06e27f6bc93b190450ae6e1087a3126b5e52.1572895605.git.gitgitgadget@gmail.com
State New
Headers show
Series
  • Add --pathspec-from-file option for reset, commit
Related show

Commit Message

ryenus via GitGitGadget Nov. 4, 2019, 7:26 p.m. UTC
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

Synchronize it to `git add`, which has a pretty good description.
This also better disambiguates <file>... header.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 Documentation/git-commit.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Junio C Hamano Nov. 6, 2019, 4:50 a.m. UTC | #1
"Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
>
> Synchronize it to `git add`, which has a pretty good description.
> This also better disambiguates <file>... header.

"When files are given on..." is no longer true with this change (it
wasn't true with the code before this change anyway).

	When pathspec is given on the command line, commit the
	contents of the files that match the pathspec without
	recording the changes already added to the index. ...

The second sentence also says "these files", but that can be left
as-is, since it would refer to "the files that match ..." explained
in the above sentence.

> +For more details about the <pathspec> syntax, see the 'pathspec' entry
> +in linkgit:gitglossary[7].

I am not sure if we want to repeat this all over the place.

We do not say "For details about the <commit> syntax, see the
'SPECIFYING REVISIONS' section of linkgit:git-rev-parse[1]" for
every command that takes <commit> from the command line.

Is your urge to suggest adding this sentence coming from that you
are much more familiar with <commit> than <pathspec>?  In other
words, if you were more familiar with Git, would you still be adding
this (and not corresponding one for <commit>)?
Alexandr Miloslavskiy Nov. 6, 2019, 3:56 p.m. UTC | #2
I think I have implemented most suggestions in PatchV2. Thanks!

> I am not sure if we want to repeat this all over the place.
> 
> We do not say "For details about the <commit> syntax, see the
> 'SPECIFYING REVISIONS' section of linkgit:git-rev-parse[1]" for
> every command that takes <commit> from the command line.
> 
> Is your urge to suggest adding this sentence coming from that you
> are much more familiar with <commit> than <pathspec>?  In other
> words, if you were more familiar with Git, would you still be adding
> this (and not corresponding one for <commit>)?

Commit is a well known term, dating from ancient times like CVS or even 
older.

Pathspec, however, is something new. When I pretend to be someone new to 
git, I see it this way:
1) Let's read "git commit" documentation
2) Where on this commandline do I put my filename?!

So yes, I would repeat it in every location that could be popular for 
new users.
Junio C Hamano Nov. 7, 2019, 5:54 a.m. UTC | #3
Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com> writes:

> I think I have implemented most suggestions in PatchV2. Thanks!
>
>> I am not sure if we want to repeat this all over the place.
>>
>> We do not say "For details about the <commit> syntax, see the
>> 'SPECIFYING REVISIONS' section of linkgit:git-rev-parse[1]" for
>> every command that takes <commit> from the command line.
>>
>> Is your urge to suggest adding this sentence coming from that you
>> are much more familiar with <commit> than <pathspec>?  In other
>> words, if you were more familiar with Git, would you still be adding
>> this (and not corresponding one for <commit>)?
>
> Commit is a well known term, dating from ancient times like CVS or
> even older.

That's more or less irrelevant.  

I am reacting to this from your change that you omitted quoting in
your reply:

> +For more details about the <pathspec> syntax, see the 'pathspec' entry
> +in linkgit:gitglossary[7].

That sentence is for those who have some notion of <pathspec> but
does not know enough about its syntax.

CVS does not let you specify <commit> like "master^{/^fix frotz}~4";
A user a user who is familiar with CVS's commits would still want to
refer to the section "For details about the <commit> syntax".

I am not advocating to add the reference to SPECIFYING REVISIONS
section; instead we should let readers know that every time they see
<defined word>, they can refer to the glossary for more details.

> Pathspec, however, is something new.

Compared to CVS, everything in Git may be new, but that was a news
in 2010, not this year.
Alexandr Miloslavskiy Nov. 7, 2019, 11:39 a.m. UTC | #4
On 07.11.2019 6:54, Junio C Hamano wrote:
> I am reacting to this from your change that you omitted quoting in
> your reply:
> 
>> +For more details about the <pathspec> syntax, see the 'pathspec' entry
>> +in linkgit:gitglossary[7].
> 
> That sentence is for those who have some notion of <pathspec> but
> does not know enough about its syntax.

In the perfect world, I would expect _every_ 'pathspec' word in text to 
be an HTML-style link to a dedicated article, not just a paragraph in 
glossary.

MSDN is in general a good example [1]: there, it's easy to read only a 
small portion of article, ignoring anything you're not interested in, 
and still have all links at hand.

Regarding dedicated page: the content of 'pathspec' in glossary is 
already long enough for a page, and it could benefit from additional 
examples. Also, having a dedicated page makes linking easier, so that 
user doesn't have to scroll glossary.

Regarding links: I don't really understand what git's doc format allows. 
Is it just pure text in worst (or even average) case?

If it's usually something with clickable links, it could be worth to 
just insert links everywhere.

If it's usually plaintext, then "see the 'pathspec' entry in 
linkgit:gitglossary[7]" is a bit too verbose to repeat on every 
occasion. Still, I'd like to see links everywhere. One big reason is to 
let reader know that the explanation actually exists!

A compromise solution is to give every article header like this:

     This article uses the following terms which are explained
     in linkgit:gitglossary[7]:
     * pathspec
     * tree-ish
     * refspec

If it's close to top of article, then chances are that everyone will 
notice it. Also, it will not require extra verbosity in plaintext.

> CVS does not let you specify <commit> like "master^{/^fix frotz}~4";
> A user a user who is familiar with CVS's commits would still want to
> refer to the section "For details about the <commit> syntax".
> 
> I am not advocating to add the reference to SPECIFYING REVISIONS
> section; instead we should let readers know that every time they see
> <defined word>, they can refer to the glossary for more details.

I now think that my arguments apply to <pathspec> AND <commit> in the 
same way. Indeed a user can't know complex <commit> variants until 
he/she reads it in git docs.

----

[1] 
https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea

Patch
diff mbox series

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index afa7b75a23..4341d0e3ab 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -13,7 +13,7 @@  SYNOPSIS
 	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
-	   [-i | -o] [-S[<keyid>]] [--] [<file>...]
+	   [-i | -o] [-S[<keyid>]] [--] [<pathspec>...]
 
 DESCRIPTION
 -----------
@@ -345,12 +345,15 @@  changes to tracked files.
 \--::
 	Do not interpret any more arguments as options.
 
-<file>...::
+<pathspec>...::
 	When files are given on the command line, the command
 	commits the contents of the named files, without
 	recording the changes already staged.  The contents of
 	these files are also staged for the next commit on top
 	of what have been staged before.
++
+For more details about the <pathspec> syntax, see the 'pathspec' entry
+in linkgit:gitglossary[7].
 
 :git-commit: 1
 include::date-formats.txt[]