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

[Matthew DeVore <matvore@comcast.net>]

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 :)