<table><tr><td style="">kossebau added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D28355">View Revision</a></tr></table><br /><div><div><p>A  .rst file in the docs/module/ directory is needed, otherwise the documentation generation will not pick up this, as it runs only over docs/.<br />
Please enable the documentation generation in your ecm build and check for yourself, by e.g. ensuring <tt style="background: #ebebeb; font-size: 13px;">BUILD_HTML_DOCS</tt> is <tt style="background: #ebebeb; font-size: 13px;">ON</tt> and browsing the generated html in the build dir.</p>

<p>That might also show that leaving empty lines without leading # as treated as limit for the documentation IIRC, so all lines which should belong to processed docs need a leading #, other than what the current patch does.</p>

<p>Next I wonder why ".cmake.in" is an expected suffix here. That seems 2x suffixes used for input files. Usually in build systems input files have the suffix ".in", while with cmake there has also been a pattern of ".cmake" introduced, for whatever reason not liking <tt style="background: #ebebeb; font-size: 13px;">.in</tt>. Having both suffixes at the same time is surprising and odd to me, what do I miss?<br />
IMHO the code should rather support both ".in" and ".cmake" (first test the one, otherwise the other), and perhaps for covering Plasma  needs additionally ".cmake.in" first, if you have too many files already with that pattern to fix this with the actual files instead rather.</p>

<p>For the actual macro signatur, I wonder if something like</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">ecm_install_configured_file(TEMPLATES <file> [<file2> [...]] DESTINATION <INSTALL_DIRECTORY> [COPYONLY] [ESCAPE_QUOTES] [@ONLY] [COMPONENT <component>])</pre></div>

<p>would not be better:</p>

<ul class="remarkup-list">
<li class="remarkup-list-item">allows to handle multiple files if needed in one go</li>
<li class="remarkup-list-item">argument block names (TEMPLATES, DESTINATION) makes the code easier to read where the macro is used IMHO, and also allows extensibility in the future, if more arguments are found to be useful.</li>
<li class="remarkup-list-item">enabling to pass <tt style="background: #ebebeb; font-size: 13px;">COPYONLY</tt>, <tt style="background: #ebebeb; font-size: 13px;">ESCAPE_QUOTES</tt>, <tt style="background: #ebebeb; font-size: 13px;">@ONLY</tt>on to <tt style="background: #ebebeb; font-size: 13px;">configure_file()</tt> might be wanted as well</li>
<li class="remarkup-list-item">COMPONENT is underused, but some use it, so wanted to be passed on to <tt style="background: #ebebeb; font-size: 13px;">install()</tt></li>
</ul>

<p>Would be interesting to see actual examples where you made use of this new macro and how it improves things.</p>

<p>Also missing; unit test (sorry) :)</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D28355#inline-161383">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ECMConfiguredInstall.cmake:12</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"># .. code-block:: cmake</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d">#   ecm_install_configured_file(foo.txt.cmake.in ${KDE_INSTALL_FULL_DATADIR})</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">The 'FULL' variants are not meant to be used usually, only where really needed, so for consistency better to leave out from the example and just use <tt style="background: #ebebeb; font-size: 13px;">KDE_INSTALL_DATADIR</tt>, so people have recommended code to copy, and also do not wonder where the <tt style="background: #ebebeb; font-size: 13px;">FULL</tt> variant is different from what they see elsewhere.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R240 Extra CMake Modules</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D28355">https://phabricator.kde.org/D28355</a></div></div><br /><div><strong>To: </strong>davidedmundson<br /><strong>Cc: </strong>kossebau, pino, kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, GB_2, bencreasy, michaelh, ngraham, bruns<br /></div>