Re: [PATCH] convert: legitimately disable clean/smudge filter with an empty override

[Lars Schneider <larsxschneider@gmail.com>]

On 25 Jan 2016, at 02:25, Junio C Hamano <gitster@pobox.com> wrote:

> larsxschneider@gmail.com writes:
> 
>> 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.
> 
> The above two sentences do not make any sense to me.  You observe
> that the command refuses to work when the variable is set to an
> empty string--why then can you claim "filter can be disabled if set
> to an empty string"?  I'd consider that the system is not working
> with such a configuration, i.e. "filter cannot be disabled by
> setting it to empty; such a request will result in an error".

If I am not mistaken then Git exits with 0 (success) and an error message
if the filter command is empty and the filter is not required. If the filter
command is empty and the filter is required then Git will exit with 1 (error).

How about this?

If the clean/smudge command of a Git filter driver (filter.<driver>.smudge and
filter.<driver>.clean) is set to an empty string ("") and the filter driver is
not required (filter.<driver>.required=false) then Git will run successfully.
However, Git will print an error for every file that is affected by the filter.

Teach Git to consider an empty clean/smudge filter as legitimately disabled
and do not print an error message if the filter is not required.

Thanks,
Lars


> 
>> Teach Git to consider an empty clean/smudge filter as legitimately disabled
>> and do not print an error message.
> 
> On the other hand, this does make sense to me, as I do not think of
> a good way to say "earlier configuration entry said we should use
> this filter, but we actually do not want to use that one (or any)"
> because a configuration, unlike attributes entry, cannot be reset.
> The closest you can do is to set it to empty, so it may be a good
> new feature to do this.
> 
>