On 24 Jan 2016, at 22:35, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Sun, Jan 24, 2016 at 10:06 AM, Torsten Bögershausen <tboegi@xxxxxx> wrote: >> On 24.01.16 13:22, larsxschneider@xxxxxxxxx wrote: >>> From: Lars Schneider <larsxschneider@xxxxxxxxx> >>> diff --git 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)) { > > More idiomatic: > > if (ca.drv && ca.drv->clean && *ca.drv->clean) { Agreed! Although I will go with Jeff's suggestion to implement that logic in apply_filter. > >>> 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)) { > > Ditto. > >>> filter = ca.drv->smudge; >>> required = ca.drv->required; >>> } >>> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh >>> @@ -252,4 +252,18 @@ test_expect_success "filter: smudge empty file" ' >>> +test_expect_success 'disable filter with empty override' ' >>> + git config filter.disable.smudge false && >>> + git config filter.disable.clean false && > > test_config perhaps? I was wondering about that, too. Especially because I could also use test_config_global. Why does no test in t0021 use these functions? Should that be fixed? > >>> + echo "*.disable filter=disable" >.gitattributes && >>> + >>> + echo test >test.disable && >>> + git -c filter.disable.clean= add test.disable 2>err && >>> + ! test -s err && >> How about >> test_cmp /dev/null err >> to make debugging easier ? > > Even better: > > test_must_be_empty err && True! I will use that. Thanks, Lars -- 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