git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: Lessley Dennington via GitGitGadget <gitgitgadget@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Derrick Stolee <stolee@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	johannes.schindelin@gmail.com,
	Lessley Dennington <lessleydennington@gmail.com>
Subject: Re: [PATCH v3 2/3] sparse-checkout: custom tab completion
Date: Tue, 18 Jan 2022 15:30:05 -0800	[thread overview]
Message-ID: <CABPp-BEwMAPHGt5xD9jDU58grbrAqCdqNY9Nh8UJGLKuLbArXQ@mail.gmail.com> (raw)
In-Reply-To: <20220118222234.GB2632@szeder.dev>

On Tue, Jan 18, 2022 at 2:22 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
>
> On Sat, Jan 15, 2022 at 05:03:24PM -0800, Elijah Newren wrote:
> > On Sat, Jan 15, 2022 at 1:57 AM SZEDER Gábor <szeder.dev@gmail.com> wrote:
> > >
> > > On Mon, Jan 10, 2022 at 06:59:51PM +0000, Lessley Dennington via GitGitGadget wrote:
...
> > > > 4. A list of directories (but not files) is provided when users enter git
> > > > sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
> > >
> > > Why limit completion only to directories?  Both of those subcommands
> > > accept files,
> >
> > Discussed in part at [1], but let me give a more detailed answer.
>
> It was a semi-rhetorical question.  Whether the reasons for expluding
> files are sound or not, it should be convincingly justified in the
> commit message.

Fair enough.

...
> I used the following command to create a sparse-checkout from
> linux.git to build and play with 'perf':
>
>   git sparse-checkout set tools/perf/ tools/scripts/ tools/build/ tools/include/ tools/arch/x86/ tools/lib/ /.gitignore /.gitattributes
>
> Including the top-level '.gitignore' and '.gitattributes' was
> important, becase those ignore object files and specify userdiff.
> Now, I wouldn't mind having other files present in the top-level
> directory, because there are only a handful of files there.  However,
> being able to specify just those two files to 'git sparse-checkout'
> was great, because I didn't even have to think about what wildcard
> pattern to use, and what negative pattern to use to exclude anything
> that might have been included recursively.

Yeah, we should really make cone mode the default, because then this
simplifies to:

    git sparse-checkout set tools/perf/ tools/scripts/ tools/build/
tools/include/ tools/arch/x86/ tools/lib/

In cone-mode you automatically get the directories specified PLUS:

  * all files in the toplevel directory (you always get these in cone mode)
  * all files directly within a parent (or ancestor) of one of the
directories you specified (thus all files directly under tools/ and
tools/arch/).

> I don't remember having any performance issues with it, on the
> contrary, I do remember that Git suddenly became much faster that in
> the full worktree.

Cool, thanks for testing it out and reporting.

If I can be permitted to share a bit of my experience...

In this specific case, you did keep it to 8 patterns and the linux
kernel "only" has ~60k files.  So O(N*M) isn't too bad here...but
experience at $DAYJOB is that even with only 60k files, the pattern
list tends to grow and it's not clear to users why there are so many
ugly pauses when "Git used to be fast".  Users tend to not make the
connection either between the sparsity patterns and the slowness for
whatever reason, and when I'm asked questions about the slowness, I
may have to check a few other possible causes before finally realizing
that it's just "too many sparsity patterns".

At least, that's the way it was at $DAYJOB before I switched our
internal "sparsify" tool to just use cone mode.  Then that type of
problem went away.

> So I'm fairly convinced that specifying files to sparse-checkout is a
> feature that can make users' life easier.  It certainly made my life
> easier.

Or we can make it even easier and faster by using cone mode.  That
really, really should be the default (as we've been talking about for
a couple cycles now), and I think your email reinforces my belief in
it.  I've got a series ready after some other things merge down to
next.

> On a related note: I just noticed the leading slashes in '/.gitignore'
> and '/.gitattributes'.  __git_complete_index_file() is not ready for
> that, I'm afraid; but I don't think the proposed patches could deal
> with that, either (but I didn't actually try).
>
> It would be great if completion could cope with patterns starting with
> '/' and/or '!'.

Ah, very good point.  The leading '/' is kind of critical here,
otherwise you get all .gitignore files in every directory at every
depth, so completing on files (as currently done) wouldn't really help
at all in your case.


Since you brought up a new good point here, can I also mention two
others not previously covered in this thread (i.e. besides scaling)?

* If we complete file and directory names in non-cone mode, and those
file and directory names contain a leading '#' or '!' or contain
anywhere in their names a '*', '?', '\', '[', or ']', then the users
may be in for a very nasty surprise.  (Which perhaps suggests that in
non-cone mode, we shouldn't provide file OR directory completions for
set/add?)

* sparse-checkout currently ignores prefix[1], causing file and
directory completions to be wrong if the user is running from a
subdirectory.  This is just a bug in cone mode, though I'm not sure if
it's bug or feature in non-cone mode[2].  A decision about what it is
might have a bearing on what kinds of completions make sense in
non-cone mode (and might reinforce files/directories, or suggest
something else.).

[1] https://lore.kernel.org/git/29f0410e-6dfa-2e86-394d-b1fb735e7608@gmail.com/
[2] https://lore.kernel.org/git/CABPp-BH5woi6KY7OBpnsS-M2EmgLHii9zs8rSwrgcPFkOAvn_A@mail.gmail.com/

  reply	other threads:[~2022-01-18 23:30 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
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 [this message]
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

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

  git send-email \
    --in-reply-to=CABPp-BEwMAPHGt5xD9jDU58grbrAqCdqNY9Nh8UJGLKuLbArXQ@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

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 NNTP newsgroup(s).