Windows: Add NFD_OVERRIDE_RECENT_WITH_DEFAULT flag

[dbierek]

Implements #151

Implemented using a cmake option with default value set to off.

src/CMakeLists.txt

option(NFD_OVERRIDE_RECENT_WITH_DEFAULT "Use defaultPath instead of recent folder on Windows" OFF)
if (NFD_OVERRIDE_RECENT_WITH_DEFAULT)
    target_compile_definitions(${TARGET_NAME} PRIVATE NFD_OVERRIDE_RECENT_WITH_DEFAULT)
endif()

nfd_win.cpp

#ifdef NFD_OVERRIDE_RECENT_WITH_DEFAULT
    // Use SetFolder() if you always want to use the default folder
    if (!SUCCEEDED(dialog->SetFolder(folder))) {
        NFDi_SetError("Failed to set default path.");
        return NFD_ERROR;
    }
#else
    // SetDefaultFolder() might use another recently used folder if available, so the user
    // doesn't need to keep navigating back to the default folder (recommended by Windows).
    if (!SUCCEEDED(dialog->SetDefaultFolder(folder))) {
        NFDi_SetError("Failed to set default path.");
        return NFD_ERROR;
    }
#endif

And then when a downstream app wants to use this they can set the cmake build option to ON:

This makes it so that downstream apps don't need to make any changes to keep the existing behavior, and can just set the build option when they want to use the new behavior.

Screenshots showing the new functionality:

[btzy]

I'm wondering if we should use a CMake build option (that sets a #define macro) instead. All other configuration options do that.

[dbierek]

I'm wondering if we should use a CMake build option (that sets a #define macro) instead. All other configuration options do that.

I've never worked with CMake build options before, but would something like the most recent changes work?

[geertbleyen]

Interestingly enough, in my team we were having the same discussion, where we want to be able to finely control the default directory when opening a directory. A cmake option would work in case this needs to be consistently done in one way or the other in the entirety of the application. Not sure if would be interesting to be able to control the behavior on a per-call basis.

[btzy]

Sorry for the delays, I have been kinda busy over the past few weeks.

Yeah, I was also thinking about having it on a per-call basis instead, which would be the more flexible thing to do, and better mirror the native API. (Note that the original extern int NFD_OVERRIDE_RECENT_WITH_DEFAULT; proposal also can't be controlled per-call.)

This is also kinda related to the IFileDialog::SetClientGuid API that Windows natively has - it will make the file dialog remember state (including the last visited folder) on a per-GUID basis, meaning that you can have a program that uses multiple different GUIDs (one for each kind of dialog, e.g. one for opening a project file, one for opening a source code file, and one for opening a config file) that will each remember their last visited folder. If this will work with your use case then it's probably the more correct thing to do than to force the default folder, since it will also remember other state. The problem is that this is not available on macOS and Linux, so it will either have to be ignored or we will need a manual implementation on those platforms.

[btzy]

Thanks for the PR! I'm happy to merge this as is, since it seems pretty unlikely that someone would want to control the behaviour on a per-call basis, especially without IFileDialog::SetClientGuid being available in NFDe.

[dbierek]

Happy to contribute! 😄That sounds good to me 👍
Thanks for the awesome lib!

[btzy]

Thanks, this looks mostly good!

[btzy]

Small nit, but can we say that it's a build option, so people who aren't too familiar with CMake won't think that they are compiler defines? Seems like the other two build options mentioned in the README say it more plainly, e.g. "add -DNFD_PORTAL=ON to the build command".

[dbierek]

I think this should be good to go now.
Screenshots showing the code path changes:
OFF:

ON:

Note related to the discussion about controlling this on individual calls:
I originally had added a field to nfdopendialognargs_t in a5daec7#diff-fac18ee5b488fa89619c3640fdef00999b0bdfa8daf091a4989edba3c1cf4bc6R378, but found that this would lead to having to include the option for all platforms even though it's Windows specific, or break the platform agnosticism.

I think even if we were to do something similar where we allow passing in a GUID, we would run into a similar issue, or we'd have to add a manual implementation for the other platforms as you mentioned.

[btzy]

I'll take a final look later today and will merge it if it looks fine!

[btzy]

Thanks, looks good!