<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/104729/">http://git.reviewboard.kde.org/r/104729/</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/104729/diff/1/?file=58555#file58555line419" style="color: black; font-weight: bold; text-decoration: underline;">plugins/textshape/TextTool.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void TextTool::createActions()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">419</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">action</span> <span class="o">=</span> <span class="k">new</span> <span class="n">KAction</span><span class="p">(</span><span class="n">KIcon</span><span class="p">(</span><span class="s">"merge"</span><span class="p">),</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Merge Cells"</span><span class="p">),</span> <span class="k">this</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">419</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">action</span> <span class="o">=</span> <span class="k">new</span> <span class="n">KAction</span><span class="p">(</span><span class="n">KIcon</span><span class="p">(</span><span class="s">"<span class="hl">edit-table-cell-</span>merge"</span><span class="p">),</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Merge Cells"</span><span class="p">),</span> <span class="k">this</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;">well the reason we don't use the "real" names for merge and spli is because they are not in kde 4.3, which is our minimum requirement at the moment.
so it's a no go until the requirement changes</pre>
</div>
<br />
<p>- C.</p>
<br />
<p>On April 26th, 2012, 12:16 a.m., Friedrich W. H. Kossebau 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 Calligra.</div>
<div>By Friedrich W. H. Kossebau.</div>
<p style="color: grey;"><i>Updated April 26, 2012, 12:16 a.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;">I grepped for all "KIcon(" (big mistake, there are quite a lot :) ) and checked if the icon exists in the Oxygen theme, is installed by Calligra itself, or are simply missing. There are more icon names/ids in the code, but finding them would have been even more work. Left for somebody else :)
(ideally icon names should have a marker, so they can be simply grepped, like translated strings are).
I used the version 4.8.2-139.1 of oxygen-icon-theme, as installed from obs://build.opensuse.org/KDE. Which is the oxygen version that Calligra depends on? I could not find a check in the CMakeLists.txt or a hint in the README.PACKAGERS .
So, I found a few typos, a few names refering to non-existing icons, and some icons for which I propose another choice. So the patch is about:
* "media-playback-start" is used in kexi a few times where "system-run" might be better
* "printer" where "document-print" might be better
* "words" and "tables" where "application-vnd.oasis.opendocument.text" and "application-vnd.oasis.opendocument.spreadsheet") might be better
* non-existing "edit_remove", "edit_add", "text_left", "text_center", "text_right", "icons", "chart-bar", "paste", "office-calendar", replaced with what seems a good match
* typo in "format-text-suscript", "video-x-genenric"
* "chart_*" icon names where oxygen has now "office-chart-*", as also used in rest of file. But some "chart_*" are without any match and without any icon, please comment on that
* "insert_table_*" and "delete_table_*" could be replaced with "edit-table-insert-*" and "edit-table-delete-*"
* "merge" and "split" seem to miss "edit-table-cell-" prefix, like in rest of icon names in that file
* "show_sheet_column" also looks like an accidental change that should be "show_table_column" still
* "mail-message" could be "internet-mail" to match other icons in sheets/dialogs/LinkDialog.cpp (and text there could be improved to "Mail Address" and "Email Address")
* "klipper" might be better "edit-paste" there
As far as I could see nothing related to http://community.kde.org/Calligra/Icons . But I found other icons to be missing (at least by the code, did not check all these by running the code):
document-page-setup (KexiProjectListView.cpp, KexiProjectNavigator.cpp, KexiMainWindow.cpp)
fontsizeup, fontsizedown (CellToolBase.cpp)
inspector (CellToolBase.cpp)
clearComment (CellToolBase_p.cpp)
snap-boundingbox (SnapGuideConfigWidget.cpp)
duplicate (braindump/src/View.cpp)
object-locked (braindump/src/import/ToolDocker.cpp, krita/plugins/extensions/imagesize/multilock_button.cc)
object-unlocked (braindump/src/import/ToolDocker.cpp, krita/plugins/extensions/imagesize/multilock_button.cc)
music-clef, music-clef-trebble, music-clef-bass, music-clef-alto (SetClefAction.cpp)
chart_circle_normal, chart_ring_normal, chart_scatter_normal, chart_radar_normal, chart_stock_normal, chart_bubble_normal, chart_surface_normal, chart_gantt_normal (ChartConfigWidget.cpp)
edit-merge, edit-duplicate (krita/plugins/extensions/dockers/defaultdockers/kis_layer_box.cpp)
extended_color_selector (krita/plugins/extensions/dockers/advancedcolorselector/kis_color_selector_settings.cpp)
erase-previous-guides, add-horizontal-edges, add-vertical-edges (plugins/defaultTools/guidestool/InsertGuidesToolOptionWidget.cpp)
krita_paintop_icon (krita/ui/widgets/kis_paintop_presets_popup.cpp)
And I wonder why are krita installs so many icons not with the usual naming pattern (hi-*/ox-*), shouldn't that be changed perhaps?
And why are there all the *-calligra icon names? In other places the non-calligra names are used, e.g. the "align-*" names. Some temporary solution that could be removed completely again? Definitely needs the knowledgde which version of the oxygen icons Calligra depends on.</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>kexi/core/KexiWindow.cpp <span style="color: grey">(f49c967)</span></li>
<li>kexi/formeditor/editlistviewdialog.cpp <span style="color: grey">(f66ceaa)</span></li>
<li>kexi/formeditor/richtextdialog.cpp <span style="color: grey">(80748a5)</span></li>
<li>kexi/main/startup/KexiNewProjectAssistant.cpp <span style="color: grey">(1c52d79)</span></li>
<li>kexi/plugins/reports/kexireportview.cpp <span style="color: grey">(ccdf3f9)</span></li>
<li>kexi/plugins/scripting/kexiscripting/kexiscriptdesignview.cpp <span style="color: grey">(d4d8e2d)</span></li>
<li>kexi/widget/navigator/KexiProjectListView.cpp <span style="color: grey">(3fa561f)</span></li>
<li>kexi/widget/navigator/KexiProjectNavigator.cpp <span style="color: grey">(9c39a1a)</span></li>
<li>kexi/widget/pixmapcollection.cpp <span style="color: grey">(b4f9dfe)</span></li>
<li>plugins/chartshape/ChartConfigWidget.cpp <span style="color: grey">(29af6cb)</span></li>
<li>plugins/chartshape/dialogs/TableEditorDialog.cpp <span style="color: grey">(62acce9)</span></li>
<li>plugins/textshape/TextTool.cpp <span style="color: grey">(1dc2e02)</span></li>
<li>sheets/dialogs/LinkDialog.cpp <span style="color: grey">(4ce4636)</span></li>
<li>sheets/plugins/calendar/CalendarToolWidget.cpp <span style="color: grey">(a7c7814)</span></li>
<li>sheets/shape/SheetsEditor.cpp <span style="color: grey">(5c1ba6b)</span></li>
<li>sheets/ui/CellToolBase.cpp <span style="color: grey">(33f2d29)</span></li>
<li>sheets/ui/CellToolBase_p.cpp <span style="color: grey">(0cba586)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104729/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>