trinity-devel@lists.pearsoncomputing.net

Message: previous - next
Month: February 2012

Re: Re: [trinity-devel] twin modifications

From: Martin Gräßlin <mgraesslin@...>
Date: Mon, 13 Feb 2012 20:16:18 +0100
On Monday 13 February 2012 12:54:08 Timothy Pearson wrote:
> > On Monday 13 February 2012 10:59:10 Timothy Pearson wrote:
> >> I have repeatedly asked him for
> >> the technical reasons that he considers twin changes to have "broken"
> >> it,
> >> and I still do not have an answer.
> > 
> > I pointed out to two incorrect commits in my very first mail to this
> > mailing
> > list. I have offered several times the help, I have told you that you can
> > ask
> > me any question about KWin. And now you complain that I never explained to
> > you
> > why the commits have broken KWin? Seriously you had enough time to ask,
> > and I
> > had expected that you would ask why it is wrong.
> > 
> > Cheers
> > Martin
> 
> Care to elaborate?  I am willing to listen.  Your original message was
> disregarded to some extent as you linked to changes that were made on
> purpose, and I usually expect people who claim something is wrong to make
> an attempt at stating *why( they think said something is wrong.
yes I know they are made on purpose, nevertheless they are wrong. It is quite 
common that developers not knowing a codebase do things incorrectly. I will 
now only elaborate on the two commits I outlined. In fact all commits I have 
seen so far would not pass a review request for KWin and as I mentioned there 
is at least one commit with the potential to prevent KWin from starting at 
all.

Let's start with 1f40ada: you modify the inline getter for keepAbove. This is 
not how KWin internally works to have window being as keep above. The proper 
method to go through is Client::setKeepAbove() which would also tell other 
interested parties that the window is in fact kept above. This method is quite 
important to use as it also takes care of putting the window into the correct 
layer of the stacking order. I think you solved that by hacking the stacking 
order.

The simplest way to achieve what you actually wanted would have been to make 
your "modal system notification" an override redirect window.

The second commit I pointed out was 9cc1e2c1: I think others already commented 
in my blog comments why this one is rather bad from a users point of view 
(introducing new config options without removing the obsoleted ones). But well 
the main issue from my point of view is that it modified an enum in a public 
header by not appending to the end, but in the middle. I think you can imagine 
what happens to 3rd party offerings compiled against the previous version.

Cheers
Martin

Attachments: