On Fri, 17 Jun 2011 08:19:15 -0700 Dan Nicholson <[email protected]> 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? > 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 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
