Review Request: Fix some icon names to reference existing icons (2.5 branch)

C. Boemann cbr at boemann.dk
Tue Jun 19 20:49:01 BST 2012 


> On June 18, 2012, 7:51 a.m., C. Boemann wrote:
> > 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.
> 
> Friedrich W. H. Kossebau wrote:
>     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.
> 
> C. Boemann wrote:
>     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
> 
> Friedrich W. H. Kossebau wrote:
>     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 :)
> 
> Friedrich W. H. Kossebau wrote:
>     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...

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


- C.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105284/#review14822
-----------------------------------------------------------


On June 19, 2012, 7:12 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105284/
> -----------------------------------------------------------
> 
> (Updated June 19, 2012, 7:12 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> 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
> 
> 
> Diffs
> -----
> 
>   libs/kopageapp/KoPADocumentModel.cpp 69feb88 
>   plugins/artistictextshape/ArtisticTextShapeFactory.cpp 2438b20 
>   plugins/pathshapes/enhancedpath/EnhancedPathShapeFactory.cpp d820c43 
>   plugins/pathshapes/pics/hi22-action-arrow-down.png c7e4dec 
>   plugins/pathshapes/pics/hi22-action-arrow-left-down.png d2443c8 
>   plugins/pathshapes/pics/hi22-action-arrow-left-up.png 20afbd4 
>   plugins/pathshapes/pics/hi22-action-arrow-left.png 2aff72f 
>   plugins/pathshapes/pics/hi22-action-arrow-right-down.png f165711 
>   plugins/pathshapes/pics/hi22-action-arrow-right-up.png f095b9a 
>   plugins/pathshapes/pics/hi22-action-arrow-up.png a1f66da 
>   plugins/pathshapes/pics/ox22-action-arrow-right-calligra.png dbb9e5a 
>   plugins/pathshapes/star/StarShapeFactory.cpp e6a9c6d 
>   plugins/pictureshape/PictureTool.cpp bf15751 
>   plugins/staging/templateshape/TemplateTool.cpp 6373aa2 
>   plugins/vectorshape/VectorTool.cpp 79742fd 
>   sheets/ui/CellToolBase.cpp 89b79f9 
> 
> Diff: http://git.reviewboard.kde.org/r/105284/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120619/82700f78/attachment.htm>

More information about the calligra-devel mailing list