[PATCH 2/4] Rename region macros to mixed case and remove screen argument

Jamey Sharp jamey at minilop.net
Fri May 21 15:57:47 PDT 2010


On Fri, May 21, 2010 at 3:32 PM, Keith Packard <keithp at keithp.com> wrote:
> On Fri, 21 May 2010 15:12:47 -0700, Jamey Sharp <jamey at minilop.net> wrote:
>> I get different results from running fix-region than are reflected in
>> this patch. I think the script is giving the right output, and the
>> patch is wrong in places.
>
> I think that's fixed in the new push I just made; are you looking at
> that one?

I wasn't, and it is indeed fixed.

>> It's kind of confusing having some functions, like RegionInit, get
>> disabled with #ifndef in this patch. I know the next commit cleans
>> that up, but since you don't use the code from the function
>> implementations at all in that later patch, can I suggest just
>> deleting them in this one?
>
> I think having this patch be shorter makes it easier to see what
> happened?

I have a lot of sympathy for the "smaller diff" argument, but if it
were me, I'd still delete them...

>> This hardly should block merge, but would you consider fixing the `git
>> diff --check` whitespace warnings in this patch? None of them were
>> introduced here (except in the sed script) but it'd be nice to fix
>> them as long as you're touching these lines anyway.
>
> Sigh. Is there an easy way to do this that doesn't touch every line in
> the server?

This seems to work:

git diff --check |
sed -n 's!^\([^:]*\):\([^:]*\):.*!sed -i "\2 s/[ \t]*$//; \2 s/
*\t/\t/g" \1!p' |
sh

I'm inordinately pleased at using sed to generate sed scripts.

Jamey


More information about the xorg-devel mailing list