[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-buildsystem
Subject: Re: Review Request 122135: Add ecm_add_app_icon function.
From: "Ralf Habacker" <ralf.habacker () freenet ! de>
Date: 2015-01-24 10:53:00
Message-ID: 20150124105300.32443.33240 () probe ! kde ! org
[Download RAW message or body]
--===============6360936761880139290==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122135/#review74653
-----------------------------------------------------------
Ship it!
modules/ECMAddAppIcon.cmake
<https://git.reviewboard.kde.org/r/122135/#comment51737>
pixel size 22 is not used in the code and should be removed here
modules/ECMAddAppIcon.cmake
<https://git.reviewboard.kde.org/r/122135/#comment51736>
trailing whitespace
modules/ECMAddAppIcon.cmake
<https://git.reviewboard.kde.org/r/122135/#comment51735>
trailing whitespace
modules/ECMAddAppIcon.cmake
<https://git.reviewboard.kde.org/r/122135/#comment51734>
This should also be ${CMAKE_CURRENT_SOURCE_DIR}
windows support otherwise looks good.
- Ralf Habacker
On Jan. 24, 2015, 11:11 vorm., Alex Merry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122135/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2015, 11:11 vorm.)
>
>
> Review request for Extra Cmake Modules, Ralf Habacker and Jeremy Whiting.
>
>
> Repository: extra-cmake-modules
>
>
> Description
> -------
>
> This adds an application icon to an executable from PNG files for
> Windows and Mac OS X. Unlike the similar kde4_add_app_icon macro from
> kdelibs, this requires icons to be explicitly listed as arguments
> (meaning CMake can tell when ones are added or deleted and reconfigure
> as appropriate), and it works with Matthias Benkmann's png2ico tool, as
> well as the KDE-Win tool of the same name.
>
> Currently missing unit tests. Also completely untested (except that
> `make test` runs on Linux, so there are no obvious syntax errors).
>
> With thanks to Ralf Habacker for the initial work on porting
> kde4_add_app_icon.
>
> CHANGELOG: Add ECMAddAppIcon module to add icons to executable targets
> on Windows and Mac OS X.
>
>
> Diffs
> -----
>
> docs/find-module/FindPng2Ico.rst PRE-CREATION
> docs/module/ECMAddAppIcon.rst PRE-CREATION
> find-modules/FindPng2Ico.cmake PRE-CREATION
> modules/ECMAddAppIcon.cmake PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/122135/diff/
>
>
> Testing
> -------
>
> `make test` passes, which just provides a very basic check for syntax errors.
>
>
> Thanks,
>
> Alex Merry
>
>
--===============6360936761880139290==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/122135/">https://git.reviewboard.kde.org/r/122135/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://git.reviewboard.kde.org/r/122135/diff/5/?file=344457#file344457line19" \
style="color: black; font-weight: bold; text-decoration: \
underline;">modules/ECMAddAppIcon.cmake</a> <span style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">19</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="c"># ``<size>`` is a numeric pixel size (typically 16, 22, 32, 48, 64, \
128 or</span></pre></td> </tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">pixel \
size 22 is not used in the code and should be removed here</p></pre> </div>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://git.reviewboard.kde.org/r/122135/diff/5/?file=344457#file344457line140" \
style="color: black; font-weight: bold; text-decoration: \
underline;">modules/ECMAddAppIcon.cmake</a> <span style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">140</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="o">${</span><span class="nv">windows_icons</span><span \
class="o">}</span><span class="ew"> </span></pre></td> </tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">trailing whitespace</p></pre> </div>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://git.reviewboard.kde.org/r/122135/diff/5/?file=344457#file344457line148" \
style="color: black; font-weight: bold; text-decoration: \
underline;">modules/ECMAddAppIcon.cmake</a> <span style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">148</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="s">ARGS</span> <span class="s2">"${_outfilename}.ico"</span> \
<span class="o">${</span><span class="nv">windows_icons</span><span \
class="o">}</span><span class="ew"> </span></pre></td> </tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">trailing whitespace</p></pre> </div>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://git.reviewboard.kde.org/r/122135/diff/5/?file=344457#file344457line150" \
style="color: black; font-weight: bold; text-decoration: \
underline;">modules/ECMAddAppIcon.cmake</a> <span style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">150</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="s">WORKING_DIRECTORY</span> <span \
class="s2">"${CMAKE_CURRENT_BINARY_DIR}"</span></pre></td> </tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This \
should also be ${CMAKE_CURRENT_SOURCE_DIR}</p></pre> </div>
</div>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">windows support otherwise looks good.</p></pre>
<p>- Ralf Habacker</p>
<br />
<p>On Januar 24th, 2015, 11:11 vorm. CET, Alex Merry wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Extra Cmake Modules, Ralf Habacker and Jeremy Whiting.</div>
<div>By Alex Merry.</div>
<p style="color: grey;"><i>Updated Jan. 24, 2015, 11:11 vorm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
extra-cmake-modules
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">This adds an application icon to an executable from \
PNG files for Windows and Mac OS X. Unlike the similar kde4_add_app_icon macro from
kdelibs, this requires icons to be explicitly listed as arguments
(meaning CMake can tell when ones are added or deleted and reconfigure
as appropriate), and it works with Matthias Benkmann's png2ico tool, as
well as the KDE-Win tool of the same name.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Currently missing unit tests. Also completely untested \
(except that `make test` runs on Linux, so there are no obvious syntax errors).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">With thanks to Ralf Habacker for the initial work on \
porting kde4_add_app_icon.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">CHANGELOG: Add ECMAddAppIcon module to add icons to \
executable targets on Windows and Mac OS X.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;"><code style="text-rendering: inherit;color: \
#4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">make \
test</code> passes, which just provides a very basic check for syntax \
errors.</p></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>docs/find-module/FindPng2Ico.rst <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>docs/module/ECMAddAppIcon.rst <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>find-modules/FindPng2Ico.cmake <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>modules/ECMAddAppIcon.cmake <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122135/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============6360936761880139290==--
_______________________________________________
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic