Re: [PATCH] dir.c: fix ignore processing within not-ignored directories

2013-06-04 Thread Karsten Blees
Am 02.06.2013 21:25, schrieb Junio C Hamano:
> Duy Nguyen  writes:
> 
>>> +   then
>>> +   false
>>> +   fi
>>> +'
>>
>> Nit pick, maybe this instead?
>>
>> test_must_fail grep "^one/a.1" output
> 
> Neither.
> 
>   ! grep "^one/a.1" output
> 

Nice. I actually tried "!" but without the space - which didn't work so I took 
the other t3001 tests as example...
--
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


Re: [PATCH] dir.c: fix ignore processing within not-ignored directories

2013-06-02 Thread Junio C Hamano
Duy Nguyen  writes:

>> +   then
>> +   false
>> +   fi
>> +'
>
> Nit pick, maybe this instead?
>
> test_must_fail grep "^one/a.1" output

Neither.

! grep "^one/a.1" output

The second bullet point in the "Don't" section of t/README may want
to be updated to clarify that test_must_fail is to test Git, and we
do not mean to catch segfaults by platform tools like grep.
--
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


Re: [PATCH] dir.c: fix ignore processing within not-ignored directories

2013-06-01 Thread Duy Nguyen
On Thu, May 30, 2013 at 3:32 AM, Karsten Blees  wrote:
> As of 95c6f271 "dir.c: unify is_excluded and is_path_excluded APIs", the
> is_excluded API no longer recurses into directories that match an ignore
> pattern, and returns the directory's ignored state for all contained paths.
>
> This is OK for normal ignore patterns, i.e. ignoring a directory affects
> the entire contents recursively.
>
> Unfortunately, this also "works" for negated ignore patterns ('!dir'), i.e.
> the entire contents is "not-ignored" recursively, regardless of ignore
> patterns that match the contents directly.
>
> In prep_exclude, skip recursing into a directory only if it is really
> ignored (i.e. the ignore pattern is not negated).
>
> Signed-off-by: Karsten Blees 

I think I've got a hang on the "unify" patch now.

Reviewed-by: Duy Nguyen 


> diff --git a/t/t3001-ls-files-others-exclude.sh 
> b/t/t3001-ls-files-others-exclude.sh
> +test_expect_success 'excluded directory overrides content patterns' '
> +
> +   git ls-files --others --exclude="one" --exclude="!one/a.1" >output &&
> +   if grep "^one/a.1" output

Actually I think this is a shortcoming of gitignore. You ask to
"exclude one except one/a.1" and one/a.1 should show up. '!' is
designed from day one to deal with the other way around ("include one
except one/a.1"). And it's arguable (and it was in the mail archive)
that if you already exclude "one", we should never ever descend there
to pick up "!a.1" from one/.gitignore. But we should do it with
already collected patterns, at least if we detect there are negated
patterns following the pattern that excludes a directory, e.g.
!one/a.1 or even !*.c. For the latter case, the user can always move
"!*.c" up before "one" if they don't want git to misinterpret and
descend in every excluded directory.

> +   then
> +   false
> +   fi
> +'

Nit pick, maybe this instead?

test_must_fail grep "^one/a.1" output

> +
> +test_expect_success 'negated directory doesn'\''t affect content patterns' '
> +
> +   git ls-files --others --exclude="!one" --exclude="one/a.1" >output &&
> +   if grep "^one/a.1" output
> +   then
> +   false
> +   fi
> +'

Same.
--
Duy
--
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