[PATCH xfree86] Signed-off-by: Oleh Nykyforchyn <oleh.nyk at gmail.com>

Dan Nicholson dbn.lists at gmail.com
Tue May 24 05:54:33 PDT 2011


On Mon, May 23, 2011 at 7:26 AM, Oleh Nykyforchyn <oleh.nyk at gmail.com> wrote:
> On Mon, 23 May 2011 05:54:55 -0700
> Dan Nicholson <dbn.lists at gmail.com> wrote:
>
>>
>> The whole reason why the current setup is weird is that it's
>> completely implicit what type of matching happens in each statement.
>> MatchProduct is string comparison (case sensitive) while
>> MatchDevicePath is a path pattern.
>
> I agree that this is a problem. Nobody who has not read the code can not know
> that for Product and Vendor we look for a substring, for DevicePath, PnPID
> and USBID a path pattern is matched etc, although these choices probably
> are optimal. My suggestion is the following:
>
> 1) preserve the existing behaviour, but clarify it in docs, including man page;

The man page does say what type of matching each uses, but it's still
not that discoverable.

>>In your suggestion, how is the
>> regex specified? If you say another sentinel character, how is that
>> less complex than adding an argument to specify what the other string
>> means? We're again making up our own syntax.
>
> 2) [already posted, but repeated]
> BTW, if we adopt two modes: simple |-separated sequence and regex, we don't
> even need an optional argument. Consider e.g. a string of the form "|regex|".
> It is useless in the simple mode, hence regex is recognized and two |-s are dropped.
> Syntax for regex inside, e.g., can be compatible with regex library, so we do not
> invent anything. People who do not use new regex syntax simply do not notice anything.
>
> Of course, there is also nothing bad with optional "regex" keyword,
> I am just not sure that "strcmp", "strstr" etc are necessary. BTW, somebody can also
> want to mix "strstr" and "strcmp" in one pattern sequence, then an optional argument
> also becomes a sequence, and so on.
>
> What do You think about such an approach?

Frankly, I'm not crazy about it for two reasons. First, it's another
magic sequence to parse out and handle and attempt to explain to
people. The |regex| syntax is something new that no one would know
about. The only regex syntax I can think of that applies and people
are familiar with is awk's /regex/, and the /s would conflict with
other valid uses of the argument. Second, what if I as a user have the
reasonable thought that I want to have multiple regexs to match
against like the non-regex matching? MatchProduct "foo|bar" works, but
why not MatchProduct "|foo|bar|"? Not to mention that you can
currently have "|foo|" and the code will just treat it as a single
normal argument (run that through strtok with '|' as the delimiter).

To put it in programming terms, which of the following two APIs would
you want to use?

1. Single overloaded argument which changes behavior based on certain values

2. Optional second argument that specifies the desired behavior

I'm pretty sure I'm going with #2. I can't see a reason why "put in
magic characters to get a regex" is better than "add an argument
specifying a regex". What am I missing?

--
Dan


More information about the xorg-devel mailing list