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.
The alternative is to hide sparse:path= support behind a flag which is disabled
by default, but I don't recommend doing that just to have an excuse to include
the URL-encoding logic.
Thank you for cleaning up.
> } 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.]"
> + }
> + return 1;
> }
> /*
> * Please update _git_fetch() in git-completion.bash when you
As the comment states, don't forget to update git-completion.bash :)
