Re: [PATCH 13/13] Build in merge

> 
> >> > +	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.