<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/105515/">http://git.reviewboard.kde.org/r/105515/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Nicely done, thanks! Just few small nitpicks below..</pre>
<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/105515/diff/2/?file=72065#file72065line118" style="color: black; font-weight: bold; text-decoration: underline;">KTp/Widgets/add-contact-dialog.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; ">bool KTp::SubscribableAccountsModel::filterAcceptsRow(int source_row, const QModelIndex &source_parent) const</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">bool SubscribableAccountsModel::filterAcceptsRow(int source_row, const QModelIndex &source_parent) const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">100</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">AccountsModelItem</span><span class="o">*</span> <span class="n">item</span> <span class="o">=</span> <span class="n">itemData</span><span class="p">.</span><span class="n">value</span><span class="o"><</span><span class="n">AccountsModelItem</span><span class="o">*></span><span class="p">();</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">113</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">AccountsModelItem</span><span class="o">*</span> <span class="n">item</span> <span class="o">=</span> <span class="n">itemData</span><span class="p">.</span><span class="n">value</span><span class="o"><</span><span class="n">AccountsModelItem</span><span class="o">*></span><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;">Star aligned to the right</pre>
</div>
<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/105515/diff/2/?file=72065#file72065line127" style="color: black; font-weight: bold; text-decoration: underline;">KTp/Widgets/add-contact-dialog.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; ">bool KTp::SubscribableAccountsModel::filterAcceptsRow(int source_row, const QModelIndex &source_parent) const</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">bool SubscribableAccountsModel::filterAcceptsRow(int source_row, const QModelIndex &source_parent) const</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">122</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KMessageBox</span><span class="o">::</span><span class="n">sorry</span><span class="p">(</span><span class="k">this</span><span class="p">,</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"The requested account has disconnected "</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;">"...has been disconnected..." I'd say. Native speakers?</pre>
</div>
<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/105515/diff/2/?file=72065#file72065line156" style="color: black; font-weight: bold; text-decoration: underline;">KTp/Widgets/add-contact-dialog.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; ">void AddContactDialog::_k_onContactsForIdentifiersFinished(Tp::PendingOperation *op)</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">151</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KMessageBox</span><span class="o">::</span><span class="n">sorry</span><span class="p">(</span><span class="k">this</span><span class="p">,</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Failed to construct a contact with the given name."</span><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;">This sounds very techie. Would "Failed to create new contact" work?</pre>
</div>
<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/105515/diff/2/?file=72065#file72065line173" style="color: black; font-weight: bold; text-decoration: underline;">KTp/Widgets/add-contact-dialog.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; ">void AddContactDialog::_k_onContactsForIdentifiersFinished(Tp::PendingOperation *op)</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">168</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KMessageBox</span><span class="o">::</span><span class="n">sorry</span><span class="p">(</span><span class="k">this</span><span class="p">,</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Failed to request presence subscription "</span></pre></td>
</tr>
<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">169</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="s">"from the requested contact."</span><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;">Imho too technical as well. "Failed to retrieve contact's presence/status"? Although that's not entirely correct, but do we need to scare users with terms like "presence subscription"?</pre>
</div>
<br />
<p>- Martin</p>
<br />
<p>On July 11th, 2012, 2:35 p.m., George Kiagiadakis 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 Telepathy.</div>
<div>By George Kiagiadakis.</div>
<p style="color: grey;"><i>Updated July 11, 2012, 2:35 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;">Being annoyed by the fact that the presence applet shows the AddContactDialog as a modal dialog (making plasma unusable in the meantime), I decided to move the contact subscription logic inside the dialog and therefore eliminate code duplication between the applet and the CL as well as make it easy to use the dialog in a non-modal way.
This patch also adds the KTP_NO_EXPORT macro, which we should start using to hide private symbols and prepare the library for frameworks.
Shortlog:
George Kiagiadakis (9):
AddContactDialog: Move function implementations inside the KTp namespace
ktp-export.h: Add KTP_NO_EXPORT macro
AddContactDialog: Move the contact creation logic from the presence applet & CL inside the dialog
AddContactDialog: Remove public accessors for account and screen name
AddContactsDialog: Use QComboBox::setModel() instead of manually populating it with entries from the model
AddContactDialog: Error out if the user did not specify a contact name
AddContactDialog: kDebug++
AddContactDialog: Use KMessageBox::sorry() instead of error() and enhance the strings
AddContactDialog: Attribute copyright to myself</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;">The dialog still seems to work as expected</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>KTp/Widgets/add-contact-dialog.h <span style="color: grey">(cc65aa3f6a94884a83bd058e876cecc5e996fb85)</span></li>
<li>KTp/Widgets/add-contact-dialog.cpp <span style="color: grey">(dfbd9aac870a4d82953820fa4feb28e3af9d7f3d)</span></li>
<li>KTp/ktp-export.h <span style="color: grey">(909c3ca9f1b8ddaed28c0fc15b2537da2a843760)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105515/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>