Openshot-qt: Preferences Refuse To Open When Blender Path Is Set

[MrTeeXD]

Describe the bug
When you set the path for Blender and then try to open the Preferences, it wont open at all

System Details (please complete the following information):

• Operating System / Distro: [Windows 10]
• OpenShot Version [2.4.3 Latest Daily Build]

To Reproduce
Steps to reproduce the behavior:

1. Go to Preferences
2. Click on the Browse Button
3. See error by trying to open the Preferences Menu

Expected behavior
When the path is set, the Preferences will no longer refuse to open

Screenshots
If applicable, add screenshots to help explain your problem.

Logs
If you are experiencing a crash, please collect and attach logs of the problem.

Additional context
This is a serious Bug that needs attention

[DylanC]

@MisterTeeXD - Well the daily build is normally more buggy than the stable version but since I did work on this feature I will have a look at this issue soon.

[DylanC]

@MisterTeeXD - Confirmed, thanks.

exceptions:ERROR Unhandled Exception
Traceback (most recent call last):
File "C:\Program Files\OpenShot Video Editor\windows\preferences.py", line 265, in selectExecutable
widget.setText(fileName)
TypeError: setText(self, str): argument 1 has unexpected type 'tuple'

@ferdnyc - This code I submitted worked on Linux but not Windows. Any ideas? Related to PR 2255.

[MrTeeXD]

Thank you, i was wondering when my issues are getting a reply

[ferdnyc]

I'm guessing it has to do with the fact that file paths are UTF-16 on Windows. It might be better to convert it instead:

widget.setText(filename.toUtf8())

That should be safe on all platforms, worst case it's a no-op on Linux and macOS.

[ferdnyc]

Oh, nope, I'm wrong. The answer is in main_window.py:

https://github.com/OpenShot/openshot-qt/blob/389eb1815afc382a9385b2bd2c16222c0fb7799d/src/windows/main_window.py#L533-L535

Seems getOpenFileName() does return a tuple. So, you should take only the first of the two returned values as the fileName.

The other thing is, both "Select executable file" and "All Files (*)" should really be translatable. So:

        fileName, fileType = QFileDialog.getOpenFileName(self,_("Select executable file"),
QDir.rootPath(),_("All Files (*)"))

[ferdnyc]

Huh. I wonder why the code snippet didn't embed?

[ferdnyc]

The thing that annoys me is, even their _own_ Python documentation is very oblique about this. The sample code reads just like the C++ docs, misleadingly so, and only the "Return type" intimates that it'll be returning a tuple.

As to _why_ it returns a tuple, the explanation has to be found outside of official Qt and/or PyQt documentation entirely:

• At the Qt forums
• At the PySide bugzilla

[DylanC]

Seems getOpenFileName() does return a tuple. So, you should take only the first of the two returned values as the fileName.

Oh man. This is what I get for assuming getOpenFileName() works the same as the C++ Qt. 😆

The other thing is, both "Select executable file" and "All Files (*)" should really be translatable

I actually did try this because I also do this in C++ with tr("example str") all the time. However, it kept complaining that it didn't recognize the underscore piece of this Eg. _("example str").

Strange why this bug didn't appear on Linux though. I'm a bit confused on that aspect. I will get on those issues asap anyway since I did cause this issue. 👍

[ferdnyc]

I'm _guessing_ maybe it's that on linux, passing a (utf8_str, utf8_str) tuple into setText() automatically just ignored the extra arg, because the first one is the right encoding and everything's probably varargs behind the scenes. But on win32 the tuple is (utf16_str, utf8_str) so that pissed off the SWIG bridge to the Qt libs.

I actually did try this because I also do this in C++ with tr("example str") all the time. However, it kept complaining that it didn't recognize the underscore piece of this Eg. _("example str").

*nod* It doesn't exist initially. Each method in the file that wants to use translation needs to import the translation hook. __init__() does it on lines 64-65, reject() on line 375. You just need to toss a

_ = get_app()._tr

at the start of def selectExecutable(), then you can use the wrapper to mark strings translatable.

[ferdnyc]

(I'm guessing it's a scope issue, since even if it's just a reference, the function handle has to live somewhere. In C/C++ the translation wrappers are always macros, so they don't exist in memory in any real form. But Python doesn't have macros, so it's gotta be an in-memory reference with an allocation and scope.)

[DylanC]

I'm guessing maybe it's that on linux, passing a (utf8_str, utf8_str) tuple into setText() automatically just ignored the extra arg, because the first one is the right encoding and everything's probably varargs behind the scenes. But on win32 the tuple is (utf16_str, utf8_str) so that pissed off the SWIG bridge to the Qt libs.

Wow, mind = blown. Seems to make perfect sense from what you say. 👍

You just need to toss a _ = get_app()._tr at the start of def selectExecutable(), then you can use the wrapper to mark strings translatable.

Yeah I figured it was something like a scope issue. I just thought that this would be setup in a way to be used across the whole file and not need to be declared again inside functions.

[DylanC]

@ferdnyc - Merged in PR 2309 for these issues. Hopefully that will be it for this feature. I don't want any more trouble. :rofl:

[ferdnyc]

Yeah I figured it was something like a scope issue. I just thought that this would be setup in a way to be used across the whole file and not need to be declared again inside functions.

*nod* Or even across the entire application (as the Python gettext docs suggest — though of course, this isn't gettext, it's QTranslator. The local scoping may have something to do with how context is extracted when building the translation files. Perhaps it's necessary/preferable so that each string can be tagged as to where in the call stack _() was declared. ¯\_(ツ)_/¯

[ferdnyc]

Strange why this bug didn't appear on Linux though. I'm a bit confused on that aspect. I will get on those issues asap anyway since I did cause this issue.

I (accidentally) played with this some more, running from source in a not-fully-updated local branch... while this bug didn't _crash_ OpenShot on Linux, it wasn't actually _working_ either. The QLineEdit contents never updated, when setText() was being accidentally passed a tuple — the call was just silently ignored. Which is maybe even stranger than crashing / working, honestly. (It's certainly less helpful!)

[DylanC]

@MisterTeeXD - This should be fixed now. Seems to be working as expected on Windows 10.

[MrTeeXD]

is the fix gonna come out?

[DylanC]

@MisterTeeXD - The fix is in the latest daily build.

Edit: Oh I think you will need to remove the Preferences folder too.
Location on Windows - C:\Users\USERNAME.openshot-qt

[MrTeeXD]

Nope, it has not been fixed, i have the latest daily build already

[DylanC]

@MisterTeeXD - Did you delete the preferences folder first?

[MrTeeXD]

No

[DylanC]

@MisterTeeXD - You need to delete the preferences folder, otherwise it will not work. 😄

[MrTeeXD]

Where do i find that :D

[DylanC]

@MisterTeeXD - Location on Windows - C:\Users\USERNAME.openshot-qt

[MrTeeXD]

Success, it worked, now my production skills can increase now, thank you :)

[DylanC]

@MisterTeeXD - You are very welcome! 👍