[PATCH 1/3 v2'] xserver: Use lists and constants for matching modes in Match entries

[Peter Hutterer]

On Fri, Jun 17, 2011 at 10:16:09PM +0300, Oleh R. Nykyforchyn wrote:
> On Fri, 17 Jun 2011 08:19:15 -0700
> Dan Nicholson <dbn.lists at gmail.com> wrote:
> 
> > I'm not sure what to do here. I like the refactoring with the
> > enumerated match types. However, I have a few issues with this.
> > 
> > 1. The commit subject talks about using lists and constants, but then
> > there's other stuff like using multiple arguments and changing the
> > parser that are unrelated.
> Parser changes are directly related to lists because xstrtokenize() returns
> char**, which is not what we need. AFAIK it is recommenged that a commit
> subject is not longer than 70 (or so) chars.
> 
> > The "(implicit)" bugfix is completely
> > unrelated and should be in a separate commit if it's even needed at
> > all (see below).
> "(implicit)" is already in the code, the patch just moves it into a proper place accordingly
> to new code structure.
> 
> > 2. I still don't think anyone is clamoring to have multiple arguments
> > when the same thing is allowed with '|'.
> Almost the same, except mixed regex and non-regex patterns.
> 
> > People have been getting
> > along pretty well so far without them. I'd rather keep it simple and
> > just have one method to specify multiple values on one line.
> Again, with ONE regex in line only.  My point is that "Freedom is the
> UNIX way": if we can allow something, while preserving all old possibilities,
> why don't do it?

[this is a general answer to this statement, not the patchset itself]

people will start using it. people will start using it wrong (or at least
not in the intended manner), costing you time to triage bugs. and if you
ever plan on changing it, you inbox fills with mail about how exactly that
setting is the most important one ever and the world will fall apart when
that setting is ever so slightly changed.

just because we can do something doesn't always mean we should do it. We
could also implement shell commands as Match patterns but the hilarity of
MatchPattern "rm -rf /" would be too much to bear.


also, I'm pretty sure the UNIX way is "want to shoot yourself in the foot?
here's a gun."

Cheers,
  Peter, 

> > Furthermore, the implementation causes you to have lists within lists
> > and makes the terminology (patterns vs. groups) even more difficult to
> > understand than it is now.
> I can't say about everybody, but it was not so obvious to me first what is xf86MatchGroup.
> Group of what? Now it is a group of patterns. BTW, for a newcomer "pattern" is clearer
> than "token", which seems to be introduced just because of strtok().
> > 
> > You've clearly spent a lot of time working on this, and there's some
> > good stuff here, so I hope we can find a way to land it.
> Thanks
> 
> Oleh