Review Request: Fixes: misspelt icon names, improved icon choices, out-dated/missing icons

C. Boemann cbr at boemann.dk
Sat Apr 28 14:15:30 BST 2012


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



plugins/textshape/TextTool.cpp
<http://git.reviewboard.kde.org/r/104729/#comment10128>

    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


- C. Boemann


On April 26, 2012, 12:16 a.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104729/
> -----------------------------------------------------------
> 
> (Updated April 26, 2012, 12:16 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   kexi/core/KexiWindow.cpp f49c967 
>   kexi/formeditor/editlistviewdialog.cpp f66ceaa 
>   kexi/formeditor/richtextdialog.cpp 80748a5 
>   kexi/main/startup/KexiNewProjectAssistant.cpp 1c52d79 
>   kexi/plugins/reports/kexireportview.cpp ccdf3f9 
>   kexi/plugins/scripting/kexiscripting/kexiscriptdesignview.cpp d4d8e2d 
>   kexi/widget/navigator/KexiProjectListView.cpp 3fa561f 
>   kexi/widget/navigator/KexiProjectNavigator.cpp 9c39a1a 
>   kexi/widget/pixmapcollection.cpp b4f9dfe 
>   plugins/chartshape/ChartConfigWidget.cpp 29af6cb 
>   plugins/chartshape/dialogs/TableEditorDialog.cpp 62acce9 
>   plugins/textshape/TextTool.cpp 1dc2e02 
>   sheets/dialogs/LinkDialog.cpp 4ce4636 
>   sheets/plugins/calendar/CalendarToolWidget.cpp a7c7814 
>   sheets/shape/SheetsEditor.cpp 5c1ba6b 
>   sheets/ui/CellToolBase.cpp 33f2d29 
>   sheets/ui/CellToolBase_p.cpp 0cba586 
> 
> Diff: http://git.reviewboard.kde.org/r/104729/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120428/652f02fe/attachment.htm>


More information about the calligra-devel mailing list