git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "SZEDER Gábor" <szeder.dev@gmail.com>,
	"Lessley Dennington via GitGitGadget" <gitgitgadget@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Derrick Stolee" <stolee@gmail.com>,
	johannes.schindelin@gmail.com,
	"Lessley Dennington" <lessleydennington@gmail.com>
Subject: Re: [PATCH v3 2/3] sparse-checkout: custom tab completion
Date: Mon, 17 Jan 2022 10:14:58 -0800	[thread overview]
Message-ID: <CABPp-BErg-RtyycXaRXYfQHEQXA4q-FU9Q6nYkSHJsqL-04oXw@mail.gmail.com> (raw)
In-Reply-To: <xmqqv8yjz5us.fsf@gitster.g>

On Sun, Jan 16, 2022 at 2:13 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> > ...  If our
> > guide is merely what the command swallows, then we should forgo
> > completion for these subcommands, because it's not possible to
> > enumerate all possible completions.
>
> I am not sure if I follow.  I do not think it makes sense to aim for
> enumerating EVERYTHING the command would accept.  But I do not know
> that is the same as "merely what the command swallows".

I don't understand your distinction; I think your second sentence is
the same thing I said.

> > I don't think that's a useful guide or starting point, so we
> > instead need to discuss what are reasonable completions.
>
> I do not think it is a good idea to refrain from suggesting anything
> that has a possibility of being wring, either, though.  If a path
> that is not a directory (either because it is a file in the current
> checkout, or because it is a directory only in a different branch)
> is given, it might not make sense in the cone-mode for the current
> checkout, but it may well make sense when a different branch is
> checked out.

Completing on files and directories would be a reasonable first cut,
but that's what we already had before this series was proposed.  It
has the downsides of
  * [cone-mode] making the majority of suggested completions wrong
  * [non-cone mode] subtly recommending individually adding all wanted
files, increasing the number of patterns and exacerbating the
quadratic behavior that non-cone mode experiences with every
unpack_trees() call that touches the working tree.
  * [both modes] potentially making it hard to find or select the
valid choices you do want (there are many more files than directories)

But, despite all that, completing on files and directories is a
somewhat reasonable first cut.

Lessley was trying to aim higher with her submission, though, and I
don't see why she should be dissuaded.  Something better would be
nice.

>  Or you may not even in the cone-mode, and in which
> case, as SZEDER suggested, a single filename may perfectly make
> sense.  A user who said READM<TAB> and does not see it completed to
> README.md would be quite confused.

I'm not sure I follow.  READM<TAB> already doesn't complete to
README.md in the following example command lines:
   'cd READM<TAB>'
   'ssh READM<TAB>'
That doesn't seem to cause user confusion, so I don't think
disallowing it in cone-mode would cause confusion.  Suggesting it, on
the other hand, may well cause confusion given that cone-mode is
explicitly about directories and is documented as such.

If you're only talking about non-cone mode, then this may be a
reasonable objection, though I'm not sure I agree with it even then.
In addition to the fact that individual file completion can be
detrimental to the user in non-cone mode for performance reasons, the
documentation never explicitly states files are acceptable to
sparse-checkout {set,add}.  It always mentions "directories" and
"patterns".  We can't complete all patterns, so we have to pick a
subset.  I don't see why "directories and files" is a better subset to
pick than "just directories".

> Are we limiting ourselves to directories only when we know we are in
> the cone-mode and showing both directories and files otherwise?

That is one possibility, though not the one Lessley proposed.

> I think the guiding principle ought to be that we show completion
> that
>
>  - is cheap enough to compute in interactive use (e.g. we should
>    refrain from looking for directories in all possible branches,
>    but instead just look at the working tree and possibly in the
>    index),

I agree, except with the suggestion to use the working tree or index
in the case of sparse-checkouts.  The working tree shouldn't be used
because it doesn't have the needed information:
  * `git clone --sparse` starts with zero subdirectories present
  * `set` and `add` are likely being used to change sparsity to
include something not already present

The index shouldn't be used because it is not cheap for interactive use:
  * it contains recursive entries below directories of interest that
have to be filtered out
  * with the sparse-index, it may have sparse-directories hiding what
we want, forcing us to fully inflate the index to find the pieces we
do want

I agree with Lessley's choice of using ls-tree and HEAD to achieve
cheap interactive use.

>  - is simple enough to explain to the users to understand what the
>    rules are, and
>
>  - gives useful enough candidates.

I agree with these 3 criteria.

> "We only look for directories (without going recursive) in the
> working tree" does satisfy the first two, but I am not sure it is
> more useful than "We show files and directories (without going
> recursive) in the working tree", which also satisfies the first
> two.

Well, Lessley certainly thought directories-only was more useful, and
in fact labelled "files and directories" as a bug/issue in her cover
letter.  Stolee commented on the series and also didn't see anything
wrong with that claim.

> Of course, if the completion limits to directories only in a
> repository in the cone-mode, I would not worry about the exclusion
> of non-directories will confuse users.

I'd prefer directories-only for both modes, but could accept
directories-only for cone mode and both-files-and-directories for
non-cone mode.

Especially since I think we're going to deprecate non-cone mode regardless.

  reply	other threads:[~2022-01-17 18:15 UTC|newest]

Thread overview: 95+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-30  0:32 [PATCH 0/2] sparse checkout: custom bash completion updates Lessley Dennington via GitGitGadget
2021-12-30  0:32 ` [PATCH 1/2] sparse-checkout: custom tab completion tests Lessley Dennington via GitGitGadget
2021-12-30 13:43   ` Derrick Stolee
2021-12-30 16:19     ` Lessley Dennington
2021-12-30 17:43       ` Derrick Stolee
2021-12-31 19:27         ` Elijah Newren
2022-01-04 19:19           ` Lessley Dennington
2021-12-30  0:32 ` [PATCH 2/2] sparse-checkout: custom tab completion Lessley Dennington via GitGitGadget
2021-12-30 13:50   ` Derrick Stolee
2021-12-30 16:24     ` Lessley Dennington
2021-12-30 19:26 ` [PATCH v2 0/2] sparse checkout: custom bash completion updates Lessley Dennington via GitGitGadget
2021-12-30 19:26   ` [PATCH v2 1/2] sparse-checkout: custom tab completion tests Lessley Dennington via GitGitGadget
2021-12-31 20:03     ` Elijah Newren
2021-12-31 22:20       ` Junio C Hamano
2021-12-31 22:25         ` Elijah Newren
2022-01-04 19:25         ` Lessley Dennington
2022-01-04 20:25           ` Elijah Newren
2022-01-05 14:05             ` Lessley Dennington
2022-01-04 19:24       ` Lessley Dennington
2021-12-30 19:26   ` [PATCH v2 2/2] sparse-checkout: custom tab completion Lessley Dennington via GitGitGadget
2021-12-31 22:52     ` Elijah Newren
2022-01-04 19:41       ` Lessley Dennington
2022-01-04 20:42         ` Elijah Newren
2022-01-05 20:20           ` Lessley Dennington
2022-01-05 22:55             ` Elijah Newren
2022-01-10 18:59   ` [PATCH v3 0/3] sparse checkout: custom bash completion updates Lessley Dennington via GitGitGadget
2022-01-10 18:59     ` [PATCH v3 1/3] sparse-checkout: custom tab completion tests Lessley Dennington via GitGitGadget
2022-01-10 18:59     ` [PATCH v3 2/3] sparse-checkout: custom tab completion Lessley Dennington via GitGitGadget
2022-01-15  9:57       ` SZEDER Gábor
2022-01-16  1:03         ` Elijah Newren
2022-01-16 22:13           ` Junio C Hamano
2022-01-17 18:14             ` Elijah Newren [this message]
2022-01-17 19:40               ` Junio C Hamano
2022-01-18 17:56                 ` Lessley Dennington
2022-01-22  1:07                   ` Lessley Dennington
2022-01-22  1:08                     ` Lessley Dennington
2022-01-22  2:11                       ` Lessley Dennington
2022-01-18 21:02               ` SZEDER Gábor
2022-01-18 21:43                 ` Elijah Newren
2022-01-18 17:59           ` Lessley Dennington
2022-01-18 22:22           ` SZEDER Gábor
2022-01-18 23:30             ` Elijah Newren
2022-01-10 18:59     ` [PATCH v3 3/3] sparse-checkout: limit tab completion to a single level Lessley Dennington via GitGitGadget
2022-01-12 23:43       ` Lessley Dennington
2022-01-13  0:00         ` Junio C Hamano
2022-01-13  0:38         ` Elijah Newren
2022-01-13 19:02           ` Lessley Dennington
2022-01-10 20:38     ` [PATCH v3 0/3] sparse checkout: custom bash completion updates Elijah Newren
2022-01-11 17:17       ` Lessley Dennington
2022-01-11 19:45         ` Taylor Blau
2022-01-12 18:35           ` Lessley Dennington
2022-01-27 21:21     ` [PATCH v4 0/3] completion: sparse-checkout updates Lessley Dennington via GitGitGadget
2022-01-27 21:21       ` [PATCH v4 1/3] completion: add sparse-checkout tests Lessley Dennington via GitGitGadget
2022-01-28  0:08         ` Elijah Newren
2022-01-28  1:56           ` Junio C Hamano
2022-01-28  2:04             ` Elijah Newren
2022-01-27 21:21       ` [PATCH v4 2/3] completion: sparse-checkout updates Lessley Dennington via GitGitGadget
2022-01-28  1:21         ` Elijah Newren
2022-01-31 20:03           ` Lessley Dennington
2022-01-31 21:37             ` Elijah Newren
2022-01-27 21:21       ` [PATCH v4 3/3] completion: ensure cone mode completion with multiple <TAB>s Lessley Dennington via GitGitGadget
2022-01-28  1:53         ` Elijah Newren
2022-02-03 20:44       ` [PATCH v5 0/3] completion: sparse-checkout updates Lessley Dennington via GitGitGadget
2022-02-03 20:44         ` [PATCH v5 1/3] completion: address sparse-checkout issues Lessley Dennington via GitGitGadget
2022-02-03 23:52           ` Elijah Newren
2022-02-04  0:34             ` Lessley Dennington
2022-02-03 20:44         ` [PATCH v5 2/3] completion: improve sparse-checkout cone mode directory completion Lessley Dennington via GitGitGadget
2022-02-03 20:44         ` [PATCH v5 3/3] completion: handle unusual characters for sparse-checkout Lessley Dennington via GitGitGadget
2022-02-03 23:58           ` Elijah Newren
2022-02-04  0:37             ` Lessley Dennington
2022-02-04  4:25             ` Ævar Arnfjörð Bjarmason
2022-02-04 21:55           ` Junio C Hamano
2022-02-03 21:48         ` [PATCH v5 0/3] completion: sparse-checkout updates Junio C Hamano
2022-02-03 22:17           ` Lessley Dennington
2022-02-03 23:28             ` Junio C Hamano
2022-02-03 23:59               ` Lessley Dennington
2022-02-04  2:43                 ` Lessley Dennington
2022-02-04  3:28                   ` Lessley Dennington
2022-02-04  4:21                   ` Ævar Arnfjörð Bjarmason
2022-02-04  3:26         ` [PATCH v6 " Lessley Dennington via GitGitGadget
2022-02-04  3:26           ` [PATCH v6 1/3] completion: address sparse-checkout issues Lessley Dennington via GitGitGadget
2022-02-04  3:26           ` [PATCH v6 2/3] completion: improve sparse-checkout cone mode directory completion Lessley Dennington via GitGitGadget
2022-02-04  3:26           ` [PATCH v6 3/3] completion: handle unusual characters for sparse-checkout Lessley Dennington via GitGitGadget
2022-02-04  6:05           ` [PATCH v6 0/3] completion: sparse-checkout updates Elijah Newren
2022-02-04 17:04             ` Junio C Hamano
2022-02-04 17:55               ` Elijah Newren
2022-02-04 19:54                 ` Junio C Hamano
2022-02-04 20:01                   ` Elijah Newren
2022-02-04 21:47                     ` Junio C Hamano
2022-02-07 17:31           ` [PATCH v7 " Lessley Dennington via GitGitGadget
2022-02-07 17:31             ` [PATCH v7 1/3] completion: address sparse-checkout issues Lessley Dennington via GitGitGadget
2022-02-07 17:31             ` [PATCH v7 2/3] completion: improve sparse-checkout cone mode directory completion Lessley Dennington via GitGitGadget
2022-02-07 17:31             ` [PATCH v7 3/3] completion: handle unusual characters for sparse-checkout Lessley Dennington via GitGitGadget
2022-04-06  9:42               ` Adam Dinwoodie
2022-02-08  4:16             ` [PATCH v7 0/3] completion: sparse-checkout updates Elijah Newren

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=CABPp-BErg-RtyycXaRXYfQHEQXA4q-FU9Q6nYkSHJsqL-04oXw@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmail.com \
    --cc=lessleydennington@gmail.com \
    --cc=stolee@gmail.com \
    --cc=szeder.dev@gmail.com \
    --subject='Re: [PATCH v3 2/3] sparse-checkout: custom tab completion' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).