| Message ID | 20240604220145.3260714-4-gitster@pobox.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 607c3d372ef89cf16874c288c60423587286d182 |
| Headers | show |
| Series | Branches are branches and not heads | expand |
On Tue, Jun 4, 2024 at 10:02 PM Junio C Hamano <gitster@pobox.com> wrote: > > We call the tips of branches "heads", but this command calls the > option to show only branches "--heads", which confuses the branches > themselves and the tips of branches. > > Straighten the terminology by introducing "--branches" option that > limits the output to branches, and deprecate "--heads" option used > that way. > > We do not plan to remove "--heads" or "-h" yet; we may want to do so > at Git 3.0, in which case, we may need to start advertising upcoming > removal with an extra warning when they are used. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > Documentation/git-show-ref.txt | 18 ++++++++++-------- > builtin/show-ref.c | 16 +++++++++------- > t/t1403-show-ref.sh | 24 ++++++++++++++++-------- > 3 files changed, 35 insertions(+), 23 deletions(-) > > diff --git a/Documentation/git-show-ref.txt b/Documentation/git-show-ref.txt > index ba75747005..616d919655 100644 > --- a/Documentation/git-show-ref.txt > +++ b/Documentation/git-show-ref.txt > @@ -9,8 +9,8 @@ SYNOPSIS > -------- > [verse] > 'git show-ref' [--head] [-d | --dereference] > - [-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags] > - [--heads] [--] [<pattern>...] > + [-s | --hash[=<n>]] [--abbrev[=<n>]] [--branches] [--tags] > + [--] [<pattern>...] > 'git show-ref' --verify [-q | --quiet] [-d | --dereference] > [-s | --hash[=<n>]] [--abbrev[=<n>]] > [--] [<ref>...] > @@ -45,12 +45,14 @@ OPTIONS > > Show the HEAD reference, even if it would normally be filtered out. > > ---heads:: > +--branches:: > --tags:: > > - Limit to "refs/heads" and "refs/tags", respectively. These options > + Limit to local branches and local tags, respectively. These options > are not mutually exclusive; when given both, references stored in > - "refs/heads" and "refs/tags" are displayed. > + "refs/heads" and "refs/tags" are displayed. Note that `--heads` > + is a deprecated synonym for `--branches` and may be removed > + in the future. > > -d:: > --dereference:: > @@ -139,7 +141,7 @@ When using `--hash` (and not `--dereference`), the output is in the format: > For example, > > ----------------------------------------------------------------------------- > -$ git show-ref --heads --hash > +$ git show-ref --branches --hash > 2e3ba0114a1f52b47df29743d6915d056be13278 > 185008ae97960c8d551adcd9e23565194651b5d1 > 03adf42c988195b50e1a1935ba5fcbc39b2b029b > @@ -183,8 +185,8 @@ to check whether a particular branch exists or not (notice how we don't > actually want to show any results, and we want to use the full refname for it > in order to not trigger the problem with ambiguous partial matches). > > -To show only tags, or only proper branch heads, use `--tags` and/or `--heads` > -respectively (using both means that it shows tags and heads, but not other > +To show only tags, or only proper branch heads, use `--tags` and/or `--branches` > +respectively (using both means that it shows tags and branches, but not other > random references under the refs/ subdirectory). > > To do automatic tag object dereferencing, use the `-d` or `--dereference` > diff --git a/builtin/show-ref.c b/builtin/show-ref.c > index 1c15421e60..f634bc3e44 100644 > --- a/builtin/show-ref.c > +++ b/builtin/show-ref.c > @@ -11,8 +11,8 @@ > > static const char * const show_ref_usage[] = { > N_("git show-ref [--head] [-d | --dereference]\n" > - " [-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags]\n" > - " [--heads] [--] [<pattern>...]"), > + " [-s | --hash[=<n>]] [--abbrev[=<n>]] [--branches] [--tags]\n" > + " [--] [<pattern>...]"), > N_("git show-ref --verify [-q | --quiet] [-d | --dereference]\n" > " [-s | --hash[=<n>]] [--abbrev[=<n>]]\n" > " [--] [<ref>...]"), > @@ -188,7 +188,7 @@ static int cmd_show_ref__verify(const struct show_one_options *show_one_opts, > > struct patterns_options { > int show_head; > - int heads_only; > + int branches_only; > int tags_only; > }; > > @@ -206,8 +206,8 @@ static int cmd_show_ref__patterns(const struct patterns_options *opts, > > if (opts->show_head) > head_ref(show_ref, &show_ref_data); > - if (opts->heads_only || opts->tags_only) { > - if (opts->heads_only) > + if (opts->branches_only || opts->tags_only) { > + if (opts->branches_only) > for_each_fullref_in("refs/heads/", show_ref, &show_ref_data); > if (opts->tags_only) > for_each_fullref_in("refs/tags/", show_ref, &show_ref_data); > @@ -286,8 +286,10 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix) > struct show_one_options show_one_opts = {0}; > int verify = 0, exists = 0; > const struct option show_ref_options[] = { > - OPT_BOOL(0, "tags", &patterns_opts.tags_only, N_("only show tags (can be combined with heads)")), > - OPT_BOOL(0, "heads", &patterns_opts.heads_only, N_("only show heads (can be combined with tags)")), > + OPT_BOOL(0, "tags", &patterns_opts.tags_only, N_("only show tags (can be combined with branches)")), > + OPT_BOOL(0, "branches", &patterns_opts.branches_only, N_("only show branches (can be combined with tags)")), > + OPT_HIDDEN_BOOL(0, "heads", &patterns_opts.branches_only, > + N_("deprecated synonym for --branches")), > OPT_BOOL(0, "exists", &exists, N_("check for reference existence without resolving")), > OPT_BOOL(0, "verify", &verify, N_("stricter reference checking, " > "requires exact ref path")), > diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh > index 33fb7a38ff..403f6b8f7d 100755 > --- a/t/t1403-show-ref.sh > +++ b/t/t1403-show-ref.sh > @@ -121,13 +121,13 @@ test_expect_success 'show-ref -d' ' > > ' > > -test_expect_success 'show-ref --heads, --tags, --head, pattern' ' > +test_expect_success 'show-ref --branches, --tags, --head, pattern' ' > for branch in B main side > do > echo $(git rev-parse refs/heads/$branch) refs/heads/$branch || return 1 > - done >expect.heads && > - git show-ref --heads >actual && > - test_cmp expect.heads actual && > + done >expect.branches && > + git show-ref --branches >actual && > + test_cmp expect.branches actual && > > for tag in A B C > do > @@ -136,15 +136,15 @@ test_expect_success 'show-ref --heads, --tags, --head, pattern' ' > git show-ref --tags >actual && > test_cmp expect.tags actual && > > - cat expect.heads expect.tags >expect && > - git show-ref --heads --tags >actual && > + cat expect.branches expect.tags >expect && > + git show-ref --branches --tags >actual && > test_cmp expect actual && > > { > echo $(git rev-parse HEAD) HEAD && > - cat expect.heads expect.tags > + cat expect.branches expect.tags > } >expect && > - git show-ref --heads --tags --head >actual && > + git show-ref --branches --tags --head >actual && > test_cmp expect actual && > > { > @@ -165,6 +165,14 @@ test_expect_success 'show-ref --heads, --tags, --head, pattern' ' > test_cmp expect actual > ' > > +test_expect_success 'show-ref --heads is deprecated and hidden' ' > + test_expect_code 129 git show-ref -h >short-help && > + test_grep ! -e --heads short-help && > + git show-ref --heads >actual 2>warning && > + test_grep ! deprecated warning && > + test_cmp expect.branches actual > +' > + > test_expect_success 'show-ref --verify HEAD' ' > echo $(git rev-parse HEAD) HEAD >expect && > git show-ref --verify HEAD >actual && > -- > 2.45.2-409-g7b0defb391 If we are renaming --heads to --branches, should --head also be renamed? Other than that question, this patch and series looks good to me.
Elijah Newren <newren@gmail.com> writes:
> If we are renaming --heads to --branches, should --head also be renamed?
I do not think so. It is specifically about HEAD (the thing that
lives above refs/ hierarchy, historically implemented as a file
whose name is "HEAD" that is directly inside $GIT_DIR).
Thanks.
On Fri, Jun 14, 2024 at 9:21 PM Junio C Hamano <gitster@pobox.com> wrote: > > Elijah Newren <newren@gmail.com> writes: > > > If we are renaming --heads to --branches, should --head also be renamed? > > I do not think so. It is specifically about HEAD (the thing that > lives above refs/ hierarchy, historically implemented as a file > whose name is "HEAD" that is directly inside $GIT_DIR). > > Thanks. I'm fine if we don't want to rename it, but I don't quite follow this particular rationale. The logic you use here seems to be about internal details ("it's the file named HEAD") and ignores what users might refer to it as ("current branch"), whereas --branches ignored the internal details ("the files under refs/heads/") and instead concentrates on what users might refer to them as ("branches") and used that as the rationale for renaming. That said, I've almost never seen users use --head (and haven't used it myself), whereas asking for heads/branches is much more common, and I'm very happy with the change from --heads to --branches. Also, even if we do agree --head should be renamed, I'd be fine with punting it to later in order to get this improvement in now...it just seemed like a small inconsistency that I thought was worth pointing out.
On Fri, Jun 14, 2024 at 9:34 PM Elijah Newren <newren@gmail.com> wrote: > > On Fri, Jun 14, 2024 at 9:21 PM Junio C Hamano <gitster@pobox.com> wrote: > > > > Elijah Newren <newren@gmail.com> writes: > > > > > If we are renaming --heads to --branches, should --head also be renamed? > > > > I do not think so. It is specifically about HEAD (the thing that > > lives above refs/ hierarchy, historically implemented as a file > > whose name is "HEAD" that is directly inside $GIT_DIR). > > > > Thanks. > > I'm fine if we don't want to rename it, but I don't quite follow this > particular rationale. The logic you use here seems to be about > internal details ("it's the file named HEAD") and ignores what users > might refer to it as ("current branch"), whereas --branches ignored > the internal details ("the files under refs/heads/") and instead > concentrates on what users might refer to them as ("branches") and > used that as the rationale for renaming. > > That said, I've almost never seen users use --head (and haven't used > it myself), whereas asking for heads/branches is much more common, and > I'm very happy with the change from --heads to --branches. Also, even > if we do agree --head should be renamed, I'd be fine with punting it > to later in order to get this improvement in now...it just seemed like > a small inconsistency that I thought was worth pointing out. ...or maybe my argument breaks down because `HEAD` is more prominent and tends to be used by users more (`git reset --hard HEAD`, `git checkout HEAD~1`), and thus there's an argument it already is somewhat aligned with user terminology?
Elijah Newren <newren@gmail.com> writes: > ...or maybe my argument breaks down because `HEAD` is more prominent > and tends to be used by users more (`git reset --hard HEAD`, `git > checkout HEAD~1`), and thus there's an argument it already is somewhat > aligned with user terminology? Yeah, you are correct to say that HEAD is a lot more prominent than "refs/heads/". "git branch --list" does not expose the "refs/heads/" part (but "git for-each-ref" does), but you'd see HEAD in many places (e.g. "git show -s <RETURN>" gives the --decorate output that says "HEAD -> master" etc.). Of course we _could_ plan to rename "HEAD" to something else, like "CURRENT" and deal with the fallout, and then rename "refs/heads/" to "refs/branches/", but for what cost to achieve what benefit? The tradeoff does not look all that good to me. So I'd say renaming --heads to --branches would probably be a good place to stop, at least for now.
diff --git a/Documentation/git-show-ref.txt b/Documentation/git-show-ref.txt index ba75747005..616d919655 100644 --- a/Documentation/git-show-ref.txt +++ b/Documentation/git-show-ref.txt @@ -9,8 +9,8 @@ SYNOPSIS -------- [verse] 'git show-ref' [--head] [-d | --dereference] - [-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags] - [--heads] [--] [<pattern>...] + [-s | --hash[=<n>]] [--abbrev[=<n>]] [--branches] [--tags] + [--] [<pattern>...] 'git show-ref' --verify [-q | --quiet] [-d | --dereference] [-s | --hash[=<n>]] [--abbrev[=<n>]] [--] [<ref>...] @@ -45,12 +45,14 @@ OPTIONS Show the HEAD reference, even if it would normally be filtered out. ---heads:: +--branches:: --tags:: - Limit to "refs/heads" and "refs/tags", respectively. These options + Limit to local branches and local tags, respectively. These options are not mutually exclusive; when given both, references stored in - "refs/heads" and "refs/tags" are displayed. + "refs/heads" and "refs/tags" are displayed. Note that `--heads` + is a deprecated synonym for `--branches` and may be removed + in the future. -d:: --dereference:: @@ -139,7 +141,7 @@ When using `--hash` (and not `--dereference`), the output is in the format: For example, ----------------------------------------------------------------------------- -$ git show-ref --heads --hash +$ git show-ref --branches --hash 2e3ba0114a1f52b47df29743d6915d056be13278 185008ae97960c8d551adcd9e23565194651b5d1 03adf42c988195b50e1a1935ba5fcbc39b2b029b @@ -183,8 +185,8 @@ to check whether a particular branch exists or not (notice how we don't actually want to show any results, and we want to use the full refname for it in order to not trigger the problem with ambiguous partial matches). -To show only tags, or only proper branch heads, use `--tags` and/or `--heads` -respectively (using both means that it shows tags and heads, but not other +To show only tags, or only proper branch heads, use `--tags` and/or `--branches` +respectively (using both means that it shows tags and branches, but not other random references under the refs/ subdirectory). To do automatic tag object dereferencing, use the `-d` or `--dereference` diff --git a/builtin/show-ref.c b/builtin/show-ref.c index 1c15421e60..f634bc3e44 100644 --- a/builtin/show-ref.c +++ b/builtin/show-ref.c @@ -11,8 +11,8 @@ static const char * const show_ref_usage[] = { N_("git show-ref [--head] [-d | --dereference]\n" - " [-s | --hash[=<n>]] [--abbrev[=<n>]] [--tags]\n" - " [--heads] [--] [<pattern>...]"), + " [-s | --hash[=<n>]] [--abbrev[=<n>]] [--branches] [--tags]\n" + " [--] [<pattern>...]"), N_("git show-ref --verify [-q | --quiet] [-d | --dereference]\n" " [-s | --hash[=<n>]] [--abbrev[=<n>]]\n" " [--] [<ref>...]"), @@ -188,7 +188,7 @@ static int cmd_show_ref__verify(const struct show_one_options *show_one_opts, struct patterns_options { int show_head; - int heads_only; + int branches_only; int tags_only; }; @@ -206,8 +206,8 @@ static int cmd_show_ref__patterns(const struct patterns_options *opts, if (opts->show_head) head_ref(show_ref, &show_ref_data); - if (opts->heads_only || opts->tags_only) { - if (opts->heads_only) + if (opts->branches_only || opts->tags_only) { + if (opts->branches_only) for_each_fullref_in("refs/heads/", show_ref, &show_ref_data); if (opts->tags_only) for_each_fullref_in("refs/tags/", show_ref, &show_ref_data); @@ -286,8 +286,10 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix) struct show_one_options show_one_opts = {0}; int verify = 0, exists = 0; const struct option show_ref_options[] = { - OPT_BOOL(0, "tags", &patterns_opts.tags_only, N_("only show tags (can be combined with heads)")), - OPT_BOOL(0, "heads", &patterns_opts.heads_only, N_("only show heads (can be combined with tags)")), + OPT_BOOL(0, "tags", &patterns_opts.tags_only, N_("only show tags (can be combined with branches)")), + OPT_BOOL(0, "branches", &patterns_opts.branches_only, N_("only show branches (can be combined with tags)")), + OPT_HIDDEN_BOOL(0, "heads", &patterns_opts.branches_only, + N_("deprecated synonym for --branches")), OPT_BOOL(0, "exists", &exists, N_("check for reference existence without resolving")), OPT_BOOL(0, "verify", &verify, N_("stricter reference checking, " "requires exact ref path")), diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh index 33fb7a38ff..403f6b8f7d 100755 --- a/t/t1403-show-ref.sh +++ b/t/t1403-show-ref.sh @@ -121,13 +121,13 @@ test_expect_success 'show-ref -d' ' ' -test_expect_success 'show-ref --heads, --tags, --head, pattern' ' +test_expect_success 'show-ref --branches, --tags, --head, pattern' ' for branch in B main side do echo $(git rev-parse refs/heads/$branch) refs/heads/$branch || return 1 - done >expect.heads && - git show-ref --heads >actual && - test_cmp expect.heads actual && + done >expect.branches && + git show-ref --branches >actual && + test_cmp expect.branches actual && for tag in A B C do @@ -136,15 +136,15 @@ test_expect_success 'show-ref --heads, --tags, --head, pattern' ' git show-ref --tags >actual && test_cmp expect.tags actual && - cat expect.heads expect.tags >expect && - git show-ref --heads --tags >actual && + cat expect.branches expect.tags >expect && + git show-ref --branches --tags >actual && test_cmp expect actual && { echo $(git rev-parse HEAD) HEAD && - cat expect.heads expect.tags + cat expect.branches expect.tags } >expect && - git show-ref --heads --tags --head >actual && + git show-ref --branches --tags --head >actual && test_cmp expect actual && { @@ -165,6 +165,14 @@ test_expect_success 'show-ref --heads, --tags, --head, pattern' ' test_cmp expect actual ' +test_expect_success 'show-ref --heads is deprecated and hidden' ' + test_expect_code 129 git show-ref -h >short-help && + test_grep ! -e --heads short-help && + git show-ref --heads >actual 2>warning && + test_grep ! deprecated warning && + test_cmp expect.branches actual +' + test_expect_success 'show-ref --verify HEAD' ' echo $(git rev-parse HEAD) HEAD >expect && git show-ref --verify HEAD >actual &&
We call the tips of branches "heads", but this command calls the option to show only branches "--heads", which confuses the branches themselves and the tips of branches. Straighten the terminology by introducing "--branches" option that limits the output to branches, and deprecate "--heads" option used that way. We do not plan to remove "--heads" or "-h" yet; we may want to do so at Git 3.0, in which case, we may need to start advertising upcoming removal with an extra warning when they are used. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/git-show-ref.txt | 18 ++++++++++-------- builtin/show-ref.c | 16 +++++++++------- t/t1403-show-ref.sh | 24 ++++++++++++++++-------- 3 files changed, 35 insertions(+), 23 deletions(-)