On 25/07/22 01:20PM, Patrick Steinhardt wrote:
> The way subcommands of git-reflog(1) are layed out does not make any
s/layed/laid/
> immediate sense. Reorder them such that read-only subcommands precede
> writing commands for a bit more structure.
>
> Furthermore, move the "expire" subcommand last. This prepares for a
> subsequent change where we are about to introduce a new "write" command
> to append reflog entries. Like this, the writing subcommands are ordered
> such that those affecting a single reflog come before those spanning
> across all reflogs.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> Documentation/git-reflog.adoc | 8 ++++----
> builtin/reflog.c | 38 +++++++++++++++++++-------------------
> 2 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/git-reflog.adoc b/Documentation/git-reflog.adoc
> index 707a9b39edb..6ae13e772b8 100644
> --- a/Documentation/git-reflog.adoc
> +++ b/Documentation/git-reflog.adoc
> @@ -11,13 +11,13 @@ SYNOPSIS
> [synopsis]
> git reflog [show] [<log-options>] [<ref>]
> git reflog list
> -git reflog expire [--expire=<time>] [--expire-unreachable=<time>]
> - [--rewrite] [--updateref] [--stale-fix]
> - [--dry-run | -n] [--verbose] [--all [--single-worktree] | <refs>...]
> +git reflog exists <ref>
> git reflog delete [--rewrite] [--updateref]
> [--dry-run | -n] [--verbose] <ref>@{<specifier>}...
> git reflog drop [--all [--single-worktree] | <refs>...]
> -git reflog exists <ref>
> +git reflog expire [--expire=<time>] [--expire-unreachable=<time>]
> + [--rewrite] [--updateref] [--stale-fix]
> + [--dry-run | -n] [--verbose] [--all [--single-worktree] | <refs>...]
>
> DESCRIPTION
> -----------
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> index 3acaf3e32c2..b00b3f9edc9 100644
> --- a/builtin/reflog.c
> +++ b/builtin/reflog.c
> @@ -17,21 +17,21 @@
> #define BUILTIN_REFLOG_LIST_USAGE \
> N_("git reflog list")
>
> -#define BUILTIN_REFLOG_EXPIRE_USAGE \
> - N_("git reflog expire [--expire=<time>] [--expire-unreachable=<time>]\n" \
> - " [--rewrite] [--updateref] [--stale-fix]\n" \
> - " [--dry-run | -n] [--verbose] [--all [--single-worktree] | <refs>...]")
> +#define BUILTIN_REFLOG_EXISTS_USAGE \
> + N_("git reflog exists <ref>")
>
> #define BUILTIN_REFLOG_DELETE_USAGE \
> N_("git reflog delete [--rewrite] [--updateref]\n" \
> " [--dry-run | -n] [--verbose] <ref>@{<specifier>}...")
>
> -#define BUILTIN_REFLOG_EXISTS_USAGE \
> - N_("git reflog exists <ref>")
> -
> #define BUILTIN_REFLOG_DROP_USAGE \
> N_("git reflog drop [--all [--single-worktree] | <refs>...]")
>
> +#define BUILTIN_REFLOG_EXPIRE_USAGE \
> + N_("git reflog expire [--expire=<time>] [--expire-unreachable=<time>]\n" \
> + " [--rewrite] [--updateref] [--stale-fix]\n" \
> + " [--dry-run | -n] [--verbose] [--all [--single-worktree] | <refs>...]")
> +
> static const char *const reflog_show_usage[] = {
> BUILTIN_REFLOG_SHOW_USAGE,
> NULL,
> @@ -42,9 +42,9 @@ static const char *const reflog_list_usage[] = {
> NULL,
> };
>
> -static const char *const reflog_expire_usage[] = {
> - BUILTIN_REFLOG_EXPIRE_USAGE,
> - NULL
> +static const char *const reflog_exists_usage[] = {
> + BUILTIN_REFLOG_EXISTS_USAGE,
> + NULL,
> };
>
> static const char *const reflog_delete_usage[] = {
> @@ -52,23 +52,23 @@ static const char *const reflog_delete_usage[] = {
> NULL
> };
>
> -static const char *const reflog_exists_usage[] = {
> - BUILTIN_REFLOG_EXISTS_USAGE,
> - NULL,
> -};
> -
> static const char *const reflog_drop_usage[] = {
> BUILTIN_REFLOG_DROP_USAGE,
> NULL,
> };
>
> +static const char *const reflog_expire_usage[] = {
> + BUILTIN_REFLOG_EXPIRE_USAGE,
> + NULL
> +};
> +
> static const char *const reflog_usage[] = {
> BUILTIN_REFLOG_SHOW_USAGE,
> BUILTIN_REFLOG_LIST_USAGE,
> - BUILTIN_REFLOG_EXPIRE_USAGE,
> + BUILTIN_REFLOG_EXISTS_USAGE,
> BUILTIN_REFLOG_DELETE_USAGE,
> BUILTIN_REFLOG_DROP_USAGE,
> - BUILTIN_REFLOG_EXISTS_USAGE,
> + BUILTIN_REFLOG_EXPIRE_USAGE,
> NULL
> };
>
> @@ -404,10 +404,10 @@ int cmd_reflog(int argc,
> struct option options[] = {
> OPT_SUBCOMMAND("show", &fn, cmd_reflog_show),
> OPT_SUBCOMMAND("list", &fn, cmd_reflog_list),
> - OPT_SUBCOMMAND("expire", &fn, cmd_reflog_expire),
> - OPT_SUBCOMMAND("delete", &fn, cmd_reflog_delete),
> OPT_SUBCOMMAND("exists", &fn, cmd_reflog_exists),
> + OPT_SUBCOMMAND("delete", &fn, cmd_reflog_delete),
> OPT_SUBCOMMAND("drop", &fn, cmd_reflog_drop),
> + OPT_SUBCOMMAND("expire", &fn, cmd_reflog_expire),
> OPT_END()
> };
Structing the subcommands order in such a manner seems sensible, but I'm
not sure the pattern will be recognized by others that may add
subcommands in the future. Maybe we could leave a comment that mentions
the order?
-Justin
