On Mon, Sep 24, 2018 at 4:21 PM Taylor Blau <m...@ttaylorr.com> wrote:
> > @@ -602,6 +605,7 @@ int cmd_config(int argc, const char **argv, const char 
> > *prefix)
> >                            PARSE_OPT_STOP_AT_NON_OPTION);
> >
> >       if (use_global_config + use_system_config + use_local_config +
> > +         use_worktree_config +
> >           !!given_config_source.file + !!given_config_source.blob > 1) {
>
> I feel like we're getting into "let's extract a function" territory,
> here, since this line is growing in width. Perhaps:
>
>   static int config_files_count()
>   {
>     return use_global_config + use_system_config + use_local_config +
>                 use_worktree_config +
>                 !!given_config_source.file +
>                 !!given_config_source.blob;
>   }
>
> Simplifying the call to:

I think there's a bigger problem here because we have no good way to
describe that a bunch of options are mutually exclusive. We get by ok
if the number of options is two, but when it gets bigger (and this is
not the only place), it gets messier. Besides the long "if" condition,
I consider the error message "only one config file at a time"
inadequate. We should tell the user what exact options are
conflicting.

So, I'm going to bury my head in the sand this time and ignore the
problem, including adding config_files_count(). It could be a
interesting mini project if someone wants to jump in. Meanwhile it'll
go to the very bottom of my long long backlog.
-- 
Duy

Reply via email to