Re: [PATCH] convert: legitimately disable clean/smudge filter with an empty override
- From
Jeff King <peff@peff.net>
- Date
- Jan 24, 2016, 21:45 UTC
- Message-ID
- <20160124214459.GA31091@sigill.intra.peff.net>
- In-Reply-To
- <1453638170-52041-1-git-send-email-larsxschneider@gmail.com>
On Sun, Jan 24, 2016 at 01:22:50PM +0100, larsxschneider@gmail.com wrote:
Show 8 quoted lines
> From: Lars Schneider <larsxschneider@gmail.com> > > A clean/smudge filter can be disabled if set to an empty string. However, > Git will try to run the empty string as command which results in a error > message per processed file. > > Teach Git to consider an empty clean/smudge filter as legitimately disabled > and do not print an error message.
That makes sense to me, as I do not think the empty filter name can possibly do anything useful. You omitted the real motivation here, but I know what it is from past discussions: you want to be able to temporarily disable a filter with "git -c filter.foo.clean= ...". Which I think makes it more immediately obvious that this is a useful thing to have, and not just user error.
Show 22 quoted lines
> diff --git a/convert.c b/convert.c
> index 814e814..58af965 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -786,7 +786,7 @@ int convert_to_git(const char *path, const char *src, size_t len,
> struct conv_attrs ca;
>
> convert_attrs(&ca, path);
> - if (ca.drv) {
> + if (ca.drv && ca.drv->clean && strlen(ca.drv->clean)) {
> filter = ca.drv->clean;
> required = ca.drv->required;
> }
> @@ -835,7 +835,7 @@ static int convert_to_working_tree_internal(const char *path, const char *src,
> struct conv_attrs ca;
>
> convert_attrs(&ca, path);
> - if (ca.drv) {
> + if (ca.drv && ca.drv->smudge && strlen(ca.drv->smudge)) {
> filter = ca.drv->smudge;
> required = ca.drv->required;
> }This catches two calls, but I think there are others. What about would_convert_to_git_filter_fd and convert_to_git_filter_fd?
Would it make more sense for apply_filter() to treat the empty string as a noop, just as it does for NULL?
I.e.:
diff --git a/convert.c b/convert.c index 814e814..02d5f1e 100644 --- a/convert.c +++ b/convert.c @@ -395,7 +395,7 @@ static int apply_filter(const char *path, const char *src, size_t len, int fd, struct async async; struct filter_params params; - if (!cmd) + if (!cmd || !*cmd) return 0; if (!dst) which I think would cover all callers? -Peff