Mmh, unfortunately GCC generates a lot of variable shadowing warnings while compiling the CLI11 library, which causes trouble in Travis because of the -Werror flag. We would have to find a way to contain the warnings generation to this library’s own code, and not generate any warning for third-party code. This is tricky because it is a header-only library. Not sure how to fix this.

Edit: I’m able to workaround this problem by forcing CMake to treat CLI11’s included directories as SYSTEM directories, which instructs GCC to not emit warnings for those files. However, this requires poking around the internals of the library, which is not that clean:

get_target_property(CLI11_include_dirs CLI11 INTERFACE_INCLUDE_DIRECTORIES)
set_target_properties(CLI11 PROPERTIES INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "${CLI11_include_dirs}")

It is loosely inspired from pdftk’s syntax.

That's just excellent, I would have done the same!

CLI11

Love it, wanted to use it for a long time. I can make sure it's placed at the right point and integrated well in our build system. Is it gcc 4.8 compatible?

Git submodule

not a big fan, will place a pinned version with cmake control for external lib if requested. similar to how we do it here: https://github.com/openPMD/openPMD-api

by default, if no output is specified

how does pdftk handle this situation?

shadowing warnings while compiling the CLI11

hm, why aren't they already included as system libs anyway?

Anyway, we could also add a gcc/clang pragma pushing/popping the warning around the include.
Also, we should report his upstream. Shadowing is subideal.

CLI11’s README claims compatibility with GCC ≥4.7. I’ve managed to compile it inside a Docker container with GCC 4.8. The shadowing warnings are almost exclusively for constructor arguments that shadow class member variables, and it looks like the folks from Clang decided not to report this use as shadowing.

Not sure about using a pragma for this, because it would imply having to surround all includes of this library, should we use it somewhere else in the future (although I guess we could make an internal header for this?).

I’m far from a CMake expert, but as far as I understand CMake treats included files as system headers only if the SYSTEM flag is passed to target_include_directories. Because we’re using the exported target from CLI11 instead of manually including the right folder, it uses the target_include_directories from CLI11’s CMake listfile, which does not have the SYSTEM flag.

Alright about submodules, I was not sure how you’d prefer including the dependency. Would you like me to change this or can you take care of it? I think you have write access to my branch if needed.

Regarding the syntax, pdftk aborts with an error if no output is specified. This is inconvenient if you want the output to be the same as the input, especially with lines files whose file names are long, because you have to type it twice. Furthermore, pdftk does not allow outputting to a file used as input—I can’t see any good reason for that, it just makes in-place modification harder. Do you think we should strive for a fully pdftk-compatible syntax? This would surely confuse users less.

If you like, we could also use CMake's ExternalProject(_Add) (or FetchContent) for the dependency to download it on the fly to the temporary build dir. It's an internal dependency, so that should work well. We can later on add an option to skip the download and use a locally available version as well.

hi @matteodelabre, sorry for being so inactive in the last months. I am still quite busy with my work...

That said, is it ok if I acknowledge you in a public talk at "Datenspuren" tomorrow? :)
Event (German): https://www.datenspuren.de/2018/fahrplan.html
Talk: https://www.datenspuren.de/2018/fahrplan/events/9324.html

Hi Axel, no worries, I’ve been quite busy too, but I should be able to make progress on this soon. About the talk, no problem; on the contrary, thanks for the acknowledgement! I hope this will convince more people into contributing to the project, and I wish you good luck for the talk.

Hi @ax3l! So, it turns out “soon” was quite a long time in the end. During the hiatus, the file format changes have made this kind of manipulation much easier, not requiring manipulation of the binary format anymore. Plus, moving pages around is now built-in in the tablet’s UI. All in all, I’m no longer convinced of the usefulness of this PR. What’s your take on this?

That's no problem, we can close it here, which still keeps it for informational purposes for readers if you like.