On Sun, Jan 24, 2016 at 01:22:50PM +0100, larsxschnei...@gmail.com wrote:

> From: Lars Schneider <larsxschnei...@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.

> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to