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

2015-03-03 Thread Michael Haggerty
On 03/02/2015 11:04 PM, Junio C Hamano wrote:
 Michael Haggerty mhag...@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.

Yes, that's good. I will change it.

 +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.

OK, will change.

 +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`.

Either way is a little bit awkward, because these two subcommands share
4 out of 8 of their options. But since delete is really quite useless
to end users, I stuck its options in a separate section as you
suggested, but inline in a paragraph, where they won't bother anybody.

 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?

OK.

Thanks for your review! A reroll will be coming soon.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-02 Thread Junio C Hamano
Michael Haggerty mhag...@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.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-02 Thread Michael Haggerty
Revamp the git reflog usage documentation in the manpage and the
command help to match the current reality and improve its clarity:

* Add documentation for some options that had been left out.

* Group the subcommands and options more logically and move more
  common subcommands/options higher.

* Improve some explanations.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 Documentation/git-reflog.txt | 138 +--
 builtin/reflog.c |   4 +-
 2 files changed, 81 insertions(+), 61 deletions(-)

diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt
index 70791b9..3af9422 100644
--- a/Documentation/git-reflog.txt
+++ b/Documentation/git-reflog.txt
@@ -17,81 +17,101 @@ The command takes various subcommands, and different 
options
 depending on the subcommand:
 
 [verse]
-'git reflog expire' [--dry-run] [--stale-fix] [--verbose]
-   [--expire=time] [--expire-unreachable=time] [--all] refs...
-'git reflog delete' ref@\{specifier\}...
 'git reflog' ['show'] [log-options] [ref]
+'git reflog expire' [--expire=time] [--expire-unreachable=time]
+   [--rewrite] [--updateref] [--stale-fix]
+   [--dry-run] [--verbose] [--all | refs...]
+'git reflog delete' [--rewrite] [--updateref]
+   [--dry-run] [--verbose] ref@\{specifier\}...
+
+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.
+
+This command manages the information recorded in the reflogs.
+
+The show subcommand (which is also the default, in the absence of
+any subcommands) shows the log of the reference provided in the
+command-line (or `HEAD`, by default). The reflog covers all recent
+actions, and in addition the `HEAD` reflog records branch switching.
+`git reflog show` is an alias for `git log -g --abbrev-commit
+--pretty=oneline`; see linkgit:git-log[1] for more information.
+
+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}`).
 
-Reflog is a mechanism to record when the tip of branches are
-updated.  This command is to manage the information recorded in it.
 
-The subcommand expire is used to prune 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 the end users -- instead, see linkgit:git-gc[1].
-
-The subcommand show (which is also the default, in the absence of any
-subcommands) will take all the normal log options, and show the log of
-the reference provided in the command-line (or `HEAD`, by default).
-The reflog will cover all recent actions (HEAD reflog records branch switching
-as well).  It is an alias for `git log -g --abbrev-commit --pretty=oneline`;
-see linkgit:git-log[1].
+OPTIONS
+---
 
-The reflog is 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.
+Options for `show`
+~~
 
-To delete single entries from the reflog, use the subcommand delete
-and specify the _exact_ entry (e.g. `git reflog delete master@{2}`).
+`git reflog show` accepts any of the options accepted by `git log`.
 
 
-OPTIONS

+Options for `expire` and/or `delete`
+
 
---stale-fix::
-   This revamps the logic -- the definition of broken commit
-   becomes: a commit that is not reachable from any of the refs and
-   there is a missing object among the commit, tree, or blob
-   objects reachable from it that is not reachable from any of the
-   refs.
-+
-This computation involves traversing all the reachable objects, i.e. it
-has the same cost as 'git prune'.  Fortunately, once this is run, we
-should not have to ever worry about missing objects, because the current
-prune and pack-objects know about reflogs and protect objects referred by
-them.
+--all::
+   (For the `expire` command only.) Process the reflogs of all
+   references.
 
 --expire=time::
-   Entries older than this time are pruned.  Without the
-   option it is taken from configuration `gc.reflogExpire`,
-