portable-file-dialogs icon indicating copy to clipboard operation
portable-file-dialogs copied to clipboard

Set default file extension in Windows save dialog

Open samangh opened this issue 1 year ago • 10 comments

First of all, thank you for your work on portable-file-dialogs. It's quite useful :-).

This pull request will set the default file extension when using the file save dialog in Windows. The default file extension is set to the first file extension provided in the filter pattern.

It turns out that the GetSaveFileNameW function in Windows does not automatically get the file extension from the filters, instead the the default extension must be manually set. So this is what this pull request does.

This should close issue #43.

samangh avatar Mar 12 '23 02:03 samangh

Questions:

  1. What happens if the user selects a different file extension from the dropdown? Does the file a) Still get saved with the default extension b) Uses the new extension selected c) Uses no extension d) Other
  1. Seem to me like regular expressions are overkill for getting the extension value. Can't a few calls to std::string::find() accomplish that?

gamagan avatar Mar 12 '23 16:03 gamagan

@gamagan, thanks for the questions. They caused me to look at this more closely and do some experimentation.

I was actually incorrect in my previous message. As long as the lpstrDefExt pointer is allocated, Windows does the right thing and uses the extension defined in the pattern filter. The actual content of lpstrDefExt is only used if the filter pattern is blank or is *.*, and the user has not entered an extension.

I have updated my pull request to match this. In the new pull request, things should work as expected. So, for example let's say you do:

pfd::save_file("Select a file", ".",                            
              { "Image Files", "*.png *.jpg *.jpeg *.bmp",
                "AudioFiles", "*.wav *.mp3",
                "All Files", "*" });

The default extension will be .png if Image Files is selected, .wav if Audio Files is selected, and no extension is added if All files is selected.

samangh avatar Mar 13 '23 00:03 samangh

Wow. That's nice that the change turned out to be so simple for the benefits it provides. Good work.

So, Windows only supports 3 letter extensions, in that case? There's no code to set the size of the buffer, so I would think that the extension size would be fixed. It wouldn't be ideal if it only supported 3 letters.

gamagan avatar Mar 13 '23 14:03 gamagan

Yes, the automatically added extensions are limited to 3 characters. This is a limitation of the deprecated GetSaveFileNameW function used here.

The lpstrDefExt value defines the default extension when the function can't figure out what the extension should be (i.e. the pattern filter blank or is *.*). We leave it empty, so that Windows does not add an extention in those cases. As we are leaving lpstrDefExt empty, it's size is irrelevant.

samangh avatar Mar 13 '23 14:03 samangh

@samhocevar, I was wondering if you've had a chance to look at the PR? I think it's a nice uncontroversial 2-line change.

samangh avatar Mar 30 '23 23:03 samangh

@samhocevar? Any chance of merging this pull request?

samangh avatar Jan 13 '24 11:01 samangh

@samhocevar? Any chance of merging this pull request?

Yup, accepting this PR would make our lives much, much easier @samhocevar

This small library is really great, it would be a waste to abandon it.

tgalaj avatar Jan 31 '24 21:01 tgalaj

I have the same problem, the extension is not automatically added and I'd like this to be merged too. For now, I'm using @samangh branch.

I did further improve this actually so I thought I would share. I didn't wanted to open yet another PR. @samangh maybe you can update yours ? Anyway, here are the two small modifications:

  • increase the extension length to 64 characters (with only 3 characters it wouldn't work with ".json" for example).
  • Solved a similar bug in file open dialogs: if the user types a name of a file that does not exists, there was no popup showing (works well including with multiple file selections).

In the end, the few lines at that place in the file become:

        if (in_type == type::save)
        {
            auto wextension = std::wstring(64, L'\0');              // Changed 3 to 64 here
            ofn.lpstrDefExt = (LPWSTR)wextension.data();
            
            if (!(options & opt::force_overwrite))
                ofn.Flags |= OFN_OVERWRITEPROMPT;

            dll::proc<BOOL WINAPI (LPOPENFILENAMEW)> get_save_file_name(comdlg32, "GetSaveFileNameW");
            if (get_save_file_name(&ofn) == 0)
                return "";
            return internal::wstr2str(woutput.c_str());
        }
        else
        {
            if (options & opt::multiselect)
                ofn.Flags |= OFN_ALLOWMULTISELECT;
            ofn.Flags |= OFN_PATHMUSTEXIST;
            if (in_type == type::open)                                 // Added these two lines
                ofn.Flags |= OFN_FILEMUSTEXIST;

            dll::proc<BOOL WINAPI (LPOPENFILENAMEW)> get_open_file_name(comdlg32, "GetOpenFileNameW");
            if (get_open_file_name(&ofn) == 0)
                return "";
        }

christophe-f8 avatar Feb 29 '24 08:02 christophe-f8

@christophe-f8 Sure, I can look at merging this in my branch.

As this is meant to be a portable library I'll have to check that the same behaviour is also implemented in other environments.

samangh avatar Mar 07 '24 11:03 samangh

@christophe-f8, your suggested changes are now in my fork.

samangh avatar Mar 10 '24 22:03 samangh