classification
Title: IDLE: gracefully handle themes (or keysets, or ...) not present
Type: enhancement Stage: resolved
Components: IDLE Versions: Python 3.6, Python 3.4, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: kbk, markroseman, python-dev, roger.serwy, terry.reedy
Priority: normal Keywords: patch

Created on 2015-10-04 18:23 by markroseman, last changed 2015-11-12 20:38 by terry.reedy. This issue is now closed.

Files
File name Uploaded Description Edit
write-new-defaults.patch markroseman, 2015-10-04 19:46 review
missingtheme.patch markroseman, 2015-10-06 00:22 review
@cfg_text.diff terry.reedy, 2015-11-04 09:28 review
Messages (12)
msg252278 - (view) Author: Mark Roseman (markroseman) * Date: 2015-10-04 18:23
As a follow-on to #24820, when a particular theme is selected in the configuration files, but that theme is not available, IDLE will print out a bunch of warning messages on console. That can occur for example when using a newer built-in theme in an older version before that theme was added, or if the user highlight config file was modified or removed.

IDLE should be smart enough to recognize that and switch to a default theme, without dumping out console error messages. May alert user to that case with an appropriate error message, e.g. theme xyz not found, using default.

The current workaround in #24820 (when a newer theme is chosen in a new version, explain it may not work in older version but explain how it could be made to) is unfortunately necessary unless a more clever solution for this backwards compatibility issue can be found.

Would apply to keysets, etc. in the same was as themes.
msg252282 - (view) Author: Mark Roseman (markroseman) * Date: 2015-10-04 18:56
Actually, I think we may be able to get away without the warning message when you select a 'new' theme, and still maintain backwards compatibility.

For 'new' themes (i.e. IDLE Dark and any more builtins we add in the future), we write the theme out to the user configuration file, keeping the same name, and perhaps adding an extra field, i.e. 'builtin=1', so that we can recognize it in newer versions, along with a comment if possible.

The existing code looks up theme names by checking if its in the default list, and if not, looks in the user list. For older versions without IDLE Dark, it will then find it in the user list.

Besides writing out the theme to user config file, the only other change that would be needed in the code is to modify GetSectionList so that when it's looking in a user config file, it doesn't include sections that either have the builtin=1 from above, or just have the same name as things in the default file.
msg252283 - (view) Author: Mark Roseman (markroseman) * Date: 2015-10-04 19:46
Patch write-new-defaults.patch attached so that we write 'newer' default themes to config-highlight.cfg if selected, and ignore them if we already have a default by that name.
msg252292 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2015-10-05 00:26
There over-arching issue is that IDLE was not built to allow addition of new builtin themes.  IDLE New is currently identical to IDLE Classic, so maybe that was intended to allow a revision of IDLE Classic (and I have an idea for that), but IDLE Dark is too drastically different to have added it as IDLE New. There are two sub-issues here.

1. Theme loading: I experienced the multiple error messages to console behavior once, but only once.  This is the 'or worse' in the new message.  I could not find out why only once, or know that the same would be true on other machines. If possible, there should only be one message. Whether it goes to console or messagebox is a different issue, part of revisiting the configure error message system.

The first fallback should be to existing and accessible builtins, namely IDLE Classic, rather than a default of no colors.  This strikes me as a buggy design.

2. New theme addition and selection: This has to take into account that we cannot patch existing releases and that all IDLEs used by one user use the same user configuration files.  (There is an issue post that suggests maybe changing this, but I cannot find it now.)  This is a minus in that we cannot currently restrict selection of IDLE Dark to new IDLEs (but see below).  This is a plus in that IDLE Dark can be 'backported' by saving as a custom theme.  We may always need some sort of message.

I thought about automatically saving as a custom theme, but besides the presumed deadline for 3.4.4 (see below), there are potential problems. A) It is magical and potentially confusing enough that I might want a message anyway. B) It may not be possible (no writable .idlerc).  I am not sure what happens if not. C) If we write with the same name, 'IDLE Dark', then two things have the same name but may or may not be identical. (Users could modify the custom version of IDLE Dark.)  This is why the interface demands a new name. Your patch assumes that they stay identical. D) If we write as a different name, users need to be notified anyway.  Might as well let them choose, so they remember. E) A custom version is not needed for students in a class with only one IDLE on the machine. F) We would have to look at possible interactions with other parts of the dialog's behavior.

"See below", time pressure: I though 3.4.4c1 was due out today.  It has not appeared, and there have been no pydev messages for 20 hours, since I posted an enquiry out the release.  If the release is a week away, we can potentially change the patch, though I would want to test very carefully.

"See below", date-specific user config: because all releases after today will have IDLE Dark, the needed marker is date rather than version.  Right now, selecting 'IDLE Dark' as a built-in theme results in user config-main.cfg having this section.

[Theme]
default = True
name = IDLE Dark

Suppose instead the result were

[Theme]
default = True
name2 = IDLE Dark
#or
[Theme]
default = True
name = IDLE New
name2 = IDLE Dark

I tested with 2.7.10 and it starts fine with either of the above, and just as important, writes the above when Classic or New are selected if the name2 line is there.  I expect the both should be true of all versions that recognize user config files, especially the startup part.

The challenge would be to have new IDLEs add the name2 line when IDLE Dark is selected and have it replace name with name2 when fetching the name.  The message could then be something to the effect of "IDLE Dark only comes with IDLE releases in Oct, 2015 and later.  To make it available to older releases, save it as a custom theme (with a new name)."

This is a bit clumsy, and would have to be generalized for further theme (but YAGNI for now), but would eliminate errors and no color with old versions.  What do you think?
msg252301 - (view) Author: Mark Roseman (markroseman) * Date: 2015-10-05 04:01
I see the 3.4.4 is not an immediate concern, so that's good.

FYI, I get the (multiple) error messages on console consistently on Mac, but it probably depends how it was launched. Agree the code for future versions should do a better job to detect the theme isn't present and switch back to a default theme rather than just puking out error messages as it tries to get each element of the non-existent theme. :-)

I still think the approach I took may be viable. It doesn't require changes to old versions, and relies on the fact that a user theme with the same name as a default theme is effectively hidden by the default theme. 

If a 'new' theme is selected, it's always written out to config-highlight, so it will be present for older versions (not errors and the internal default colors). Given that, I need help understanding what the 'name' vs. 'name2' etc. solution gives us?

(Not being able to write .idlerc wouldn't be an issue, because then you wouldn't be able to write out the change to say that is the theme to use anyway).

The only drawback I see is that a user running a previous version can customize IDLE Dark and those changes would not appear when using a newer versions of IDLE that had IDLE Dark as a default.

This would negatively affect only those users who change to IDLE Dark, actually modify the theme, and switch back and forth between versions of IDLE. The consequences of this occurring are only that the changes don't show up. In other words, I feel this affects a very tiny number of people, in a relatively minor way.

The existing warning on choosing themes will be encountered by anyone who might switch themes in the new version, and particularly for the non-hardcore, would be at the least surprising if not confusing or alarming. I think it's safe to assume that far more people try existing themes vs. create custom themes and use multiple versions of IDLE.

Given the very minor consequences and how few people it would affect, vs. how many people would encounter that warning and the impression it would leave, this seems a case of I'd rather be unobtrusive and 98+% correct then obtrusive and 100% correct. 

If it were my product, there's no question I'd drop the warning under those circumstances, and if it were a choice between including the warning or not including a new theme, I'd personally choose to not include the new theme. 

Anyway, it seems we have time to think more about this...
msg252370 - (view) Author: Mark Roseman (markroseman) * Date: 2015-10-06 00:22
On a more direct note, have attached missingtheme.patch, which ensures that all queries for the active theme go through CurrentTheme(), which has been modified to verify that the theme exists; if not, it returns 'IDLE Classic'
msg252372 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2015-10-06 01:12
Schedule update: 3.4.4 will be delayed untill after 3.5.1, which will be much sooner than the original minimum of 4 months after 3.5.0.  Larry promises at least two weeks notice before 3.5.1rc1 but will give no hint of when notice might be given.  So we have at least two week to do something correct.

I am against duplicating names because it is wrong in principle.  On the other hand, I understand the objection to a popup always and forever.  It is past time anyway to enable a working [Help] button, #22726.  The current popup help text should go there as tab-specific text.  The only automatic notice on selecting a new builtin theme would be 'New theme, see Help', maybe in orange (warning color, but not emergency red ;-) added in the empty space below.

I believe one new 'name2 = new_theme' addition to config-main would be enough for future additions.  We add to configHandler 'new_themes, a set, currently {'IDLE Dark'}.  If a selected builtin is in the set, the message is displayed and name2 is written.  On reading, if name2 is present but not recognized, revert to IDLE Classic, which is present on all systems now.

Intended result: all IDLEs keep working, new releases get a minimally intrusive notice pointing to a full explanation.
msg252441 - (view) Author: Mark Roseman (markroseman) * Date: 2015-10-07 00:00
Okay, that's reasonable enough for me.  :-) I'd still be for nuking the warning so that nothing gets displayed unless you go looking in online help.
msg254043 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2015-11-04 09:28
Here is patch with a subdued message to see Help when IDLE Dark is selected.  It goes away when something else is selected. I tried no emphasis, colored background, relief, and the dull red text in the patch.  I tested, but I want to retest systematically when fresh.  Test on *nix would good, though I do not think anything is system specific.

Under some conditions, entry 'new = ' is being put in user config-main.cfg [Theme] section. It seems harmless but I hope to track it down, while retesting, before committing.  It might just be something I did in my installed idlelib, but I'd like to know.
msg254044 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2015-11-04 09:30
I also want to document new Current Theme in docstring and maybe comments for future readers.
msg254553 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-11-12 20:25
New changeset b95c10cb457d by Terry Jan Reedy in branch '2.7':
Issue #25313: Change the handling of new built-in text color themes to better
https://hg.python.org/cpython/rev/b95c10cb457d

New changeset 6185c5603eed by Terry Jan Reedy in branch '3.4':
Issue #25313: Change the handling of new built-in text color themes to better
https://hg.python.org/cpython/rev/6185c5603eed
msg254556 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2015-11-12 20:38
The patch applied leaves 'name = IDLE New' alone so older IDLEs can use that instead of IDLE Classic as an alternative to IDLE Dark.  I will open an issue to revise IDLE New slightly.  I also added type='bool' when getting 'default' (the lack thereof lead to IDLE Dark being returned as a custom theme.

After working with this, I like the idea of present users with a single list of themes to select, instead of two buttons and two drop-down lists.

I tested fairly thoroughly, starting with no user [Theme] section.  When I added a print to debug idleConf.CurrentTheme, it was called 13 times on startup, with only Shell opened.
History
Date User Action Args
2015-11-12 20:38:32terry.reedysetstatus: open -> closed
resolution: fixed
messages: + msg254556

stage: resolved
2015-11-12 20:25:32python-devsetnosy: + python-dev
messages: + msg254553
2015-11-04 09:30:18terry.reedysetmessages: + msg254044
2015-11-04 09:28:22terry.reedysetfiles: + @cfg_text.diff

messages: + msg254043
versions: + Python 3.4
2015-10-07 00:00:29markrosemansetmessages: + msg252441
2015-10-06 01:12:10terry.reedysetmessages: + msg252372
2015-10-06 00:22:41markrosemansetfiles: + missingtheme.patch

messages: + msg252370
2015-10-05 04:01:30markrosemansetmessages: + msg252301
2015-10-05 00:27:01terry.reedysetmessages: + msg252292
2015-10-04 19:46:06markrosemansetfiles: + write-new-defaults.patch
keywords: + patch
messages: + msg252283
2015-10-04 18:56:39markrosemansetmessages: + msg252282
2015-10-04 18:23:48markrosemancreate