Skip to content

gh-85604: Check IDLE theme colors at startup - #157620

Open
serhiy-storchaka wants to merge 2 commits into
python:mainfrom
serhiy-storchaka:gh-85604-bad-theme-color
Open

serhiy-storchaka wants to merge 2 commits into
python:mainfrom
serhiy-storchaka:gh-85604-bad-theme-color

Conversation

@serhiy-storchaka

Copy link
Copy Markdown
Member

A typo in a color in ~/.idlerc/config-highlight.cfg, such as keyword-foreground = bpo-00224, crashed the Settings dialog with TclError: unknown color name "bpo-00224" when the Highlights page painted its sample, so the theme could not be fixed from the GUI. Editor windows did not crash only because WidgetRedirector silently swallows TclError, leaving the element uncolored without any warning.

Colors can only be validated by Tk, so the check is done in pyshell.main() right after the root is created and before any window uses the theme. check_theme_colors() validates every color of every user theme with winfo_rgb(), prints a warning for each invalid one (in the style of #152747) and removes it from the loaded configuration, so that GetThemeDict() supplies the default color instead. The user file is not modified until the settings are saved.

Missing theme elements were already handled by GetThemeDict().

🤖 Generated with Claude Code

An invalid color in a user highlight theme crashed the Settings dialog.
Now it is ignored with a warning and the default color is used instead.
@serhiy-storchaka serhiy-storchaka added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels Sep 16, 2026
@terryjreedy

terryjreedy commented Sep 16, 2026

Copy link
Copy Markdown
Member

Tests / Windows / Build and test (Win32, switch-case) (pull_request)Failing after 18m
test altered the execution environment (env changed):�[0m
2026-09-16T13:58:03.3484411Z test.test_asyncio.test_sslproto:
2026-09-16T13:58:03.3484912Z uncaught thread exception (AssertionEr
rerunning
EDIT successful!

@terryjreedy terryjreedy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

check_theme_color and its test should be in config.py, along with other config checking code, and invoked as part of the config checking process. The error messages should by added to the file_load_error_error message rather than sent to stderr to, usually, be lost.

pyshell itself would not need to be touched. EDIT I otherwise like this.

In the long run, all error messages after tkinter is successfully loaded should be sent to a new error message window (outwin or text window) that does not block continuation and that allows a user to save or print and that remains until a user deletes it.

@bedevere-app

bedevere-app Bot commented Sep 16, 2026

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@serhiy-storchaka

Copy link
Copy Markdown
Member Author

Done. The check is now IdleConf.check_highlight(root) in config.py, and the invalid colors are reported together with unreadable files by config_error_message(root), which replaces file_load_error_message(). Since colors can only be checked with Tk, loading (on import) and checking (after the root is created) are two stages, so pyshell.main() still has to pass the root, but that is the only change there.

I have made the requested changes; please review again

@bedevere-app

bedevere-app Bot commented Sep 16, 2026

Copy link
Copy Markdown

Thanks for making the requested changes!

@terryjreedy: please review the changes made to this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting change review needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants