<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/109268/">http://git.reviewboard.kde.org/r/109268/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 3rd, 2013, 9:09 p.m. UTC, <b>Lasath Fernando</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/109268/diff/1/?file=116772#file116772line42" style="color: black; font-weight: bold; text-decoration: underline;">lib/text-chat-config.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<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">42</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">instance</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think you should probably put a mutex lock around this.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Wouldn't I then have to use a mutex in all of the methods? I think the accesses to the members of TextChatConfig are not guaranteed to be atomically.
If it is deemed necessary I would then add a class variable of type QMutex (using QMutex::Recursive) to TextChatConfig and use that in all methods.</pre>
<br />
<p>- Stefan</p>
<br />
<p>On March 3rd, 2013, 8:44 p.m. UTC, Stefan Eggers wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Telepathy.</div>
<div>By Stefan Eggers.</div>
<p style="color: grey;"><i>Updated March 3, 2013, 8:44 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;">Combining settings in a singleton class so that there is no confusion about the current settings.
Up to now the settings for where new chats open were loaded by
ktp-text-ui on startup and not re-read anymore. Changing the settings
in the config dialog had no immediate effect; only after the next
start of ktp-text-ui it finally changed.
Now whenever the settings get asked for the singleton provides the
current value. Thus every change to the settings via the config dialog
will take effect immediately.
Also gives a way to easily add new settings in one place. Will be
useful for implementing the additional settings required by bug 282201
(make it configurable if "user is typing" gets show) which needs two
more bools.</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;">1. Starting with "Open new conversations:" set to "As tabs in the same window".
2. Open a chat with user A.
3. Open a chat with user B.
4. Observe that it opens in a new tab.
5. Change setting to "As new windows".
6. Close chat with user B.
7. Open a chat with user B.
8. Observe that it opens in a new window.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=316004">316004</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>config/behavior-config.h <span style="color: grey">(562187ee308182f70b66b09ecf489a13fbfb6fdd)</span></li>
<li>app/telepathy-chat-ui.cpp <span style="color: grey">(920210891e58166d1c14f5431a913a8e5f912de2)</span></li>
<li>app/telepathy-chat-ui.h <span style="color: grey">(70800f42a4dd8f5627495661a2f9c71535f20763)</span></li>
<li>config/behavior-config.cpp <span style="color: grey">(214ee3e5641185d200c9aca2892aee78e9915e5b)</span></li>
<li>lib/CMakeLists.txt <span style="color: grey">(4e03671dcdaee338aca57dae12512b75d7f0be15)</span></li>
<li>lib/KTp/TextChatConfig <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/text-chat-config.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/text-chat-config.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109268/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>