On Fri, May 24, 2019 at 7:08 PM Matthew DeVore <matvore@xxxxxxxxxxx> wrote:
>
> 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.
> > } 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".
