search

ncruces / zenity

Zenity dialogs for Golang, Windows, macOS
https://pkg.go.dev/github.com/ncruces/zenity
MIT License
736 stars 37 forks source link

Use IFileDialog where available. #91

Closed ncruces closed 9 months ago

ncruces commented 9 months ago

Alternative fix for #89.

ncruces commented 9 months ago

Thinking of merging this regardless, wdyt @lonnywong?

lonnywong commented 9 months ago

Thinking of merging this regardless, wdyt @lonnywong?

Sorry for the delay. It doesn't work on that machine. It may fall back to GetOpenFileName, I'd like to try debugging to confirm, but he's not free recently.

ncruces commented 9 months ago

That's disappointing. If you can help debug, it'd be great. I'll test more just to see if this big change isn't a regression. And I'll probably merge it too.

lonnywong commented 9 months ago

@ncruces It crashes without falling back to GetOpenFileName. Should we use the solution https://github.com/ncruces/zenity/pull/89/commits/84d5fe5fb3c2a724ce313d340487a01300e1cc8d ? It works with the patch.

ncruces commented 9 months ago

If we're going to leave COM initialized, windows can do it for us.

When it returns S_OK (nil), we leave it initialized. When it returns S_FALSE, it was already initialized, so we can CoUninitialize (drop the ref count back to what it was). When it returns RPC_E_CHANGED_MODE, we asked it to change mode, but we don't care about the mode.

Otherwise, it's an error.

lonnywong commented 8 months ago

@ncruces Thanks.

https://github.com/ncruces/zenity/pull/89 has been fixed by https://github.com/ncruces/zenity/commit/99f1a258bcfc7eaa8b3a90332e51ae06f8768454: https://github.com/ncruces/zenity/blob/99f1a258bcfc7eaa8b3a90332e51ae06f8768454/file_windows.go#L453-L472

As long as the reference count does not decrease to 0, the bug will not be triggered.

ncruces commented 8 months ago

That was the expectation, great to see it confirmed! It just felt better to leave it to windows to manage the thread local stuff.