On 24 Jan 2016, at 22:45, Jeff King <peff@xxxxxxxx> wrote:
> On Sun, Jan 24, 2016 at 01:22:50PM +0100, larsxschneider@xxxxxxxxx wrote:
>
>> From: Lars Schneider <larsxschneider@xxxxxxxxx>
>>
>> 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?
Yes :-)
Thanks,
Lars
>
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html