Re: [PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair

Duy Nguyen [off-list ref] writes:

On Thu, Jan 10, 2013 at 2:26 AM, Junio C Hamano [off-list ref] wrote:

quoted

Martin von Zweigbergk [off-list ref] writes:

quoted

We use the path arguments in two places in reset.c: in
interactive_reset() and read_from_tree(). Both of these call
get_pathspec(), so we pass the (prefix, arv) pair to both
functions. Move the call to get_pathspec() out of these methods, for
two reasons: 1) One argument is simpler than two. 2) It lets us use
the (arguably clearer) "if (pathspec)" in place of "if (i < argc)".
---
If I understand correctly, this should be rebased on top of
nd/parse-pathspec. Please let me know.

Yeah, this will conflict with the get_pathspec-to-parse_pathspec
conversion Duy has been working on.

Or I could hold off nd/parse-pathspec if this series has a better
chance of graduation first. Decision?

I am greedy and want to have both ;-)

Before deciding that, I'd appreciate a second set of eyes giving
Martin's series an independent review, to see if it is going in the
right direction.  I think I didn't spot anything questionable in it
myself, but second opinion always helps.

There is no textual conflict between the two topics at the moment,
but because the ultimate goal of your series is to remove all uses
of the pathspec.raw[] field outside the implementation of pathspec
matching, it might help to rename the field to _private_raw (or
remove it), and either make get_pathspec() private or disappear, to
ensure that the compiler will help us catching semantic conflicts
with new users of it at a late stage of your series.

On Fri, Jan 11, 2013 at 6:09 AM, Junio C Hamano [off-list ref] wrote:

quoted

Or I could hold off nd/parse-pathspec if this series has a better
chance of graduation first. Decision?

I am greedy and want to have both ;-)

Apparently I have no problems with your being greedy.

There is no textual conflict between the two topics at the moment,
but because the ultimate goal of your series is to remove all uses
of the pathspec.raw[] field outside the implementation of pathspec
matching, it might help to rename the field to _private_raw (or
remove it), and either make get_pathspec() private or disappear, to
ensure that the compiler will help us catching semantic conflicts
with new users of it at a late stage of your series.

There are still some uses for get_pathspec() and new call sites won't
cause big problems because they would need init_pathspec() to convert
get_pathspec() results to struct pathspec. I will rename raw[] though.
-- 
Duy