Last commit broke build with poppler<0.64, where getNextChar doesn't accept a const char first argument.
Details
- Reviewers
aacid pino - Group Reviewers
Calligra: 3.0 - Commits
- R8:3760bb301245: Fix build with poppler<0.64
R8:3e832a0950ea: Fix build with poppler<0.64
Builds with poppler as far back as 0.55
Diff Detail
- Repository
- R8 Calligra
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Please do a feature test at cmake time instead.
filters/karbon/pdf/SvgOutputDev.cpp | ||
---|---|---|
28 | NACK, this is not poppler core API |
filters/karbon/pdf/CMakeLists.txt | ||
---|---|---|
10 | Do you think if (NOT Poppler_VERSION VERSION_LESS "0.64.0") makes it more readable? I'm a bit undecided myself |
How about
if (Poppler_VERSION VERSION_LESS "0.64.0") add_definitions("-DHAVE_POPPLER_PRE_0_64")
That also removes the ambiguity while reading the code ("is this 0.64.0 and higher or only that specific version?") and *may* make it easier to clean obsolete code later on when support for Poppler < 0.64.0 is dropped.
I don't mind either way. In the future this could be changed to VERSION_GREATER_EQUAL when this can depend on cmake 3.7
Looks good, since you seem to have a pre 0.64 build at hand, could you check it still actually builds after i added all the overrides? Maybe we need some more ifdef
Indeed git master doesn't build with 0.55:
In file included from /home/antonio/Software/KDE/calligra/filters/karbon/pdf/PdfImport.cpp:23:
/home/antonio/Software/KDE/calligra/filters/karbon/pdf/SvgOutputDev.h:56:10: error: ‘void SvgOutputDev::drawString(GfxState*, const GooString*)’ marked ‘override’, but does not override
void drawString(GfxState * state, const GooString * s) override; ^~~~~~~~~~
But this one needs to be fixed in the 3.1 branch too, so I'd say let's push this to 3.1 and then fix the remaining issues in master in a separate commit
When you say "But this one needs to be fixed in the 3.1 branch too" you mean D16406 (i.e. this very same patch) or the compilation error because of the overrides?
The fact that the 3.1 branch "compiles" with the new poppler is just that, "it compiles", but it doesn't really work since the signature of the function it is supposed to reimplement has changed.
So we need "all the fixes" in "all the branches".
But sure, i guess let's commit this and then figure out what to do, not sure with the rest, at least this is a step forward :)
I mean this one, the other error doesn't happen in 3.1 since you only pushed the changes to master
Sure, as i said the fact that the error doesn't happen "means nothing", the code isn't working with a newer poppler because the functions signature is wrong (aka old)