Re: [PATCH] completion: new config var to use --sort in for-each-ref

From: D. Ben Knoble <ben.knoble@gmail.com>
Date: Jun 27, 2025, 19:53 UTC
Message-ID: <CALnO6CB8GWVNPS_eud6QEpzh7M5oQ6Vcm1-Hx5VqP=L1ovtyaw@mail.gmail.com>
In-Reply-To: <a3cd775d-99fa-49de-881b-1e8f178f9102@app.fastmail.com>

On Fri, Jun 27, 2025 at 3:48 PM Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com> wrote:

Show 29 quoted lines

>
> > Previously when completing refs eg. "git checkout <TAB>"
> > all refs were alphabetically ordered, this was an
> > implicit ordering and could not be changed.
>
> Style-wise, the commit message is supposed to discuss the current
> behavior (without the patch) in the present tense and what this patch
> does in the imperative mood.  Like you are commanding the code to
> change.[1]  Something like:
>
>     When completing refs, e.g. "git checkout <TAB>", all refs are
>     alphabetically ordered; this is an implicit ordering and cannot be
>     changed.
>
> This also gels with the general pattern of:
>
> 1. Introduce the current behavior
> 2. The problem it causes (maybe merged with (1))
> 3. What to do to fix it
>
> > Previously when completing refs eg. "git checkout <TAB>"
>
> s/eg./e.g./
>
> Maybe also some commas like
>
>     ..., e.g. "git checkout <TAB>", ...
>
> Or maybe it should be “i.e.”?

Alright, I'll take the bait:
- "e.g." and "i.e." are typically offset by commas, e.g., like this
- since there are other ways to complete refs, using "e.g." (roughly
"for example") is better than "i.e." (roughly, "that is") here

Show 43 quoted lines

>
> > This commit adds a new config var to allow setting
> > a custom ordering, the conf value will be used
> > for the --sort=<val> of for-each-ref.
> >
> > When a custom ordering is not set then alphabetical
> > default is kept, but this time is explicit as we
> > pass --sort='refname'
> >
> > This commit also adds '-o nosort' to 'complete'
> > to disable its default alphabetical ordering so
> > our custom ordering prevails.
>
> Super nitpick: the paragraphs could be wrapped closer to 72 characters/
> columns.[2][3]  This alternative (slightly modified) is closer
> to that yet not very uneven, still.
>
>     Previously when completing refs eg. "git checkout <TAB>" all refs
>     were alphabetically ordered, this was an implicit ordering and could
>     not be changed.
>
>     This commit adds a new config var to allow setting a custom ordering,
>     the conf value will be used for the --sort=<val> of for-each-ref.
>
>     When a custom ordering is not set then alphabetical default is kept,
>     but this time is explicit as we pass --sort='refname'
>
>     This commit also adds '-o nosort' to 'complete' to disable its default
>     alphabetical ordering so our custom ordering prevails.
>
> > This commit also adds '-o nosort' to 'complete'
>
> “This commit” should be replaced with just the imperative style
> “Also add”.[1]
>
> † 1: See Documentation/SubmittingPatches, “imperative-mood”
> 🔗 2: https://lore.kernel.org/git/CAPig+cT1VfY8QiUvrrV3-obTBP1439b6iwaebJtGwML5MScnQA@mail.gmail.com/
>
> --
> cheers
>
> Kristoffer Haugsbakk
>

All great suggestions, thanks.

-- 
D. Ben Knoble