<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/105284/">http://git.reviewboard.kde.org/r/105284/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 18th, 2012, 7:51 a.m., <b>C. Boemann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">fyi -calligra at the end of icon names are most foten ignored in the naming scheme. If the icon engine cannot find icons with -calligra it removes that part and finds a more general icon.
However what this allows us is to have specialized calligra icons if we wish to. So not saying you change is wron, (we may not want the possibility of a specialized icon), but it's not wrong to have -calligra either
And in reply to jaraslaw: rather than renaming the icon we request we should rename the icon file to follow the fdo standard.</pre>
</blockquote>
<p>On June 18th, 2012, 11:25 p.m., <b>Friedrich W. H. Kossebau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">yes, had the "-postfix"-stripping-until-found systematic of the icon theme in mind when doing this.
But not another thing I might have just found out:
icons from locations added to the icon dirs to use only with KIconLoader::global()->addAppDir("calligra") seem to be checked only after those in the default locations. So whatever is installed into ${DATA_INSTALL_DIR}/calligra/icons will only be loaded if there is nothing in the default location.
Just check what is currently (also without this patch) displayed for the arrow icons set for the arrow shapes: go to the "Add Shape" docker and select the "Arrows" section. You will see the diamond-like arrow icons of at least the 4.8 theme.
I will update the patch with the arrow icons being renamed to the pattern mimetype-x-shape-arrow-{up,down,left,right}, following the pattern of the other pathshape icons. That should be the proper fix here.</pre>
</blockquote>
<p>On June 19th, 2012, 8:43 a.m., <b>C. Boemann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">good catch yes indeed.
The reason why it doesn't find the icons could also be that the icon engine doesn't use icons it has to scale up (or is it down). Anyway that may be why it doesn't use them.
Btw oxygen uses names like draw-arrow-down but also draw-polyline (it's for tools but might be worth considering even so)
For font size up and down the correct names are: format-font-size-more and format-font-size-less</pre>
</blockquote>
<p>On June 19th, 2012, 12:11 p.m., <b>Friedrich W. H. Kossebau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ah, missed that draw-arrow-* is also part of oxygen icons 4.3, yes, also a possible choice (besides -forward and -backward not perfect icon naming :) ).
Rewgarding "font size up and down": those actions are about all-to-lower-case and all-to-upper-case, so not really about the font size, that is why I would not use any currently existing icon here.
And understanding more and more of the KIcon* code I see it's even worse with regard to icon look-up and KIconLoader::global()->addAppDir("calligra"): it is not only that this directory is tried last, the "-postfix"-stripping-until-found is done on each directory (set). So if looking up an icon "first-second" and the "normal" directories contain an icon "first" and the "calligra" directory (set) contains an icon "first-second-third" you will get the icon "first".
Seems that the idea to use a shared central icon pool in the "calligra" directory is slightly flawed with the current KIcon* code :/
A workaround might be to install all shared Calligra icons directly to the Oxygen icons directories, namespacing the icon names with a "-calligra" postfix. Another one would be to drop the idea of shared icons and install a copy for each program :/
Or wait, perhaps there is a solution with using KStandardDirs::addResourceDir(...), let me play a little, will report soon :)</pre>
</blockquote>
<p>On June 19th, 2012, 6:44 p.m., <b>Friedrich W. H. Kossebau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Sigh... using KStandardDirs::addResourceDir(...) also does not gain anything. The only dir which will be queried before the globally installed icons is "<component-name>/icons", by the component set for the KIconLoader (by default the global one). All other dirs will be only checked after the globally installed icons. So all those icons in the additional dirs will be invisible for which there is any icon in the global dirs whose names matches the starting tokens of those icons (e.g. "arrow-down" in global dir will mask "arrow-down-shape" in an additional dir).
So... I think to solve the arrow issue we should go for the draw-arrow-* icons as you proposed and simply remove all those arrow icons in plugins/pathshapes/pics, as they are either not used (all the non-rectangle ones, like left-up) or can be made unnecessary be the draw-arrow-* icons part of the Oxygen icons.
The Oxygen icons do not have the same style like the rest of the path shape icons, but neither do the existing icons, so no regression here.
Update of the patch coming next...</pre>
</blockquote>
<p>On June 19th, 2012, 7:49 p.m., <b>C. Boemann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">then how about: x-shape-text-artistic
it would still use x-shape-text for now
And format-font-uppercase and format-font-lowercase
I know they don't exist but at least it is more saying</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">x-shape-text-artistic, yes, good idea
hm, not sure about "font", better "text", so "format-text-uppercase/lowercase" ?
Would also be aligned with all the format-text-* in the current theme.</pre>
<br />
<p>- Friedrich W. H.</p>
<br />
<p>On June 19th, 2012, 7:12 p.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 June 19, 2012, 7:12 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;">Fixes all those icon names referencing to non-existing icons for which there is a proper icon in Oxygen 4.3.5 or coming with Calligra itself (IMHO :))
Also removes all arrow icons from plugins/pathshapes/pics, because they are either not used or can be replaced by draw-arrow-* icons from Oxygen icons.
Okay to commit both to master and 2.5?
Dealt entries from the report of the koIcon script, commented how I propose to solve it:
arrow-down-calligra
plugins/pathshapes/enhancedpath/EnhancedPathShapeFactory.cpp:298
arrow-up-calligra
plugins/pathshapes/enhancedpath/EnhancedPathShapeFactory.cpp:265
arrow-left-calligra
plugins/pathshapes/enhancedpath/EnhancedPathShapeFactory.cpp:232
Use the draw-arrow-* icons from Oxygen.
star
plugins/pathshapes/star/StarShapeFactory.cpp:40
14_layer_novisible
libs/kopageapp/KoPADocumentModel.cpp:314
14_layer_visible
libs/kopageapp/KoPADocumentModel.cpp:314
Oxygen from 4.3.5 has layer-visible-on and layer-visible-off icons, so use them.
fontsizeup
sheets/ui/CellToolBase.cpp:415
fontsizedown
sheets/ui/CellToolBase.cpp:421
There is also no icon in Kate/KWrite for these setting all text to lower/upper-case, so for now just use no icon here as well.
text
plugins/artistictextshape/ArtisticTextShapeFactory.cpp:33
Solved by reusing x-shape-text
open
plugins/pictureshape/PictureTool.cpp:110
plugins/staging/templateshape/TemplateTool.cpp:90
plugins/vectorshape/VectorTool.cpp:75
Solved by changing to document-open</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>libs/kopageapp/KoPADocumentModel.cpp <span style="color: grey">(69feb88)</span></li>
<li>plugins/artistictextshape/ArtisticTextShapeFactory.cpp <span style="color: grey">(2438b20)</span></li>
<li>plugins/pathshapes/enhancedpath/EnhancedPathShapeFactory.cpp <span style="color: grey">(d820c43)</span></li>
<li>plugins/pathshapes/pics/hi22-action-arrow-down.png <span style="color: grey">(c7e4dec)</span></li>
<li>plugins/pathshapes/pics/hi22-action-arrow-left-down.png <span style="color: grey">(d2443c8)</span></li>
<li>plugins/pathshapes/pics/hi22-action-arrow-left-up.png <span style="color: grey">(20afbd4)</span></li>
<li>plugins/pathshapes/pics/hi22-action-arrow-left.png <span style="color: grey">(2aff72f)</span></li>
<li>plugins/pathshapes/pics/hi22-action-arrow-right-down.png <span style="color: grey">(f165711)</span></li>
<li>plugins/pathshapes/pics/hi22-action-arrow-right-up.png <span style="color: grey">(f095b9a)</span></li>
<li>plugins/pathshapes/pics/hi22-action-arrow-up.png <span style="color: grey">(a1f66da)</span></li>
<li>plugins/pathshapes/pics/ox22-action-arrow-right-calligra.png <span style="color: grey">(dbb9e5a)</span></li>
<li>plugins/pathshapes/star/StarShapeFactory.cpp <span style="color: grey">(e6a9c6d)</span></li>
<li>plugins/pictureshape/PictureTool.cpp <span style="color: grey">(bf15751)</span></li>
<li>plugins/staging/templateshape/TemplateTool.cpp <span style="color: grey">(6373aa2)</span></li>
<li>plugins/vectorshape/VectorTool.cpp <span style="color: grey">(79742fd)</span></li>
<li>sheets/ui/CellToolBase.cpp <span style="color: grey">(89b79f9)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105284/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>