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

Ship it!


ok let us give it a go, reverts are cheap and this shouldn't halt developent if 
something was a bit wrong.


- C. Boemann


On May 1, 2012, 4:27 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104729/
> -----------------------------------------------------------
> 
> (Updated May 1, 2012, 4:27 p.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 161f085 
>   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
> 
>

_______________________________________________
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel

Reply via email to