[FFmpeg-devel] [PATCH] Make parse_options take as argument a parse argument function

[Diego Biurrun]

On Tue, Aug 14, 2007 at 12:27:31AM +0200, Stefano Sabatini wrote:
> On date Monday 2007-08-13 20:10:38 +0200, Diego Biurrun encoded:
> > On Mon, Aug 13, 2007 at 11:06:01AM +0200, Stefano Sabatini wrote:
> > > 
> > > Sorry for insisting, can someone apply this (I have other patches
> > > waiting for it)?
> > 
> > I will in a moment, but please clarify the log message:
> > 
> >   make cmdutils.c:parse_options take as argument a parse bare argument function.
> > 
> > What exactly do you want that function to take as argument?  Is the
> > following still an accurate description of your change?
> > 
> >   Make cmdutils.c:parse_options accept a function to parse bare
> >   parameters as argument.
> 
> Yes, and clearer than my original one (here the confusion derives
> from the term "argument" used both with the meaning of "function
> argument" and of "command line argument"). Maybe still better:
> 
> Make cmdutils.c:parse_options accept as argument a function to parse
> bare command line parameters.

Perfect.

> > > + * @param parse_arg_function name of the function to be called to process an argument without
> > 
> > Name of the function called to process an argument without
> > 
> > > + * a leading option name flag, NULL if such argument don't have to be processed.
> > 
> > flag. NULL if such an argument does not have to be processed.
> 
> I really meant: NULL if such argumentS don't have to be processed.
> 
> This looks clearer to me:
> @param parse_arg_function Name of the function called to process every argument
> without a leading option name flag, NULL if such arguments don't have to
> be processed.
> 
> I prefer "arguments" *plural* to highlight the fact that such bare
> arguments may be more than just one, for exaple as in:
> ffmpeg -i input.mpeg foo.avi bar.mpeg baz.ogg
> 
> Hope it's clearer now.

Yes, thanks.

> > > + */
> > > +void parse_options(int argc, char **argv, const OptionDef *options, void (* parse_arg_function)(const char*));
> > 
> > And please try to keep lines below 80 characters.
> 
> Fixed.

I fixed it some more in the Doxygen comments :-)

Patch applied, thanks.

Diego