Re: [PATCH v3 2/3] sparse-checkout: custom tab completion

On Sun, Jan 16, 2022 at 2:13 PM Junio C Hamano [off-list ref] wrote:

Elijah Newren [off-list ref] writes:

quoted

...  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.

quoted

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.