diff mbox series

[v2] Revert "doc: move git-cherry to plumbing"

Message ID e5b20f9ceb437a82c422136cb81b05a0521cab07.1736682716.git.code@khaugsbakk.name (mailing list archive)
State New
Headers show
Series [v2] Revert "doc: move git-cherry to plumbing" | expand

Commit Message

Kristoffer Haugsbakk Jan. 12, 2025, 11:54 a.m. UTC
From: Kristoffer Haugsbakk <code@khaugsbakk.name>

This reverts commit 61018fe9e005a54e18184481927519d64035220a.

git-cherry(1) is a high level command for checking what commits have and
have not been applied to some other branch.  Or at least as high level
as the git(1) suite offers.  In other words:

• it is a useful interrogator for a particular workflow; and
• there are no higher level commands on offer.

By contrast its use for scripting is somewhat narrow since it only
prints the patch application status and the hashes of the downstream
branch (not also the upstream branch equivalents).  git-patch-id(1)
gives a fuller picture by printing each hash and its corresponding
patch id.

Now this command is not nearly as convenient for the purpose of deleting
a *merged* branch as:

    git branch -d <branch>

Since that command will refuse to delete the branch if the commits are
not in the configured upstream ref.  But again it is the most convenient
command for the patch workflow.

This command might only be considered plumbing by way of the plumbing
contract that says that plumbing commands have stable output.  But
hopefully listing this command as Porcelain does not give the impression
that the output is not stable.  Output stability was in any case not the
motivation for moving this command to plumbing.

Users who need this interrogator should not have to look down in the
plumbing section in order to find it.

This also reverts its removal from Bash completion which Duy Nguyen
reported as a regression.[1]  The correct change for that plumbing move
would apparently have been to remove the `complete` category.

[1]: https://lore.kernel.org/git/CACsJy8AVGbS_NTZsUj_hD9D+t4YV1_S4KTD25Kda85syvoowyg@mail.gmail.com/

Reported-by: Duy Nguyen <pclouds@gmail.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---

Notes (series):
    § v1
    
    Resend with CC dropped.  Neither of the two seem to have been active
    for years.
    
    § v2
    
    The `---` comment on the patch:
    
    > Up to discussion whether cherry should be considered plumbing.
    > I lean towards considering it a rarely-used porcelain command, but
    > a case could be made either way so let's see what the list thinks.
    
    I don’t know why rarely-used is relevant.  This change now lists
    git-cherry(1) down in the (Porcelain) Interrogators section, along with
    commands such as:
    
    • git-bugreport(1)
    • git-count-objects(1)
    • git-diagnose(1)
    • git-whatchanged(1)
    
    Not everyday tools.  And that’s okay.

 command-list.txt                       |  2 +-
 contrib/completion/git-completion.bash | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)


base-commit: cc01bad4a9f566cf4453c7edd6b433851b0835e2

Comments

Kristoffer Haugsbakk Jan. 12, 2025, 12:30 p.m. UTC | #1
I forgot in-reply-to:

<e5b20f9ceb437a82c422136cb81b05a0521cab07.1732973210.git.code@khaugsbakk.name>
Junio C Hamano Jan. 13, 2025, 4:56 p.m. UTC | #2
kristofferhaugsbakk@fastmail.com writes:

> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> This reverts commit 61018fe9e005a54e18184481927519d64035220a.
>
> git-cherry(1) is a high level command for checking what commits have and
> have not been applied to some other branch.  Or at least as high level
> as the git(1) suite offers.  In other words:
>
> • it is a useful interrogator for a particular workflow; and
> • there are no higher level commands on offer.
>
> By contrast its use for scripting is somewhat narrow since it only
> prints the patch application status and the hashes of the downstream
> branch (not also the upstream branch equivalents).  git-patch-id(1)
> gives a fuller picture by printing each hash and its corresponding
> patch id.
>
> Now this command is not nearly as convenient for the purpose of deleting
> a *merged* branch as:
>
>     git branch -d <branch>
>
> Since that command will refuse to delete the branch if the commits are
> not in the configured upstream ref.  But again it is the most convenient
> command for the patch workflow.
>
> This command might only be considered plumbing by way of the plumbing
> contract that says that plumbing commands have stable output.  But
> hopefully listing this command as Porcelain does not give the impression
> that the output is not stable.  Output stability was in any case not the
> motivation for moving this command to plumbing.

I do not follow the above reasoning at all.

It is not like it is a crime to intarctively make use of a plumbing
command, or we intentionally try to hide plumbing command from them
by making it deliberately less accessible.  "git cat-file commit X"
may be handier than "git show -s X" for some people and that is not
to be frowned upon.

And what you call "might only be" is really the crucial thing to
consider.  If we want to keep a tool's output stable and machine
readable, we need to mark it as "meant for Porcelain writers", and
classifying the tool as plumbing is a pretty much established way to
do so.
Kristoffer Haugsbakk Jan. 14, 2025, 10:24 a.m. UTC | #3
On Mon, Jan 13, 2025, at 17:56, Junio C Hamano wrote:
> kristofferhaugsbakk@fastmail.com writes:
>> This command might only be considered plumbing by way of the plumbing
>> contract that says that plumbing commands have stable output.  But
>> hopefully listing this command as Porcelain does not give the impression
>> that the output is not stable.  Output stability was in any case not the
>> motivation for moving this command to plumbing.
>
> I do not follow the above reasoning at all.
>
> It is not like it is a crime to intarctively make use of a plumbing
> command, or we intentionally try to hide plumbing command from them
> by making it deliberately less accessible.  "git cat-file commit X"
> may be handier than "git show -s X" for some people and that is not
> to be frowned upon.
>
> And what you call "might only be" is really the crucial thing to
> consider.  If we want to keep a tool's output stable and machine
> readable, we need to mark it as "meant for Porcelain writers", and
> classifying the tool as plumbing is a pretty much established way to
> do so.

Okay.  I understand now.
Junio C Hamano Jan. 14, 2025, 5:35 p.m. UTC | #4
"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:

> On Mon, Jan 13, 2025, at 17:56, Junio C Hamano wrote:
>
>> It is not like it is a crime to intarctively make use of a plumbing
>> command, or we intentionally try to hide plumbing command from them
>> by making it deliberately less accessible.  "git cat-file commit X"
>> may be handier than "git show -s X" for some people and that is not
>> to be frowned upon.
>>
>> And what you call "might only be" is really the crucial thing to
>> consider.  If we want to keep a tool's output stable and machine
>> readable, we need to mark it as "meant for Porcelain writers", and
>> classifying the tool as plumbing is a pretty much established way to
>> do so.
>
> Okay.  I understand now.

I forgot to mention one thing worth addressing in your message,
though.  Making the tool more discoverable.  It is a valuable
consideration.

But I somehow think moving a tool between plumbing and Porcelain
boundary is not the way to do so.

There is a collection of "howto" articles in Documentation/howto/
that is meant to be the place to learn how to go from workflow and
objective to tools.  If you have a success story that your use of
"git cherry" helped greatly what you wanted to achieve, it may be a
good place to share it.

Thanks.
diff mbox series

Patch

diff --git a/command-list.txt b/command-list.txt
index e0bb87b3b5c..d73c8f59e63 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -72,7 +72,7 @@  git-check-mailmap                       purehelpers
 git-check-ref-format                    purehelpers
 git-checkout                            mainporcelain
 git-checkout-index                      plumbingmanipulators
-git-cherry                              plumbinginterrogators          complete
+git-cherry                              ancillaryinterrogators          complete
 git-cherry-pick                         mainporcelain
 git-citool                              mainporcelain
 git-clean                               mainporcelain
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 3d4dff3185c..5026ef595cd 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1746,6 +1746,17 @@  _git_checkout ()
 
 __git_sequencer_inprogress_options="--continue --quit --abort --skip"
 
+_git_cherry ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin cherry
+		return
+	esac
+
+	__git_complete_refs
+}
+
 __git_cherry_pick_inprogress_options=$__git_sequencer_inprogress_options
 
 _git_cherry_pick ()