Re: [RFC PATCH] list-objects-filter: disable 'sparse:path' filters

From: Christian Couder <christian.couder@gmail.com>
Date: May 24, 2019, 19:43 UTC
Message-ID: <CAP8UFD1uojiT+PSAWTq9+hvr5Nzc707=4+Wfmm=pwJbzN1es-w@mail.gmail.com>
In-Reply-To: <20190524170758.GB7160@comcast.net>

On Fri, May 24, 2019 at 7:08 PM Matthew DeVore <matvore@comcast.net> wrote:

Show 10 quoted lines

>
> On Fri, May 24, 2019 at 02:03:18PM +0200, Christian Couder wrote:
> > For now though, let's just disable 'sparse:path' filters.
>
> This is probably the right thing to do. I did jump through a lot of hoops to
> support escaping sub-filters in my pending filter combination patchset, since
> sparse spec path names can have arbitrary characters. After this patch we only
> support a handful of characters in filterspecs, so a lot of that escaping logic
> can be dropped, at least for now. Anyway, this is not a complaint, just an
> observation.

Thanks for telling about it.

Show 12 quoted lines

> >       } else if (skip_prefix(arg, "sparse:path=", &v0)) {
> > -             filter_options->choice = LOFC_SPARSE_PATH;
> > -             filter_options->sparse_path_value = strdup(v0);
> > -             return 0;
> > +             if (errbuf) {
> > +                     strbuf_addstr(
> > +                             errbuf,
> > +                             _("sparse:path filters are now disabled"));
>
> This wording may leave room for misunderstanding, since it sounds a little like
> the filter can be re-enabled somehow. Maybe you can say "sparse:path filters
> support has been dropped [optional: 'for security reasons' etc.]"

Yeah, that seems better to me.

> >        * Please update _git_fetch() in git-completion.bash when you
>
> As the comment states, don't forget to update git-completion.bash :)

Ok, I will resend soon with better wording and an update to "git-completion.bash".