| Message ID | patch-07.34-a070c97f29b-20220902T092734Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | doc/UX: make txt & -h output more consistent | expand |
Ævar Arnfjörð Bjarmason wrote: > Fix the incorrect "[-o | --option <argument>]" syntax, which should be > "[(-o | --option) <argument>]", we were previously claiming that only > the long option accepted the "<argument>", which isn't what we meant. > > Unlike a preceding commit where this was fixed for commands which had nit: if you're referring to patch 10 [1], it wouldn't be a "preceding" commit. Otherwise, I'm not sure which commit you're referring to? In any case, I'd personally prefer that this patch is combined with patch 10 anyway, since both fix the same underlying issue (just in different places). Neither patch is particularly large, and it would be nice to make (and review!) all of the "fix alternates syntax" changes at once. [1] https://lore.kernel.org/git/patch-10.34-965f6b3b3cb-20220902T092734Z-avarab@gmail.com/ > inconsistent "-h" and *.txt, for "stash" it's been with us in both the > "-h" and *.txt since bd514cada4b (stash: introduce 'git stash store', > 2013-06-15). > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
On Mon, Sep 19 2022, Victoria Dye wrote: > Ævar Arnfjörð Bjarmason wrote: >> Fix the incorrect "[-o | --option <argument>]" syntax, which should be >> "[(-o | --option) <argument>]", we were previously claiming that only >> the long option accepted the "<argument>", which isn't what we meant. >> >> Unlike a preceding commit where this was fixed for commands which had > > nit: if you're referring to patch 10 [1], it wouldn't be a "preceding" > commit. Otherwise, I'm not sure which commit you're referring to? Thanks, yes this is incorrect. It's due to rebasing/reordering this & missing this mention. > In any case, I'd personally prefer that this patch is combined with patch 10 > anyway, since both fix the same underlying issue (just in different places). > Neither patch is particularly large, and it would be nice to make (and > review!) all of the "fix alternates syntax" changes at once. Yes, it's the same issue, but I think I'll keep them split up (working on a re-roll). The reason is (as mentioned in the CL) to cleanly split up the "consistency" changes from the rest. I.e. in this one we need to change both *.c and *.txt versions, so it's a "new" change, i.e. reviewers need to do more work to eyeball & review a version that's never used that form in-tree. Whereas the "consistency" commits are ones where we *only* change one of *.c or *.txt, to make it consistent with the other existing and corresponding form. That still requires review to see which version should "win", but with 34/34 (where we test all of those at the end) doesn't require the reviewer to consider whether the change is sensible at all (if it doesn't make sense we already have that version in 1/2 places in-tree already).
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt index 6e15f475257..0df21321e50 100644 --- a/Documentation/git-stash.txt +++ b/Documentation/git-stash.txt @@ -14,12 +14,12 @@ SYNOPSIS 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>] 'git stash' branch <branchname> [<stash>] 'git stash' [push [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet] - [-u|--include-untracked] [-a|--all] [-m|--message <message>] + [-u|--include-untracked] [-a|--all] [(-m|--message) <message>] [--pathspec-from-file=<file> [--pathspec-file-nul]] [--] [<pathspec>...]] 'git stash' clear 'git stash' create [<message>] -'git stash' store [-m|--message <message>] [-q|--quiet] <commit> +'git stash' store [(-m|--message) <message>] [-q|--quiet] <commit> DESCRIPTION ----------- @@ -47,7 +47,7 @@ stash index (e.g. the integer `n` is equivalent to `stash@{n}`). COMMANDS -------- -push [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--pathspec-from-file=<file> [--pathspec-file-nul]] [--] [<pathspec>...]:: +push [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [(-m|--message) <message>] [--pathspec-from-file=<file> [--pathspec-file-nul]] [--] [<pathspec>...]:: Save your local modifications to a new 'stash entry' and roll them back to HEAD (in the working tree and in the index). diff --git a/builtin/stash.c b/builtin/stash.c index 1ba24c11737..0fc359e9d39 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -29,7 +29,7 @@ static const char * const git_stash_usage[] = { N_("git stash branch <branchname> [<stash>]"), "git stash clear", N_("git stash [push [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet]\n" - " [-u|--include-untracked] [-a|--all] [-m|--message <message>]\n" + " [-u|--include-untracked] [-a|--all] [(-m|--message) <message>]\n" " [--pathspec-from-file=<file> [--pathspec-file-nul]]\n" " [--] [<pathspec>...]]"), N_("git stash save [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet]\n" @@ -73,13 +73,13 @@ static const char * const git_stash_clear_usage[] = { }; static const char * const git_stash_store_usage[] = { - N_("git stash store [-m|--message <message>] [-q|--quiet] <commit>"), + N_("git stash store [(-m|--message) <message>] [-q|--quiet] <commit>"), NULL }; static const char * const git_stash_push_usage[] = { N_("git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n" - " [-u|--include-untracked] [-a|--all] [-m|--message <message>]\n" + " [-u|--include-untracked] [-a|--all] [(-m|--message) <message>]\n" " [--] [<pathspec>...]]"), NULL };
Fix the incorrect "[-o | --option <argument>]" syntax, which should be "[(-o | --option) <argument>]", we were previously claiming that only the long option accepted the "<argument>", which isn't what we meant. Unlike a preceding commit where this was fixed for commands which had inconsistent "-h" and *.txt, for "stash" it's been with us in both the "-h" and *.txt since bd514cada4b (stash: introduce 'git stash store', 2013-06-15). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Documentation/git-stash.txt | 6 +++--- builtin/stash.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)