Re: [PATCH v2 5/7] reflog: improve and update documentation

[Junio C Hamano <gitster@pobox.com>]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> +Reference logs, or "reflogs", record when the tips of branches and
> +other references were updated in the local repository. Reflogs are
> +useful in various Git commands, to specify the old value of a
> +reference. For example, `HEAD@{2}` means "where HEAD used to be two
> +moves ago", `master@{one.week.ago}` means "where master used to point
> +to one week ago", and so on. See linkgit:gitrevisions[7] for more
> +details.

Looks very good, especially the part that mentions "in the local
repository".  It seems to be a common novice misunderstanding what
`master@{one.week.ago}` means, and it might be beneficial to be more
verbose by saying "where master used to point to one week ago in
this local repository".

> +The "expire" subcommand prunes older reflog entries. Entries older
> +than `expire` time, or entries older than `expire-unreachable` time
> +and not reachable from the current tip, are removed from the reflog.
> +This is typically not used directly by end users -- instead, see
> +linkgit:git-gc[1].
> +
> +The "delete" subcommand deletes single entries from the reflog. Its
> +argument must be an _exact_ entry (e.g. "`git reflog delete
> +master@{2}`").

Just like "expire", "delete" should be accompanied by the same
"typically not".  I do not think it is even worth mentioning that it
exists merely as an implementation detail for likgit:git-stash[1]
and for no other reason.


> +Options for `expire` and/or `delete`
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I think this started from a hope that these two share many, but
looking at the result I notice the shared ones are a tiny and
trivial minority.  It probably makes sense to retitle this section
"Options for expire" (and remove "For the expire command only"), and
add an "Options for delete" section immediately after it that looks
like:

	Options for `delete`
        ~~~~~~~~~~~~~~~~~~~~

	--updateref::
        --rewrite::
        --dry-run::
        	The `delete` command takes these options whose
                meanings are the same as those for `expire`.

> diff --git a/builtin/reflog.c b/builtin/reflog.c
> index 49c64f9..dd68a72 100644
> --- a/builtin/reflog.c
> +++ b/builtin/reflog.c
> @@ -13,9 +13,9 @@
>   */
>  
>  static const char reflog_expire_usage[] =
> -"git reflog expire [--verbose] [--dry-run] [--stale-fix] [--expire=<time>] [--expire-unreachable=<time>] [--all] <refs>...";
> +"git reflog expire [--expire=<time>] [--expire-unreachable=<time>] [--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] [--verbose] [--all] <refs>...";
>  static const char reflog_delete_usage[] =
> -"git reflog delete [--verbose] [--dry-run] [--rewrite] [--updateref] <refs>...";
> +"git reflog delete [--rewrite] [--updateref] [--dry-run | -n] [--verbose] <refs>...";
>  
>  static unsigned long default_reflog_expire;
>  static unsigned long default_reflog_expire_unreachable;

Thanks for being complete, but I sense that it may be time we
switched to parse-options here, which gives us the help string for
free.  Perhaps throw in a comment line before this hunk

	/* NEEDSWORK: switch to parse-options */

or something to leave hint for other people?

Thanks.