Consider compiling source separately

[funbiscuit]

I very like the concept of single header libraries and I prefer to use them instead of others when I have a choice. Usually they have special macro that you need to declare before including them to indicate that they need to include implementation of all functions. For example:

In exactly one .cpp file we write:

#define SOME_LIB_IMPLEMENTATION
#include <some-lib.h>

And in all others we write just

#include <some-lib.h>

This library can be included in multiple files without any compiler errors but still it compiles multiple times. And some times there are reasons to even create separate .cpp file for header compilation so it takes less memory to compile the whole project. For example, in one of my projects I use Qt and I include this lib in MainWindow class. If i remove inclusion of header it takes about 180MB less to compile this one source file. To compile it separately it uses much less than 180MB RAM. Of course 180MB RAM is not very much for a normal PC but it is quite a lot for virtual machine since I usually don't allocate much memory for them.

So I propose to separate all implementation to define-protected part of the header. And user will have to define some constant like PORTABLE_FD_IMPLEMENTATION before including this header in file where user wants to put implementation. All other files will use lightweight header.
As a side effect it will make a cleaner interface. I mean it will be easier to see it in header without scrolling hundreds of lines.

[samhocevar]

Thanks for the suggestion. I have more or less implemented it in cddb90d.

I have however decided to make the feature opt-in; you will have to #define PFD_SKIP_IMPLEMENTATION whenever you do not want the implementation. That is because I do not want people to get unexpected link errors when all they do is update a .h file.

[funbiscuit]

Nice work! This behavior seems to be the only backwards-compatible way. I may also suggest adding simple wrapper around header to invert behavior. Something like pfd.h with following contents

#ifndef PFD_IMPLEMENTATION
#define PFD_SKIP_IMPLEMENTATION
#endif
#include "portable-file-dialogs.h"

So if you're using pfd in multiple files you won't need to use define in each file, just include wrapper and define in one (possible separate, pfd only) implementation file.

Btw, you also have a missing ; at line 209 (windows only)

[mnesarco]

Hi, I am trying to use PFD_SKIP_IMPLEMENTATION to create a separate compilation unit, but I get some link errors:

portable-file-dialogs/pfd.hpp

#pragma once
#define PFD_SKIP_IMPLEMENTATION
#include "portable-file-dialogs.h"

portable-file-dialogs/pfd.cpp

#include "portable-file-dialogs.h"

test.cpp

#include "portable-file-dialogs/pfd.hpp"

// ...
auto selection = pfd::open_file("Select a file", ".", { "SVG Files", "*.svg"}, pfd::opt::none).result();
// ...

file pfd.cpp is compiled and linked:

[build] [2/3  66% :: 0.196] Building CXX object test/CMakeFiles/test.dir/Debug/src/portable-file-dialogs/pfd.cpp.o

Link errors:

[build] ...: undefined reference to `pfd::open_file::open_file(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&, pfd::opt)'
[build] ...: undefined reference to `pfd::open_file::result()'

[funbiscuit]

@mnesarco Well, looks like it won't work the way it's currently implemented. Implementation of all internal methods, like pfd::open_file::open_file is written with "inline" keyword. Because of that, creation of separate .cpp file with inclusion of portable-file-dialogs.h doesn't create any symbol that will be visible to linker, that's why you get unresolved externals. To fix that, we need to remove inline keyword from all methods inside PFD_SKIP_IMPLEMENTATION block. But that will break all code, which doesn't use this define (since there will be multiple definitions with the same name). I think that the only reasonable choice would be to add something like that:

#ifndef PFD_INTERNAL 
#ifndef PFD_SKIP_IMPLEMENTATION
#define PFD_INTERNAL inline
#elif
#define PFD_INTERNAL
#endif
#endif

All "inline" keywords inside PFD_SKIP_IMPLEMENTATION block will be replaced by PFD_INTERNAL macro.
And then in your pfd.h you'll need to define that macro to be empty:

#pragma once
#define PFD_SKIP_IMPLEMENTATION
#define PFD_INTERNAL
#include "portable-file-dialogs.h"

The only limitation of this approach is that you need to choose one option. Either you use portable-file-dialogs.h everywhere and don't use PFD_SKIP_IMPLEMENTATION macro OR you create separate pfd.h with pfd.cpp and use pfd.h everywhere.

[mnesarco]

Thank you for the detailed info. I like the second option, keeping portable-file-dialogs.h internal and using pfd.h everywhere with the corresponding pfd.cpp linked. Do you plan to introduce the PFD_INTERNAL macro approach in the master branch?

[funbiscuit]

@samhocevar what do you think? Is that PFD_INTERNAL solution looks reasonable for you? Or maybe you can think of another solution?

[mnesarco]

PFD_SKIP_IMPLEMENTATION does not work at all. So another approach is to create a small wrapper, it requires to redeclare the API, but the public API is quite small. So I have tested this:

pfd.hpp : Always include only this file

#pragma once

#include <string>
#include <vector>
#include <memory>

namespace pfd
{
    class notify;
    class message;
    class open_file;
    class save_file;
    class select_folder;


    enum class Icon
    {
        info = 0,
        warning,
        error,
        question,
    };


    enum class Button
    {
        cancel = -1,
        ok,
        yes,
        no,
        abort,
        retry,
        ignore,
    };


    enum class Choice
    {
        ok = 0,
        ok_cancel,
        yes_no,
        yes_no_cancel,
        retry_cancel,
        abort_retry_ignore,
    };


    enum class Options : uint8_t
    {
        none = 0,
        // For file open, allow multiselect.
        multiselect     = 0x1,
        // For file save, force overwrite and disable the confirmation dialog.
        force_overwrite = 0x2,
        // For folder select, force path to be the provided argument instead
        // of the last opened directory, which is the Microsoft-recommended,
        // user-friendly behaviour.
        force_path      = 0x4,
    };


    inline Options operator |(Options a, Options b) { return Options(uint8_t(a) | uint8_t(b)); }
    inline bool operator &(Options a, Options b) { return bool(uint8_t(a) & uint8_t(b)); }


    class Settings
    {
    public:
        static int const default_wait_timeout = 20;
        static bool available();
        static void verbose(bool value);
        static void rescan();
    };


    class Notify
    {
    public:
        Notify(std::string const &title, std::string const &message, Icon icon = Icon::info);
        ~Notify();
        bool ready(int timeout = Settings::default_wait_timeout) const;
        bool kill() const;
    private:
        notify* impl;
    };


    class Message
    {
    public:
        Message(std::string const &title, std::string const &text, Choice choice = Choice::ok_cancel, Icon icon = Icon::info);
        ~Message();
        bool ready(int timeout = Settings::default_wait_timeout) const;
        bool kill() const;
        Button result() const;
    private:
        message* impl;
    };


    class OpenFile
    {
    public:
        OpenFile(std::string const &title,
                std::string const &default_path = "",
                std::vector<std::string> const &filters = { "All Files", "*" },
                Options options = Options::none);
        ~OpenFile();
        bool ready(int timeout = Settings::default_wait_timeout) const;
        bool kill() const;
        std::vector<std::string> result() const;
    private:
        open_file* impl;
    };


    class SaveFile
    {
    public:
        SaveFile(std::string const &title,
                std::string const &default_path = "",
                std::vector<std::string> const &filters = { "All Files", "*" },
                Options options = Options::none);
        ~SaveFile();
        bool ready(int timeout = Settings::default_wait_timeout) const;
        bool kill() const;
        std::string result() const;
    private:
        save_file* impl;
    };


    class SelectFolder
    {
    public:
        SelectFolder(std::string const &title,
                    std::string const &default_path = "",
                    Options options = Options::none);
        ~SelectFolder();
        bool ready(int timeout = Settings::default_wait_timeout) const;
        bool kill() const;
        std::string result() const;
    private:
        select_folder* impl;
    };

}

pfd.cpp : Link this file

#include "pfd.hpp"
#include "portable-file-dialogs.h"

#include <cstdint>
#include <memory>

bool pfd::Settings::available()                     { return settings::available(); }
void pfd::Settings::verbose(bool value)             { settings::verbose(value); }
void pfd::Settings::rescan()                        { settings::rescan(); }


pfd::Notify::Notify(
    std::string const &title, 
    std::string const &message, 
    Icon t_icon)
    : impl{new pfd::notify{title, message, icon(int(t_icon))}} {}

pfd::Notify::~Notify() { delete impl; }

bool pfd::Notify::ready(int timeout) const          { return impl->ready(timeout); }
bool pfd::Notify::kill() const                      { return impl->kill(); }


pfd::Message::Message(
    std::string const &title, 
    std::string const &text, 
    Choice t_choice, 
    Icon t_icon)
    : impl{new pfd::message{title, text, choice(int(t_choice)), icon(int(t_icon))}} {}

pfd::Message::~Message() { delete impl; }

bool pfd::Message::ready(int timeout) const         { return impl->ready(timeout); }
bool pfd::Message::kill() const                     { return impl->kill(); }
pfd::Button pfd::Message::result() const            { return Button(int(impl->result())); }


pfd::OpenFile::OpenFile(std::string const &title,
    std::string const &default_path,
    std::vector<std::string> const &filters,
    Options options)
    : impl{new open_file{title, default_path, filters, opt(uint8_t(options))}} {}

pfd::OpenFile::~OpenFile() { delete impl; }

bool pfd::OpenFile::ready(int timeout) const            { return impl->ready(timeout); }
bool pfd::OpenFile::kill() const                        { return impl->kill(); }
std::vector<std::string> pfd::OpenFile::result() const  { return impl->result(); }


pfd::SaveFile::SaveFile(
    std::string const &title,
    std::string const &default_path,
    std::vector<std::string> const &filters,
    Options options)
    : impl{new save_file{title, default_path, filters, opt(uint8_t(options))}} {}

pfd::SaveFile::~SaveFile() { delete impl; }

bool pfd::SaveFile::ready(int timeout) const        { return impl->ready(timeout); }
bool pfd::SaveFile::kill() const                    { return impl->kill(); }
std::string pfd::SaveFile::result() const           { return impl->result(); }


pfd::SelectFolder::SelectFolder(
    std::string const &title,
    std::string const &default_path,
    Options options)
    : impl{new select_folder{title, default_path, opt(uint8_t(options))}} {}

pfd::SelectFolder::~SelectFolder() { delete impl; }

bool pfd::SelectFolder::ready(int timeout) const    { return impl->ready(timeout); }
bool pfd::SelectFolder::kill() const                { return impl->kill(); }
std::string pfd::SelectFolder::result() const       { return impl->result(); }

Usage: use the new API (just capitalize the types. Another option is to use a different namespace and keep the type names equal)

// Use
#include <pfd.hpp>

auto selection = pfd::OpenFile("Select a file", ".",
                                { "SVG Files", "*.svg"},
                                pfd::Options::none).result();

// Instead of
#include <portable-file-dialogs.h>

auto selection = pfd::openFile("Select a file", ".",
                                { "SVG Files", "*.svg"},
                                pfd::options::none).result();

Drawbacks:

1. To effectively hide the implementations using opaque pointers, dynamic allocation was used.
2. Duplicated API