Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

current 0.9.5-snapshot of fstl only show *.STL and not *.stl files in the load GUI #73

Closed
mercurioblu opened this issue Apr 7, 2021 · 10 comments · Fixed by #92
Closed
Labels

Comments

@mercurioblu
Copy link

mercurioblu commented Apr 7, 2021

I am on a Debian SID PPC64 big endian and installed fstl from Debian repo (http://ftp.ports.debian.org/debian-ports/pool-ppc64/main/f/fstl/) as well as a freshly compiled version with sources downloaded from git.

While trying to load a file, the GUI in the version installed from Debian (version 0.9.4-1) only show stl files with a lower case extension, whereas the freshly compiled version (version 0.9.5-snapshot) only shows those with an upper case extension.

See image below, on the left the version installed from Debian, on the right the freshly compiled version.
On the bottom of the image a list of the two files contained in the currently selected folder by both fstl versions, one with a lower case extension, and one with an upper case extension.

If I click on the "*.stl" on the left, or on "STL files" on the right in the lower-right corner of the file dialog no other options shows up.

image

@mkeeter
Copy link
Collaborator

mkeeter commented Apr 8, 2021

Perhaps the comma between *.stl, *.STL is invalid syntax here – can you try removing it and seeing if that fixes it?

@sur5r
Copy link
Contributor

sur5r commented Apr 8, 2021

For me, it works both with and without the comma. But my file chooser looks different from the one in the screenshots. I assume there is some GTK/Qt interaction going wrong.

Searching the net, I found https://phabricator.kde.org/D17677 so maybe we can do something like

diff --git a/src/window.cpp b/src/window.cpp
index 0724dc5..6925b37 100644
--- a/src/window.cpp
+++ b/src/window.cpp
@@ -140,7 +140,7 @@ Window::Window(QWidget *parent) :
 void Window::on_open()
 {
     QString filename = QFileDialog::getOpenFileName(
-                this, "Load .stl file", QString(), "STL files (*.stl, *.STL)");
+                this, "Load .stl file", QString(), "STL files (*.[sS][tT][lL])", nullptr, QFileDialog::HideNameFilterDetails);
     if (!filename.isNull())
     {
         load_stl(filename);

to achieve a better result. The QFileDialog::HideNameFilterDetails option will hide the RegEx from the dropdown so it will only say "STL files".

It would help to know how @mercurioblu s Qt is configured to show the different file chooser to reproduce this issue.

@mercurioblu
Copy link
Author

@mkeeter Yes, removing the comma from https://github.com/fstl-app/fstl/blob/master/src/window.cpp#L143 works, now both files extensions uppercase and lowercase are correctly listed in file dialog. Cheers!

@mercurioblu
Copy link
Author

@sur5r Please, guide me (instructions for dummies I mean) on how to share with you my QT configuration. I use Debian SID with LXDE as desktop environment.

@sur5r
Copy link
Contributor

sur5r commented Apr 8, 2021

The LXDE hint should be enough, will try and reproduce to get a deeper understanding.

@DeveloperPaul123
Copy link
Member

DeveloperPaul123 commented Apr 8, 2021

Perhaps the comma between *.stl, *.STL is invalid syntax here – can you try removing it and seeing if that fixes it?

Should we push this change? Based on the Qt docs it seems that they indicate that no commas are needed. Or do we want to use sur5r's solution?

@sur5r
Copy link
Contributor

sur5r commented Apr 8, 2021

I will check the behavior under LXDE nevertheless. But I think makes sense to fix it now and maybe improve it later. The current status is obviously not correct. I have to admit I have no idea why I put the comma in there. I'm pretty sure I looked at the Qt docs back then as well...

@DeveloperPaul123
Copy link
Member

@sur5r If you have a patch already with the fix, could you file a PR? If not I can do it. And no worries on the comma 😄

@sur5r
Copy link
Contributor

sur5r commented Apr 8, 2021

I can file a PR in 2-3 hours, busy ATM. So feel free to commit it. I'm curious whether the RegEx version works with LXDE as well because this seems nicer IMHO.

@iperetta
Copy link

For me, it works both with and without the comma. But my file chooser looks different from the one in the screenshots. I assume there is some GTK/Qt interaction going wrong.

Searching the net, I found https://phabricator.kde.org/D17677 so maybe we can do something like

diff --git a/src/window.cpp b/src/window.cpp
index 0724dc5..6925b37 100644
--- a/src/window.cpp
+++ b/src/window.cpp
@@ -140,7 +140,7 @@ Window::Window(QWidget *parent) :
 void Window::on_open()
 {
     QString filename = QFileDialog::getOpenFileName(
-                this, "Load .stl file", QString(), "STL files (*.stl, *.STL)");
+                this, "Load .stl file", QString(), "STL files (*.[sS][tT][lL])", nullptr, QFileDialog::HideNameFilterDetails);
     if (!filename.isNull())
     {
         load_stl(filename);

to achieve a better result. The QFileDialog::HideNameFilterDetails option will hide the RegEx from the dropdown so it will only say "STL files".

It would help to know how @mercurioblu s Qt is configured to show the different file chooser to reproduce this issue.

Yes, this work for Fedora 36 and Qt 5.15.5-2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants