Pre-populate file dialogs with sensible defaults

[nabijaczleweli]

Example:

Tested on Windows only, with the assumption that other platforms' Platform::FileDialog behaves the same.

Ref: #538

[whitequark]

Thanks for the PR! I'm going to test it on Linux a bit later and merge.

[whitequark]

Could you please test it on Windows with Explorer configured to show file extensions?

[nabijaczleweli]

This does indeed look wrong and prompts to override uwu.slvs on save, so not just visual difference:

Passing pp.WithExtension("png/csv/&c.") to SetFilename() should solve this I think? Will amend in a bit.

(Closed the PR by accident, sorry!)

[whitequark]

Passing pp.WithExtension("png/csv/&c.") to SetFilename() should solve this I think?

That won't work quite right either because the file type selector is remembered between uses of a dialog, so the extension should actually be taken from the selector. I think this might need a new method, something like SuggestFilename().

[nabijaczleweli]

Two changes:

• I made Path::SetExtension("") strip the extension entirely (as I feel would be the expected behaviour), and
• replaced the SetFilename() calls with a new SuggestFilename() function, which calls SetFilename() with a stripped extension, which behaves like I'd expect on Windows (but does it do the same on other GUI platforms?):
  

Will rebase the commits around to be more presentable and update the messages after testing to reduce noise.

[nabijaczleweli]

I've managed to build SolveSpace on my laptop, and the GTK backend doesn't seem to honour the SetFilename()/gtkChooser->set_filename() call at all?

This working diff:

diff --git a/src/platform/guigtk.cpp b/src/platform/guigtk.cpp
index 73fb01e..81e0c0d 100644
--- a/src/platform/guigtk.cpp
+++ b/src/platform/guigtk.cpp
@@ -1242,7 +1242,11 @@ public:
     }
 
     void SetFilename(Platform::Path path) override {
+        printf("SetFilename1(%s -> %s)\n", gtkChooser->get_filename().c_str(), path.raw.c_str());
         gtkChooser->set_filename(path.raw);
+        // printf("SetFilename1(%s -> %s)\n", gtkChooser->get_filename().c_str(), path.FileStem().c_str());
+        // gtkChooser->set_filename(path.FileStem());
+        printf("SetFilename2(%s)\n", gtkChooser->get_filename().c_str());
     }
 
     void AddFilter(std::string name, std::vector<std::string> extensions) override {
@@ -1296,6 +1300,7 @@ public:
             return;
 
         Platform::Path path = GetFilename();
+        printf("FilterChanged(%s -> %s)\n", path.raw.c_str(), extension.c_str());
         SetCurrentName(path.WithExtension(extension).FileName());
     }
 
@@ -1311,10 +1316,12 @@ public:
     }
 
     void CheckForUntitledFile() {
+        printf("CheckForUntitledFile()1: %s\n", gtkChooser->get_current_name().c_str());
         if(gtkChooser->get_action() == Gtk::FILE_CHOOSER_ACTION_SAVE &&
                 Path::From(gtkChooser->get_current_name()).FileStem().empty()) {
             gtkChooser->set_current_name(std::string(_("untitled")) + "." + GetExtension());
         }
+        printf("CheckForUntitledFile()2: %s\n", gtkChooser->get_current_name().c_str());
     }
 };

Yields

nabijaczleweli@nabtop:~/src/solvespace/build$ bin/solvespace /tmp/owo.slvs

(solvespace:2628): dbind-WARNING **: 21:48:35.517: AT-SPI: Error retrieving accessibility
ded by any .service files
SolveSpace!

Missing (absent) translation for '&Helix'
Missing (absent) translation for 'Re&volve'
Missing (absent) translation for 'Show &Underconstrained Points'
Missing (absent) translation for file-type'PNG image'
FilterChanged( -> png)
SetFilename1( -> /tmp/owo)
SetFilename2()
CheckForUntitledFile()1: .png
CheckForUntitledFile()2: untitled.png

and likewise with the commented block being in.

[nabijaczleweli]

Quoting https://developer.gnome.org/gtkmm/stable/classGtk_1_1FileChooser.html#a35bbea274ddb0a7b948e6f23b70b9bed:

Sets filename as the current filename for the file chooser, by changing to the file’s parent folder and actually selecting the file in list; all other files will be unselected.

If the chooser is in Gtk::FILE_CHOOSER_ACTION_SAVE mode, the file’s base name will also appear in the dialog’s file name entry.

Note that the file must exist, or nothing will be done except for the directory change.

The documentation then goes on to reference an example, which seems to be missing from the header as well as the doc.

I don't really know what to do about this…

[whitequark]

the GTK backend doesn't seem to honour the SetFilename()/gtkChooser->set_filename() call at all?

It does. The problem is that the semantics of SetFilename are not what you think: that function selects an existing file. That's the reason I asked you to make a separate SuggestFilename function, which would not require that.

[nabijaczleweli]

Right, I made SuggestFilename() fully virtual and copied the implementation over to the platform impls. The GTK one is marked TODO for now as it clearly doesn't work. I don't think I can test the Cocoa one, but a glance at the documentation suggests it should.

[phkahler]

@nabijaczleweli where do we stand with this one?

[nabijaczleweli]

Rebased and fixed on Win32. See TODO in guigtk.cpp and discussion above for Linux, I don't have an understanding of GTK or currently a way to port to and test there, unfortunately. No idea about Darwin, but, again, docs say it should be fine(?).

[phkahler]

I can test on Linux/GTK, but it will probably have to wait until tomorrow. Fixing it if it isn't right is another story... So you have OSX and Windows covered?

[nabijaczleweli]

I wrote this on Windows so that should be fine. The OSX implementation is a stub, which should work going by the Cocoa docs, but I don't even know if it builds, let alone funxions.

[phkahler]

I tried this on Linux / GTK and it doesn't solve the problem there. The suggested export filenames I get remain the same as before: "untitled.stl" or "untitled.step".

[nabijaczleweli]

Indeed, the GTK implementation does need to be written.

[phkahler]

@vespakoen and @ruevs can you guys have a look at this and possibly #545 and #650. Somewhere in these 3 is a good thing - I think mostly in this one.

[ruevs]

@phkahler As a start I rebased it on master.

Builds and runs on Windows. All the Export options offer a file name the same as the opened .slvs and with the correct extension.

Now I need to look at the actual code and what it does...

[phkahler]

On Linux I tried:

    void SuggestFilename(Platform::Path path) override {
//        SetFilename(path.WithExtension(""));  // TODO
        gtkChooser->set_current_name(path.FileStem());
    }

But that suggests the file name with no extension at all. The original commented out give unknown.xxx where xxx is the correct extension. Some of these function names make little sense ;-)

[phkahler]

OK, this looks like it works with GTK:

        gtkChooser->set_current_name(path.FileStem()+"."+GetExtension());

[phkahler]

@vespakoen can you verify/fix the macOS version of this?