patchadd: Clarify documentation of -A and -u

The documentation of '-A' is very confusing for someone who doesn't already know what it does. In particular, it describes the option's meaning by reference to that of '-u', but it's no more similar to the latter than it is to the default behavior. Describe it directly (and in a way that uses the word 'all'), and then describe the contrast separately.

Signed-off-by: Greg Price <price@mit.edu>
---
 Documentation/git-add.txt | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

Greg Price <price@MIT.EDU> writes:

I do not know if mentioning what -A does in the description for -u (and vice versa) makes it easier to understand or more confusing (not rhetorical: I suspect some may find it easier and others not).

But "and the default behaviour will..." here _is_ confusing. After reading this patch three times, I still cannot tell what "default" you are trying to describe. Is it "-u" without arguments? Is it "add" without "-u" nor "-A"? Is it something else???

Exactly the same comment applies to the other hunk.

Having said that, I agree that the current description for "-u" is way suboptimal. It begins like this:

    -u::
    --update::
            Only match <pathspec> against already tracked files in
            the index rather than the working tree. That means that it
            will ...

Whenever you see an incomprehensible technobabble followed by "That means", "In other words", etc., you (not limited to Greg-you but figuratively "everybody") should see if it makes it easier to read to remove everything up to that "That means" and adding a bit more to the remainder. In this particular case, the technobabble is not even correct---"match"ing is only a small part of what "add -u" does to find what to "add".

I would suggest rewriting the first part of "-u" perhaps like this:

	-u::
        --update::
		Update modified and/or removed paths in the index
		that match <pathspec> with the current state of the
		working tree files.  No new path is added because
		this considers only the paths that are already in
		the index.

The text for "-A" may look like this:

	-A::
        --all::
		Update the index to record the current state of the
		working tree files that match <pathspec>.  Note that
		new paths will be added to the index, in addition to
		modified and/or removed paths.

I agree with your patch that "If no <pathspec> is given" should be repeated for both "-u" and "-A".

On Wed, Mar 06, 2013 at 09:10:57AM -0800, Junio C Hamano wrote:

I meant the behavior without -A or -u. This could be fixed directly by s/the default behavior will/with neither -A nor -u we/. But we can also keep revising -- there's definitely more room to make this clearer! I suggest new versions at the bottom of this message.

I do think that it's important that the reader be able to see clearly what the option does as a contrast with what the command does without the option. Normally in a man page this is how the option is described in the first place -- for example, the next entry in this very man page says "Record *only* the fact that the path will be added later", rather than "Record the fact that ..." These two descriptions suffer from not doing that, so that it's not clear which parts of the behavior they describe are just what 'add' does with no options and which parts are actually due to the option.

Seems like a reasonable heuristic.

These are good -- I especially like that they're shorter. I do think they're still likely to be confusing. The lead sentences are hard to tell apart from each other or one's mental model of what 'add' alone does, though the contrasts that follow them help. I also think the lead sentence for '--all' isn't really correct -- we update the index not only for the working tree files that match <pathspec>, but also where there is no working tree file, only an index entry. (So the sentence actually describes what 'add' with neither option does.)

Maybe it's worth taking a step back.  The overall taxonomy is
 * 'add' alone considers matching filenames in the working tree
 * 'add -u' considers matching filenames in the index
 * 'add -A' considers matching filenames in both the index and the
   working tree
and in each case we make the index match the working tree on those
files.  Or, put another way,
 * 'add' alone modifies and adds files
 * 'add -u' modifies and removes files
 * 'add -A' modifies, adds, and removes files

Here's a crack at making those distinctions clear. I've also tried to make the descriptions as parallel as possible, as what they're saying is very similar.

-u::
--update::
	Update the index just where it already has an entry matching
	<pathspec>.  This removes as well as modifies index entries to
	match the working tree, but adds no new files.

-A::
--all::
	Update the index not only where the working tree has a file
	matching <pathspec> but also where the index already has an
	entry.  This adds, modifies, and removes index entries to
	match the working tree.

These are the shortest in the discussion so far, and I think they're also the clearest.

Then follow both with the "If no <pathspec>" paragraph. I just noticed that the paragraph actually needs a small modification to fit '-A', too. New patch below.

Greg

From: Greg Price <price@mit.edu>
Date: Thu, 7 Mar 2013 02:08:21 -0800
Subject: [PATCH] add: Clarify documentation of -A and -u

The documentation of '-A' and '-u' is very confusing for someone who doesn't already know what they do. Describe them with fewer words and clearer parallelism to each other and to the behavior of plain 'add'.

Also mention the default <pathspec> for '-A' as well as '-u', because it applies to both.

Signed-off-by: Greg Price <price@mit.edu>
---
 Documentation/git-add.txt | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

Greg Price <price@MIT.EDU> writes:

When we have bulletted list that enumerates options and describes what each option does and how each option affects the behaviour, I'd prefer to see us *not* talking about what happens when you do *not* give that option, unless it makes it hard to understand that option without such an extra description. The overall description of what the command does without the options should go to the top level.

Good; this was the short phrasing I was looking for but couldn't come up with myself without repeating "the index" over and over.

(for future reference) Drop the three lines and have "-- >8 --" here.

[patch kept unsnipped for others']