Skip to content

launcher: keep the cause when a config's ROM cannot be read#239

Merged
LinuxJedi merged 1 commit into
LinuxJedi:mainfrom
hobbo91:fix/rom-load-error
Jul 20, 2026
Merged

launcher: keep the cause when a config's ROM cannot be read#239
LinuxJedi merged 1 commit into
LinuxJedi:mainfrom
hobbo91:fix/rom-load-error

Conversation

@hobbo91

@hobbo91 hobbo91 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Loading a config naming a ROM that is not there showed "reading ROM " on the configuration screen's status line, which reads like progress rather than a failure. The status line formatted the error with Display, which prints only the outermost context, so the "No such file or directory" cause underneath was dropped. The full path then ran off the edge of the panel.

Format with {:#} so the whole chain reaches the status line, and shorten filesystem paths in it so the file name and its nearest directories stay visible:

Reading ROM .../roms/kick31.rom: No such file or directory (os error 2)

The Run path also logs the full chain now, so nothing is lost from the log.

The CLI already printed the path and the cause and is untouched.

Screenshot 2026-07-20 at 13 56 42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves launcher/configuration error reporting when a configured ROM can’t be read by preserving the underlying cause in the status line and shortening long filesystem paths so the message remains readable in the UI.

Changes:

  • Format launcher/config load errors with {:#} to retain the full error chain (including the OS “not found” cause).
  • Shorten filesystem paths in status messages so the filename and nearby directories remain visible.
  • Refactor path clipping to share a character-based helper between UI and status-line formatting, and add regression tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/video/window.rs Adds status-line error formatting improvements (full chain + path shortening) and logs full chained errors on run failures.
src/video/ui.rs Extracts a reusable character-count path clipping helper for use outside pixel-based UI layout.
src/video/window/tests.rs Adds tests covering chained error display and path-shortening behavior in status messages.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/video/window.rs
Comment thread src/video/window/tests.rs Outdated
Loading a config naming a ROM that is not there showed "reading ROM <path>" on
the configuration screen's status line, which reads like progress rather than a
failure. The status line formatted the error with Display, which prints only
the outermost context, so the "No such file or directory" cause underneath was
dropped. The full path then ran off the edge of the panel.

Format with {:#} so the whole chain reaches the status line, and shorten
filesystem paths in it so the file name and its nearest directories stay
visible:

    Reading ROM .../roms/kick31.rom: No such file or directory (os error 2)

The shortening reuses the printer output row's path clipper, split out as a
character-based helper, and applies to every path-bearing launcher error rather
than the ROM alone. The Run path also logs the full chain now, so nothing is
lost from the log.

The CLI already printed the path and the cause and is untouched.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@hobbo91
hobbo91 force-pushed the fix/rom-load-error branch from 6c6774e to c3bad11 Compare July 20, 2026 14:54

@LinuxJedi LinuxJedi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Many thanks!

@LinuxJedi
LinuxJedi merged commit 58c2b93 into LinuxJedi:main Jul 20, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants