| Message ID | pull.719.v2.git.git.1583325514607.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] doc: use 'rev' instead of 'commit' for merge-base arguments | expand |
"Takuya N via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Takuya Noguchi <takninnovationresearch@gmail.com> > > The notation <commit> can be misunderstandable only for commit SHA1, Let's step back a bit. When our documentation says $ git log <commit> we expect that the readers to understand that all of the following are accepted: $ git log master $ git log master~10 $ git log 3868ac720f $ git log 3868ac720f7a26f3241f43764d0dc790ec55238f What gave you the impression that only the last one is valid? We need to fix _that_. The side discussion we had with Dscho touched another important point (i.e. when a command wants to take a commit but a user gives it a tag that points at a commit, the command almost always accepts the tag, finds the commit the tag points at, and uses that commit instead of the tag---we often mark such a parameter that expects commit but does not have to name a commit object <commit-ish>), but it is more or less orthogonal. All of these are accepted, $ git log v2.25.0 $ git log 61e952148 $ git log 61e9521487999585dc2b8f27c2a65226fb531a07 not just the last one. > but merge-base accepts any commit references. Like rev-parse, the name > of arguments should be <rev> instead of <commit>. And "like rev-parse" is a poor justification. It is a lowest-level command that was written in dark ages, and the language used in the documentation hasn't been updated to more modern terms. Back then, we said "revision" (and "rev" is a short for it) and the term was invented to mean commit (see "$ git help glossary") but was used loosely and more-or-less interchangeably with "object name" (in other words, when the speaker who said "rev" did not necessarily mean to limit the reference to commits, the word did not limit itself to commit-ish but also covered trees and blobs). Now, in the context of "git rev-parse", $ git rev-parse eacd13fab7e3 does make sense. It is OK to give it a short version of a tree object name (it is the tree of the commit pointed by the v2.25.0 release), so "git rev-parse <object-name>" (or "git rev-parse <rev>") would be OK. It is however a poor example to base our decision on how to explain merge-base. The command wants to _use_ two (or more) commits to work on, so like many other commands, when it is fed a tag, it tries to see if it points at a commit (and barfs if it does not) and use that commit instead. In other words, it takes commit-ish. Saying that it takes <commit> is *not* so bad, but changing it to <rev> is a move backwards, I'd have to say. Thanks for working on it, but s/ref/rev/ is not a good change.
Junio C Hamano <gitster@pobox.com> writes: > ... another important > point (i.e. when a command wants to take a commit but a user gives > it a tag that points at a commit, the command almost always accepts > the tag, finds the commit the tag points at, and uses that commit > instead of the tag ... A side note I forgot to add. A few commands take both commit and tag but want to do different things depending on the kind of object they get, and for them, <commit> and <commit-ish> should be used carefully in the documentation. For example, "git cat-file commit v2.25.0" and "git cat-file tag v2.25.0" do two different things. They should be described as $ git cat-file commit <commit-ish> $ git cat-file tag <tag> "git merge <commit>" and "git merge <tag>" do different things, even though <tag> must be a <commit-ish>. But most of the Git subcommands (including the "cat-file commit" case) peel a tag as needed, so these special cases are minority.
diff --git a/Documentation/git-merge-base.txt b/Documentation/git-merge-base.txt index 2d944e0851f..60438a00871 100644 --- a/Documentation/git-merge-base.txt +++ b/Documentation/git-merge-base.txt @@ -9,11 +9,11 @@ git-merge-base - Find as good common ancestors as possible for a merge SYNOPSIS -------- [verse] -'git merge-base' [-a|--all] <commit> <commit>... -'git merge-base' [-a|--all] --octopus <commit>... -'git merge-base' --is-ancestor <commit> <commit> -'git merge-base' --independent <commit>... -'git merge-base' --fork-point <ref> [<commit>] +'git merge-base' [-a|--all] <rev> <rev>... +'git merge-base' [-a|--all] --octopus <rev>... +'git merge-base' --is-ancestor <rev> <rev> +'git merge-base' --independent <rev>... +'git merge-base' --fork-point <ref> [<rev>] DESCRIPTION ----------- diff --git a/builtin/merge-base.c b/builtin/merge-base.c index e3f8da13b69..e03f2269e88 100644 --- a/builtin/merge-base.c +++ b/builtin/merge-base.c @@ -29,11 +29,11 @@ static int show_merge_base(struct commit **rev, int rev_nr, int show_all) } static const char * const merge_base_usage[] = { - N_("git merge-base [-a | --all] <commit> <commit>..."), - N_("git merge-base [-a | --all] --octopus <commit>..."), - N_("git merge-base --independent <commit>..."), - N_("git merge-base --is-ancestor <commit> <commit>"), - N_("git merge-base --fork-point <ref> [<commit>]"), + N_("git merge-base [-a | --all] <rev> <rev>..."), + N_("git merge-base [-a | --all] --octopus <rev>..."), + N_("git merge-base --independent <rev>..."), + N_("git merge-base --is-ancestor <rev> <rev>"), + N_("git merge-base --fork-point <ref> [<rev>]"), NULL }; @@ -158,7 +158,7 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix) OPT_CMDMODE(0, "is-ancestor", &cmdmode, N_("is the first one ancestor of the other?"), 'a'), OPT_CMDMODE(0, "fork-point", &cmdmode, - N_("find where <commit> forked from reflog of <ref>"), 'f'), + N_("find where <rev> forked from reflog of <ref>"), 'f'), OPT_END() };