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

Keith Packard keithp at keithp.com
Fri May 21 15:32:23 PDT 2010


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?

> - doc/xml/Xserver-spec.xml incorrectly dropped the pScreen argument
> from BitmapToRegion.
> - The patch misses some REGION_BREAK uses.
> - Most of the RegionDestroy calls wound up with a space after the open-paren.

Ok, these are all fixed in the new push I just did.


> 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?

> 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?

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20100521/69f5ad08/attachment.pgp>


More information about the xorg-devel mailing list