Re: [PATCH 3/4] tree-walk: actually move tree_entry_interesting() to tree-walk.c

[Nguyen Thai Ngoc Duy <pclouds@gmail.com>]

2010/9/7 Elijah Newren <newren@gmail.com>:
> 2010/9/5 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>> This function can be potentially used in more places than just
>> tree-diff.c. This patches removes struct diff_options dependency from
>> the function, and moves it to tree-walk.c.
>>
>> No functionality change intended.
>
> Thanks for working on this.  I like having the declaration of
> tree_entry_interesting() moved to tree_walk.h, at the very least.  The
> change to make tree_entry_interesting() take an entry instead of a
> tree_desc makes sense too.
>
> I'm unsure about replacing the diff_options with paths + pathlens +
> nr_paths -- that might be exposing too much implementation detail in
> the API.  In particular, I'm worried that if we try to add support for
> negated pathspecs or globs or regexes to tree_entry_interesting(),
> then we'll need to pass different data to this function and update an
> awful lot of callers.
>
> Perhaps we should make a new struct containing paths + pathlens +
> nr_paths, make tree_entry_interesting() take such a struct, modify
> diff_options have such a struct instead of the current three paths,
> pathlens, and nr_paths fields, and modify diff_tree_setup_path()s to
> take such a struct instead of a diff_options* (and perhaps move
> diff_tree_setup_paths() out of diff.h and tree-diff.c into some other
> file(s)?).
>
> Thoughts?

You're right again. Perhaps something like struct exclude_list from dir.h?

struct pathspec_list {
	int nr;
	int alloc;
	struct pathspec {
		const char *path;
		int pathlen;
		int flags;
	} **paths;
};

Hmm.. or just use struct exclude_list, with diff_options.path{s,len}
becoming exclude_list.base{,len}. exclude_list.pattern can be used for
glob/regex matching (if we are ever going to support that).
-- 
Duy