[MPlayer-dev-eng] [PATCH] suboption escaping

[Reimar Döffinger]

Hi,
On Thu, Aug 18, 2005 at 04:21:21PM -0500, Joey Parrish wrote:
> > I think it is yet another, because it does not work exactly like any of
> > the other methods, e.g. it doesn't support escaping via "" (like the
> > normal option parser),
> 
> I don't believe this method via "" exists.  Try this:
> $ mencoder -info 'artist="Something: Something else"'
> 
> You'll get:
>   Option info: Unknown suboption  Something else"

Not for suboptions, but for options (at least in the config file from
what I remember, but I hae to admit I rarely need it).

> > and also it won't work (from what I can tell) for
> > specifying an option ending in \ (important for e.g. windows) - unless
> > you make it the last option and add two \.
> 
> A simple change to make.

Sure, just an example. The problem I have with it is that it looks to me
like a "brute-force" hack specifically targeted at the ':' character without
fixing the problem in general so that specifying anything would be
possible.

> > I also think that your patch makes it impossible to specify an option
> > value that contains a '='?
> 
> Was that not already the case?  How do you propose to change the sscanf
> format?

I think currently the first = delimits the option name, in the value you
can have as many '=' as you want.
And I actually wonder why sscanf is use here, it seems a bit overkill to
me (though it is really short).

> > I am also unhappy about the fact that so many place must be changed, but
> > that probably isn't your patches fault :-(.
> 
> It's not so many places!  The second version encapsulates it all in our
> own strtok.  Outside of that function, we change _3_ lines of code.  The
> sscanf format, and the strtok calls.

Sorry, somehow though that this was one patch, where it was actually two
different ways to fix...

> > I'd like this to be fixed, but IMHO this way is even more a hack than my
> > %len% method is (and I am still not entirely happy about that one, it is
> > only really useful for GUIs).
> 
> I disagree strongly, because escaping using a backslash is a very common
> and expected Unix way to do things.  %len% is something I had never
> heard of before I saw it proposed on this list.

Yes, but the backslash escaping behaves in a certain way that your patch
does not conform to (e.g. \\ is supposed to stand for \ etc.).
Also I'd like to state my dislike of backslash escaping, since it is not
possible to do without modifying the string and possibly lots of
memmoves.
My %len% approach as I said is not really good, its only advantage is
that it is extremly simple to implement in a program.
I prefer "" escaping, though it has the disadvantage that it is more
difficult to protect from the shell and tab completion usually doesn't
work.
IMHO the current way to parse it is broken (first strtok and then
sscanf). I think it would be better to first assign the subopt variable
(by finding the end of the option string by searching for '=' or ':'),
and then parse the value (if any). e.g. if it starts with " search for
the closing " and copy all of it. if not you can do backslash escaping
processing. Etc.

Greetings,
Reimar Döffinger