[MPlayer-dev-eng] [PATCH] support run command with specific osd_level

[Reimar Döffinger]

Hello,
On Thu, Jan 31, 2008 at 10:56:30PM +0800, Ulion wrote:
> 2008/1/31, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > While I do not exactly object to it, I think you are introducing an
> > inconsistency with this:
> >
> > On Wed, Jan 30, 2008 at 09:51:50PM +0800, Ulion wrote:
> > > @@ -754,6 +755,17 @@
> > >    while (str[0] == ' ' || str[0] == '\t')
> > >      ++str;
> > >
> > > +  if (strncmp(str, "osd_level ", 10) == 0) {
> > > +    str += 10;
> > > +    osd = strtol(str, &str, 10);
> > > +    if (str[0] != ' ' && str[0] != '\t')
> > > +      return NULL;
> > > +    do {
> > > +      ++str;
> > > +    }
> > > +    while (str[0] == ' ' || str[0] == '\t');
> > > +  }
> > > +
> > >    if (strncmp(str, "pausing ", 8) == 0) {
> >
> > Neither for the pausing prefix nor for any other case will the parser
> > accept multiple spaces or even a single tab in-between, only the leading
> > tabs/spaces are stripped (the reason being to allow to write the key ->
> > command mapping in a table-like way).
> > IMHO if allowing more than just a single space as separators within the
> > command is desired, it would make much more sense to implement it
> > consistently for all cases, i.e. after the pausing prefix, before and
> > in-between the arguments etc., but that would be non-trivial.
> 
> yes, I want to let all codes here support both space and tab, but now
> I'm only consider code of current patch, and we can change other parts
> later since it's unrelated changes.

Well, is the fact that it is something unrelated not a good reason not
to do half of the patch now and the other half later?
Especially since this code will almost certainly not stay like this when
you do the other patch, since I certainly would object to a patch that
duplicates the 
>   while (str[0] == ' ' || str[0] == '\t')
>      ++str;
or equivalent 5 times (a guesstimate of how often this would be needed).