D13698: Improve ECMAddAppIconMacro.

[Christophe Giboudeaux]

cgiboudeaux added inline comments.

INLINE COMMENTS

> FindIcoTool.cmake:25
> +#
> +# Since 5.49.
> +

The next release will be 5.48

> ECMAddAppIcon.cmake:11-12
> +#                   ICONS <icon> [<icon> [...]]
> +#                   [SIDEBAR_ICONS <icon> [<icon> [...]] # Since 5.49
> +#                   [OUTFILE_BASENAME <name>]) # Since 5.49
> +#                   )

5.48

> ECMAddAppIcon.cmake:30
> +# icons to the generated iconset. They are used when a folder monitored by the
> +# application is dragged into Finder's sidebar. Since 5.49.
> +#

5.48

> ECMAddAppIcon.cmake:35
> +# and ``<OUTFILE_BASENAME>.ico`` on Windows. If you don't specify it, it defaults
> +# to ``<sources_var>.<ext>``. Since 5.49.
> +#

5.48

> ECMAddAppIcon.cmake:142
>  
> -    foreach(icon ${ARG_ICONS})
> -        get_filename_component(icon_full ${icon} ABSOLUTE)
> -        if (NOT EXISTS "${icon_full}")
> -            message(AUTHOR_WARNING "${icon_full} does not exist, ignoring")
> -        else()
> -            get_filename_component(icon_name ${icon} NAME)
> -            string(REGEX MATCH "([0-9]+)\\-[^/]+\\.([a-z]+)$"
> -                               _dummy "${icon_name}")
> -            set(size  "${CMAKE_MATCH_1}")
> -            set(ext   "${CMAKE_MATCH_2}")
> -            if (NOT (ext STREQUAL "svg" OR ext STREQUAL "svgz"))
> -                if (NOT size)
> -                    message(AUTHOR_WARNING "${icon_full} is not named correctly for ecm_add_app_icon - ignoring")
> -                elseif (NOT ext STREQUAL "png")
> -                    message(AUTHOR_WARNING "${icon_full} is not a png file - ignoring")
> -                else()
> -                    list(FIND known_sizes "${size}" offset)
> -                    if (offset GREATER -1)
> -                        list(APPEND icons_at_${size}px "${icon_full}")
> -                    endif()
> -                endif()
> -            endif()
> -        endif()
> -    endforeach()
> +    _ecm_add_app_icon_categorize_icons("${ARG_ICONS}" "icons" "16;32;48;64;128;256;512;1024")
> +    if(ARG_SIDEBAR_ICONS)

24 is mentioned in the doc but unused here. Is it expected?

> ECMAddAppIcon.cmake:161
> +                  ${sidebar_icons_at_32px}
> +                  ${sidebar_icons_at_36px}
> +                  ${sidebar_icons_at_64px})

36? It's not mentioned in the doc.

> ECMAddAppIcon.cmake:228
> +
> +            foreach(size 16 32 48 64 128 ${maxSize})
> +                if(NOT icons_at_${size}px)

not 24 ?

> CMakeLists.txt:14
>  
> -ecm_add_app_icon(OUT ICONS ${ICONS})
> +ecm_add_app_icon(OUT ICONS ${ICONS} SIDEBAR_ICONS ${SIDEBAR_ICONS} OUTFILE_BASENAME "SuperBasename")
>  

The test shouldn't "replace" the simple form. Add a new one instead to have both forms tested.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D13698

To: dschmidt, vonreth, vpinon, apol, alexmerry
Cc: cgiboudeaux, kde-frameworks-devel, kde-buildsystem, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20180629/ce2dff24/attachment.html>