Skip to content

Utils: fall back when a Linux file dialog errors#6274

Open
patrickwehbe wants to merge 1 commit into
ArchipelagoMW:mainfrom
patrickwehbe:fix/linux-file-dialog-fallback
Open

Utils: fall back when a Linux file dialog errors#6274
patrickwehbe wants to merge 1 commit into
ArchipelagoMW:mainfrom
patrickwehbe:fix/linux-file-dialog-fallback

Conversation

@patrickwehbe

Copy link
Copy Markdown
Contributor

What is this fixing or adding?

Fixes #5991.

On Linux the native file pickers open_filename, save_filename and open_directory shell out to kdialog or zenity via _run_for_stdout, which reads only the helper's stdout and discards its exit code. These helpers exit 0 when the user picks a path, 1 when the user cancels, and >= 2 on an error (e.g. a missing display, a malformed argument, or the binary failing to start).

Because the exit code was thrown away, an errored helper returned empty stdout that looked identical to a user cancellation: the picker silently returned None, the error was swallowed, and the existing tk fallback was never reached. The user just saw "nothing happened".

This PR adds a small _run_file_dialog helper next to _run_for_stdout that interprets the exit code and returns (handled, selection):

  • exit 0 -> (True, <path>) -- a selection was made;
  • exit 1 -> (True, None) -- the user cancelled (conclusive, do not fall back);
  • any other exit code -> logs a warning and returns (False, None) so the caller falls through to the next backend.

Each of the three pickers now replaces its return _run_for_stdout(...) calls for both kdialog and zenity with a fall-through pattern that returns only when the helper reports a conclusive result. A broken kdialog therefore now correctly falls back to zenity, and then to tk. messagebox is intentionally left unchanged since it is not a file picker.

How was this tested?

  • Added test/general/test_utils.py with mock-based tests that patch Utils.is_linux, Utils.env_cleared_lib_path, shutil.which and subprocess.run to exercise the Linux branch on any OS:
    • exit 0 returns the chosen path and never consults the next backend;
    • exit 1 (user cancel) returns None and does not fall back;
    • exit 2 (tool error) falls back to the next backend (zenity), which then succeeds.
  • python -m pytest test/general/test_utils.py -q -> 3 passed.
  • python -m py_compile Utils.py -> clean.
  • python -m flake8 --max-line-length=120 Utils.py test/general/test_utils.py -> no new warnings on the changed lines.

On Linux the native file pickers (open_filename, save_filename and
open_directory) shell out to kdialog or zenity and read only the helper's
stdout, ignoring its exit code. kdialog/zenity exit 0 on a selection, 1 on a
user cancel, and >=2 on an error. Because the exit code was discarded, an
errored helper produced empty stdout that was indistinguishable from a user
cancel, so the dialog silently returned None and the tk fallback was never
reached.

Add a _run_file_dialog helper that interprets the exit code and returns
(handled, selection): handled is True for a selection (0) or user cancel (1),
and False on any error so the caller falls through to the next backend. The
three pickers now stop only when the helper reports a conclusive result; a
broken kdialog now correctly falls back to zenity and then tk. messagebox is
left untouched since it is not a file picker.

Fixes ArchipelagoMW#5991.
@github-actions github-actions Bot added affects: core Issues/PRs that touch core and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. labels Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

affects: core Issues/PRs that touch core and may need additional validation. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Linux save_filename assumes successful KDialog/Zenity result

1 participant