All of lore.kernel.org
 help / color / mirror / Atom feed
From: Toon Claes <toon@iotcl.com>
To: Mirko Faina <mroik@delayed.space>,
	Pablo Sabater <pabloosabaterr@gmail.com>
Cc: git@vger.kernel.org, Ben Knoble <ben.knoble@gmail.com>
Subject: Re: [PATCH] bash-completions: add --[no-]graph-indent
Date: Mon, 09 Nov 2026 16:40:27 +0100	[thread overview]
Message-ID: <87zevixbbo.fsf@dev.null.iotcl.com.invalid> (raw)
In-Reply-To: <aujlNwM6X3BLz22V@exploit>

Mirko Faina <mroik@delayed.space> writes:

> On Thu, Oct 29, 2026 at 01:45:15PM +0000, Pablo Sabater wrote:
>> I didn't know about this so I completely forgot to add it, however from
>> what I read, this is the __git_log_common_options and I'm not sure if
>> this is a "common" opt, as I don't even see --graph here.
>> 
>> I searched where --graph is, and it's here:
>> 
>> l 2383-2409 'git-completion.bash'
>> 
>> 	__gitcomp "
>> [more opts]
>> 		--graph
>> [more opts]
>> 		$merge
>> 		$__git_diff_common_options
>> 		"
>> 	return
>> 	;;
>> 
>> I think it would fit better here (not sure, I just quickly skimmed the
>> file).
>
> Yes, you're right. I was on automatic mode when choosing where to place
> it and defaulted to the place where I put an option of mine that worked
> for log. Since this is specific to "git log" I'll place it where you
> suggested.
>
>> Also, I missed this for the --graph-lane-limit option so maybe it should
>> be added too?
>
> Not familiar with the option but will look into it, might as well add
> it.
>
>> My main doubt is, both these options proposed (yours and
>> --graph-lane-limit) are --graph dependent, which means if someone
>> autocompletes them but doesn't use --graph, they will get an error
>> saying that --graph is needed. I'd say we do what other --graph
>> dependent options do, but AFAIK these two are the only ones.
>> 
>> From Toon's Git Merge talk, more options for --graph option will be
>> developed so we should maybe think about how to do it in a way that 
>> doesn't autocomplete if it's going to error? (maybe that does exist,
>> given the multiple options that depend on other options)
>
> From what I can tell the only reason we options through bash-completions
> is because these options are not handled by the parse-options.h
> machinery. Ideally we'd want to port these options to that? I'm assuming
> the reason it wasn't was because of the common options.
>
> With the parse-options.h machinery you can't tell which option depends
> on which other, so it just shows every option available for a command
> when autocompleting. I'm not sure it's worth it to implement such a
> feature with bash-completions when the parse-options.h machinery
> doesn't.

Yes, I agree. It would be cool if parse-options.h would be smarter in
this regard, but for now that's not the case, and I'm fine with that.
I'm pretty sure (although I cannot think of any right now) there are
other options (for other commands) that only work if combined.

I appreciate thinking about this, but that's something for the future.

> Thank you for the review, will reroll.
> Mirko Faina

-- 
Laters,
Toon

  reply	other threads:[~2026-11-09 15:40 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-13 19:50 [PATCH] bash-completions: add --[no-]graph-indent Mirko Faina
2026-10-29 12:16 ` Mirko Faina
2026-10-29 13:45 ` Pablo Sabater
2026-10-29 14:17   ` Ben Knoble
2026-11-02 19:53   ` Mirko Faina
2026-11-09 15:40     ` Toon Claes [this message]
2026-11-02 21:47 ` [PATCH v2] completions: add some missing options for git log Mirko Faina
2026-11-09 15:41   ` Toon Claes
2026-11-09 20:21     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87zevixbbo.fsf@dev.null.iotcl.com.invalid \
    --to=toon@iotcl.com \
    --cc=ben.knoble@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=mroik@delayed.space \
    --cc=pabloosabaterr@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.