Subject: Re: [PATCH v3 06/12] dir: if our pathspec might match files under a dir, recurse into it

From: Junio C Hamano <[email-masked]>
Date: Fri, 13 Sep 2019 12:45:17 -0700

Elijah Newren <[email-masked]> writes:

> For git clean, if a directory is entirely untracked and the user did not
> specify -d (corresponding to DIR_SHOW_IGNORED_TOO), then we usually do
> not want to remove that directory and thus do not recurse into it.

Makes sense.  To clean named paths in such a directory, we'd need an
option to recurse into it to find them, yet make sure the directory
itself does not get removed.

> However, if the user manually specified specific (or even globbed) paths
> somewhere under that directory to remove, then we need to recurse into
> the directory to make sure we remove the relevant paths under that
> directory as the user requested.

Surely.

> @@ -1939,8 +1939,10 @@ static enum path_treatment read_directory_recursive(struct dir_struct *dir,
>  		/* recurse into subdir if instructed by treat_path */
>  		if ((state == path_recurse) ||
>  			((state == path_untracked) &&
> -			 (dir->flags & DIR_SHOW_IGNORED_TOO) &&
> -			 (get_dtype(cdir.de, istate, path.buf, path.len) == DT_DIR))) {
> +			 (get_dtype(cdir.de, istate, path.buf, path.len) == DT_DIR) &&
> +			 ((dir->flags & DIR_SHOW_IGNORED_TOO) ||
> +			  do_match_pathspec(istate, pathspec, path.buf, path.len,
> +					    baselen, NULL, DO_MATCH_LEADING_PATHSPEC) == MATCHED_RECURSIVELY_LEADING_PATHSPEC))) {
>  			struct untracked_cache_dir *ud;
>  			ud = lookup_untracked(dir->untracked, untracked,
>  					      path.buf + baselen,

OK.

> diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh
> index 12617158db..d83aeb7dc2 100755
> --- a/t/t7300-clean.sh
> +++ b/t/t7300-clean.sh
> @@ -691,7 +691,7 @@ test_expect_failure 'git clean -d skips nested repo containing ignored files' '
>  	test_path_is_file nested-repo-with-ignored-file/file
>  '
>  
> -test_expect_failure 'git clean handles being told what to clean' '
> +test_expect_success 'git clean handles being told what to clean' '
>  	mkdir -p d1 d2 &&
>  	touch d1/ut d2/ut &&
>  	git clean -f */ut &&
> @@ -707,7 +707,7 @@ test_expect_success 'git clean handles being told what to clean, with -d' '
>  	test_path_is_missing d2/ut
>  '
>  
> -test_expect_failure 'git clean works if a glob is passed without -d' '
> +test_expect_success 'git clean works if a glob is passed without -d' '
>  	mkdir -p d1 d2 &&
>  	touch d1/ut d2/ut &&
>  	git clean -f "*ut" &&

Nice.

Thanks.