[MPlayer-dev-eng] [PATCH] -noconfig option

[Alban Bedel]

On Fri, 11 Apr 2008 18:08:37 +0400
Andrew Savchenko <Bircoph at list.ru> wrote:

> Hello,
> 
> The first patch adds functionality described above. Note that 
> option must be checked before configs parsing in the same way as 
> -really-quiet.

Good idea.

>  int m_config_parse_config_file(m_config_t* config, char *conffile)
>  {
> +        if (noconfig)
> +                return 1;

I don't like that very much. Having a global simply disable a function
is ugly. Better check before looking for config files. It's a bit more
work but you could also allow finer disabling (to only use the user's
config file for ex.).

> --- mplayer_base/mp_msg.h	2008-03-08 02:46:16.000000000 +0300
> +++ mplayer/mp_msg.h	2008-04-11 17:30:24.000000000 +0400
> @@ -3,6 +3,7 @@
>  
>  // defined in mplayer.c and mencoder.c
>  extern int verbose;
> +extern int noconfig;

This has nothing to do in mp_msg.h.

> The second patch is just cosmetics required after first patch 
> applying.

No need to split that lonely blank line. Added code is allowed to
contain black lines :)

> Anyone can figure out that there is some code duplication in 
> mplayer.c and mencoder.c. I dislike code duplication, so the third 
> patch is intended to fix this by moving common code (related to 
> the first patch) from these files to mpcommon.[ch]. This patch 
> requires the first patch to be applied.

> +void preparse_command_line(int argc, char* argv[])
> +{
> +    int i;
> +  
> +    for(i=1; i<argc; i++) {
> +        if(!strcmp(argv[i], "-really-quiet"))
> +            verbose= -10;
> +        if(!strcmp(argv[i], "-noconfig"))
> +            noconfig= 1;
> +    }
> +}

With respect to this patch, this is fine. But it is quiet a hack
that will catch stuff which would not normally be treated as command
line options. Ideally this should latter be replaced by something
robuster.

	Albeu