<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/111954/">http://git.reviewboard.kde.org/r/111954/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 9th, 2013, 8:20 a.m. UTC, <b>Friedrich W. H. Kossebau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hi Sebastian,
In the design of these product definition macros currently all the rules are whitelisting rules, not blacklisting rules.
The FILTER_<appname> products are kind of meta-products (or rather productsets) which are used to spare people to have to list each and every filter in their custom productset (and also the premade ones). I am unhappy with that they are also called products, plan to turn them into rather "productsets" as well, because they have a different purpose/semantic. Sets will be listed in sets then as well.
Support for BUILD_xxx=FALSE in the cmake call was rather meant as minimal legacy support for some existing scripts out there, which had xxx as some appname. But it never was done to properly support (sub-)products, like the filters (or filter groups).
Given the recent feedback I plan to add this finally (wanted to do last WE already, so hopefully will do at the upcoming).
So if you do not want to have filters build, create your own productset file in which they are not listed (or for the porting turn all related lines into comments in the predefined productssets, though that will have no effect on private ones).
If filters are broken in general, then with the current system you might need to add them one-by-one in the disable-because-broken area (yes, porting with mass-breaking states was not considered, bah :) ).
But the proposed patch would not match the current logic, so I have to reject it. Will see to solve this in the system design ASAP.</pre>
</blockquote>
<p>On August 9th, 2013, 5:18 p.m. UTC, <b>Inge Wallin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I would also prefer if we continued this migration from filter_<app>_something to filter_<format>_something. So however this goes, can we name the macros or variables after ODT, ODS, etc?</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Inge, the products matching the individual filters are already named like that (unless targetting more than one format).
This patch was about groups of filters, but there exists the problem that some of the filters have either as source or target the working memory model of some app, not a file (with whatever format). So that first needs to be changed before such a name change makes sense.
E.g. for Words/Author there is the FILTER_ASCII_TO_WORDS which uses the KoDocument target, not some file (OUTPUT_AS_ODT_FILE is not defined).
</pre>
<br />
<p>- Friedrich W. H.</p>
<br />
<p>On August 8th, 2013, 8:26 p.m. UTC, Sebastian Sauer wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Calligra and Friedrich W. H. Kossebau.</div>
<div>By Sebastian Sauer.</div>
<p style="color: grey;"><i>Updated Aug. 8, 2013, 8:26 p.m.</i></p>
<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;">Those new product flags define for each top-level application a matching FILTER_<appname> flag. That flag allows to disable all filters for a certain app(s) but it (seems?) they are not evaluated and applied. Following patch works for me in that if I now add something like
calligra_disable_product(FILTER_WORDS "Qt5 TODO")
calligra_disable_product(FILTER_SHEETS "Qt5 TODO")
calligra_disable_product(FILTER_STAGE "Qt5 TODO")
to the top-level CMakeLists.txt then all filters for Words, Sheets and Stage are disabled means not build.
p.s. an alternate way to solve that could be to e.g.
in calligra/CMakeLists.txt
- calligra_define_product(FILTER_APPLIXSPREAD_TO_KSPREAD "Applix Spreadsheet to KSpread filter" NEEDS SHEETS_PART)
+ calligra_define_product(FILTER_APPLIXSPREAD_TO_KSPREAD "Applix Spreadsheet to KSpread filter" NEEDS SHEETS_PART FILTER_SHEETS)
What's the prefered way?</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;">Note that testing was done with the Qt5-branch. I ask for review (and merge to master) so I get feedback I didn't got something wrong here how those product-flags are supposed to work. Since there are way more such switches I may have to add more such cases here and there and so it would make sense to at least have the initial patch reviewed asap before investing time into solving a not-existing problem and/or a solving it in a wrong way :)
</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>filters/CMakeLists.txt <span style="color: grey">(79a42a2)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111954/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>