[MPlayer-dev-eng] [PATCH] Use the normal option parser for command line preparsing

[Alban Bedel]

On Sun, 13 Apr 2008 20:33:42 +0400
Andrew Savchenko <Bircoph at list.ru> wrote:

> Hi,
> 
> On Sunday 13 April 2008 18:52, Alban Bedel wrote:
> > --- parser-cfg.c	(revision 26439)
> > +++ parser-cfg.c	(working copy)
> > @@ -245,4 +245,42 @@
> >  	return ret;
> >  }
> >
> > +extern int mp_msg_levels[];
> > +
> > +/// Parse the command line option that must be handled at
> > startup. +int m_config_preparse_command_line(m_config_t *config,
> > int argc, char **argv) +{
> 
> Why this is needed to be implemented as separate function instead 
> of reusing m_config_parse_*_commandline()?
> Perhaps it will be better just to shut up parser errors, set 
> appropriate config mode and run normal 
> m_config_parse_mp_commandline() or m_config_parse_me_commandline() 
> respectively?

The normal command line parser build a tree or list with all the
filenames, that's useless at this point. IMHO it's better to just have
a separate function than adding more hacks to the command line parsers.

> > Index: m_config.c
> > ================================================================
> >=== --- m_config.c  (revision 26439)
> > +++ m_config.c  (working copy)
> > @@ -308,6 +308,13 @@
> >      mp_msg(MSGT_CFGPARSER,
> > MSGL_ERR,MSGTR_InvalidCmdlineOption,arg); return M_OPT_INVALID;
> >    }
> > +  // During pre-parse don't set non-pre-parse options
> > +  // Otherwise don't set pre-parse options
> > +  if(((config->mode == M_COMMAND_LINE_PRE_PARSE) &&
> > +      !(co->opt->flags & M_OPT_PRE_PARSE)) ||
> > +     ((config->mode != M_COMMAND_LINE_PRE_PARSE) &&
> > +      (co->opt->flags & M_OPT_PRE_PARSE)))
> 
> The problem is here. Just imagine that option is marked as 
> CONF_PRE_PARSE and is not specified in the command line, but is 
> specified in the config file. It such case option just will be 
> ignored.
> 
> I suggest to add another flag (e.g. M_OPT_PREPARSE_DONE) and toggle 
> it while preparsing, and only skip option at the main parse if 
> this flag is set.

I see what you mean, I'll do that.

> > +    set = 0;
> 
> Why not exit from m_config_parse_option() immediately?

Because to correctly parse the command line we need to know how many
parameters each option take, and that is returned by the option parser.

	Albeu