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

2010/9/5 Nguyễn Thái Ngọc Duy [off-list ref]:

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?

Elijah