[prev in list] [next in list] [prev in thread] [next in thread]
List: kwin
Subject: Re: Review Request: Placement as singleton
From: Thomas_Lübking <thomas.luebking () web ! de>
Date: 2012-11-22 19:15:41
Message-ID: 20121122191541.5876.78830 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107416/#review22390
-----------------------------------------------------------
kwin/placement.cpp
<http://git.reviewboard.kde.org/r/107416/#comment17169>
Q_ASSERT(!s_self); or even "if (s_self) return;"
- Thomas L=C3=BCbking
On Nov. 22, 2012, 1 p.m., Martin Gr=C3=A4=C3=9Flin wrote:
> =
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107416/
> -----------------------------------------------------------
> =
> (Updated Nov. 22, 2012, 1 p.m.)
> =
> =
> Review request for kwin.
> =
> =
> Description
> -------
> =
> Remove Placement* member variable from Workspace
> =
> Not needed anymore given that Placement became a singleton.
> =
> Remove Placement wrappers from Workspace
> =
> The two methods:
> * place
> * placeSmart
> have only forwarded the call to the Placement object. Now that Placement
> is a singleton there is no need to have them. Every user can call them
> directly without going over Workspace.
> =
> Move cascadeDesktop and unclutterDesktop to Placement
> =
> It is more Placement related and does not really fit into geometry given
> that it only calls methods on Placement. It probably only was inside
> Workspace due to being part of the DBus interface. The DBus methods are
> used by external components so it needs to stay.
> =
> The DBus Wrapper is now calling the methods on the singleton Placement
> directly, so no need in Workspace anymore.
> =
> Make Placement a Singleton
> =
> No real change as there has only been one Placement instance inside
> Workspace anyway.
> =
> =
> Diffs
> -----
> =
> kwin/dbusinterface.cpp 6d64a9fc6acd60cba212454586a30a8e34ff296e =
> kwin/geometry.cpp ef63cae95841c5c80d2a00430f79812d6dc95160 =
> kwin/manage.cpp 43df1b9e78d36833a6c2812454f49a25a371060c =
> kwin/placement.h 0fe894ef77ea95b88133b2911285a04cf7a0175e =
> kwin/placement.cpp d5ad358016dd3bfbd64ae9b9a13a952cc357b2fe =
> kwin/workspace.h 708e23333dbce69bc4cf7e808cf752ceb701d8d1 =
> kwin/workspace.cpp f80649da26374ef07022190abf0e81bcf34a4b77 =
> =
> Diff: http://git.reviewboard.kde.org/r/107416/diff/
> =
> =
> Testing
> -------
> =
> it compiles :-)
> =
> =
> Thanks,
> =
> Martin Gr=C3=A4=C3=9Flin
> =
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/107416/">http://git.reviewboard.kde.org/r/107416/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/107416/diff/2/?file=95837#file95837line45" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kwin/placement.cpp</a> <span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
"></pre></td> <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: \
0; ">Placement *Placement::create(Workspace *ws)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">45</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="p">{</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Q_ASSERT(!s_self); or even "if (s_self) return;"</pre> </div>
<br />
<p>- Thomas</p>
<br />
<p>On November 22nd, 2012, 1 p.m., Martin Gräßlin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for kwin.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Nov. 22, 2012, 1 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Remove Placement* member variable from Workspace
Not needed anymore given that Placement became a singleton.
Remove Placement wrappers from Workspace
The two methods:
* place
* placeSmart
have only forwarded the call to the Placement object. Now that Placement
is a singleton there is no need to have them. Every user can call them
directly without going over Workspace.
Move cascadeDesktop and unclutterDesktop to Placement
It is more Placement related and does not really fit into geometry given
that it only calls methods on Placement. It probably only was inside
Workspace due to being part of the DBus interface. The DBus methods are
used by external components so it needs to stay.
The DBus Wrapper is now calling the methods on the singleton Placement
directly, so no need in Workspace anymore.
Make Placement a Singleton
No real change as there has only been one Placement instance inside
Workspace anyway.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">it compiles :-)</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kwin/dbusinterface.cpp <span style="color: \
grey">(6d64a9fc6acd60cba212454586a30a8e34ff296e)</span></li>
<li>kwin/geometry.cpp <span style="color: \
grey">(ef63cae95841c5c80d2a00430f79812d6dc95160)</span></li>
<li>kwin/manage.cpp <span style="color: \
grey">(43df1b9e78d36833a6c2812454f49a25a371060c)</span></li>
<li>kwin/placement.h <span style="color: \
grey">(0fe894ef77ea95b88133b2911285a04cf7a0175e)</span></li>
<li>kwin/placement.cpp <span style="color: \
grey">(d5ad358016dd3bfbd64ae9b9a13a952cc357b2fe)</span></li>
<li>kwin/workspace.h <span style="color: \
grey">(708e23333dbce69bc4cf7e808cf752ceb701d8d1)</span></li>
<li>kwin/workspace.cpp <span style="color: \
grey">(f80649da26374ef07022190abf0e81bcf34a4b77)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107416/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
kwin mailing list
kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic