Re: [PATCH 13/13] Build in merge

On Sun, Jun 29, 2008 at 10:40:53PM -0700, Junio C Hamano [off-list ref] wrote:

quoted

Actually reset_hard does not return if an error occures:

I know that; didn't I already say "Luckily no"?  The point was it was not
apparent from the above 6 lines alone.

Ah, OK.

quoted

quoted

quoted

+	for (i = 0; i < use_strategies.nr; i++) {
+		if ((unsigned int)use_strategies.items[i].util &
+			NO_FAST_FORWARD)
+			allow_fast_forward = 0;
+		if ((unsigned int)use_strategies.items[i].util & NO_TRIVIAL)
+			allow_trivial = 0;

Can we abstract out these ugly casts?  Any code that use path_list to
store anything but list of paths (i.e. some value keyed with string) tends
to have this readability issue.

If you don't cast, you can't use the & operator. If I change the
path_list_item's util to be an unsigned number then I break fast-export.
I think if we _really_ want to get rid of those casts, we could have
something like:

No, no, no.  That is not what I meant.

The places that use use_strategies in your code knows too much about the
internal implementation detail of path_list, while path_list pretends to
be a general purpose "table keyed with string" facility.  The fact is that
the table is not a very useful general purpose abstraction unless you are
pointing at some structures that exist regardless of your use of path_list
(e.g. you have some "struct object" and you hold pointers in a path_list).
It does not work very well as an abstraction for use case like yours.

With something like:

        static inline unsigned nth_strategy_flags(struct path_list *s, int nth)
        {
                return (unsigned) s->items[nth].util;
        }

the checks would be more like:

	if (nth_strategy_flags(&use_strategies, i) & NO_FAST_FORWARD)
		...

or even:

        static inline check_nth_strategy_flags(struct path_list_item *i, unsigned flags)
        {
        	return !((unsigned) i->util & flags);
        }

        if (check_nth_strategy_flags(&use_strategies,items[i], NO_FAST_FORWARD)
        	...	

either of which would be much easier on the eye.

Probably this is subjective, but I think the previous form is easier to
read, so I choose that one.