IDLE: Cleanup config code

[terryjreedy]

BPO	27452
Nosy	@terryjreedy, @serhiy-storchaka
PRs	bpo-27452: IDLE: Cleanup config.py code #14577 [3.8] bpo-27452: IDLE: Cleanup config.py code (GH-14577) #14802 [3.7] bpo-27452: IDLE: Cleanup config.py code (GH-14577) #14803
Files	config-clean.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/terryjreedy'
closed_at = <Date 2019-07-16.21:32:41.991>
created_at = <Date 2016-07-04.20:09:51.024>
labels = ['3.8', 'expert-IDLE', 'type-feature', '3.7', '3.9']
title = 'IDLE: Cleanup config code'
updated_at = <Date 2019-07-16.21:32:41.984>
user = 'https://github.com/terryjreedy'

bugs.python.org fields:

activity = <Date 2019-07-16.21:32:41.984>
actor = 'terry.reedy'
assignee = 'terry.reedy'
closed = True
closed_date = <Date 2019-07-16.21:32:41.991>
closer = 'terry.reedy'
components = ['IDLE']
creation = <Date 2016-07-04.20:09:51.024>
creator = 'terry.reedy'
dependencies = []
files = ['43623']
hgrepos = []
issue_num = 27452
keywords = ['patch']
message_count = 13.0
messages = ['269809', '269852', '269855', '269856', '269859', '269864', '269865', '269870', '348040', '348041', '348043', '348044', '348045']
nosy_count = 3.0
nosy_names = ['terry.reedy', 'python-dev', 'serhiy.storchaka']
pr_nums = ['14577', '14802', '14803']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue27452'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

[terryjreedy]

The config module code can be improved even without changing behavior. While working on bpo-27380, I noticed these two:

IdleUserConfParser.RemoveFile is a simple one-liner called once. Put it inline.

IdleConf.CreateConfigHandlers: A) As near as I can tell, __file__ is now always the absolute path of the file regardless if executed directly or imported. B) Creating two dicts of file names that are immediately used and deleted is useless and to me makes the code less readable.

I intend these changes only for 3.6 and will not apply until after the new unix keys patch. Serhiy, have you noticed any similar cleanups that would be appropriate to add?

[serhiy-storchaka]

If config.py is just executed, __file__ can be a relative path.

RemoveFile() looks as a part of public API. If you are sure that nobody needs to remove a config file, you can remove this method.

[terryjreedy]

How can you get a relative path in 3.6 (or other current Python)? To test, I wrote file.py containing "print(file)". Executing from IDLE, with "python path/to/file.py", "path/to/file.py", "file.py" in the path/to directory, and "python -m to.file" (where path is on sys.path) all resulted in the absolute path. After adding "input()" to suspend execution, double clicking in Explorer gives the same result. Have I forgotten something? Do any of these result is something different on Linux? (or Mac?, for that matter)

The only reason to execute rather than import config.py is to run the test at the end of the file after editing the file. In the absence of a thorough automated test, I occasionally do so.

The approximately 500 lines of output is too much to read in detail (although one might check one specific part), but that it runs and produces the same number of lines before and after a change is reassuring. I should add a line counter and checksum to the dump function.

As for removing RemoveFile: idlelib in 3.6 is, with a few exceptions, a private API.

[serhiy-storchaka]

$ ./python -m file
/home/serhiy/py/cpython/file.py
$ ./python file.py
file.py

[python-dev]

New changeset da83e115afea by Terry Jan Reedy in branch '2.7':
Issue bpo-27452: add line counter and crc to IDLE configHandler test dump.
https://hg.python.org/cpython/rev/da83e115afea

New changeset 127569004538 by Terry Jan Reedy in branch '3.5':
Issue bpo-27452: add line counter and crc to IDLE configHandler test dump.
https://hg.python.org/cpython/rev/127569004538

New changeset c2e21bc83066 by Terry Jan Reedy in branch 'default':
Issue bpo-27452: add line counter and crc to IDLE config test dump.
https://hg.python.org/cpython/rev/c2e21bc83066

[python-dev]

New changeset adbeef96ae95 by Terry Jan Reedy in branch 'default':
Issue bpo-27452: make command line idle-test> python test_help.py work.
https://hg.python.org/cpython/rev/adbeef96ae95

[terryjreedy]

Ah, the invocation I did not test ;-). It does not matter in this case because "os.path.dirname('config.py')" is '' and the join leaves relative names for the config files that work fine for opening them.

F:\Python\dev\36\Lib\idlelib>..\..\pcbuild\win32\python_d.exe config.py

{'keys': <main.IdleConfParser object at 0x000002633B200080>,
...

At one time, I thought, sys.path[0] was '', representing the current directory, and not the absolute path of the starting directory. I am not sure if there are any cross platform, cross implementation, guarantees.

There are 12 other idlelib files using __file__. I ran each in either idlelib or idle_test as appropriate. 11 run. test_help.py fails at this line (which I wrote)
helpfile = join(abspath(dirname(dirname(file))), 'help.html')
as the double dirname does not have the expected effect. A version of the conditional from config, with dirname(sys.path[0]), would work. However, taking the abspath first is easier.
helpfile = join(dirname(dirname(abspath(file))), 'help.html')

The comment in config appears to refer to the exec command/function. I don't know what either does with __name__, __file__, or sys.path.

---
With the two IdleConf dict keys sorted, the first patch results in consistent output. configparser.ConfigParser uses OrderedDicts by default, so re-running on unchanged files results in unchanged iteration order.

[serhiy-storchaka]

In this case it works, because os.join('', filename) returns filename.

[terryjreedy]

New changeset f8d4cc7 by Terry Jan Reedy (Cheryl Sabella) in branch 'master':
bpo-27452: IDLE: Cleanup config.py code (GH-14577)
f8d4cc7

[terryjreedy]

As near as I can tell, 'python file.py' results in absolute __file__ in 3.9 versus still relative in 3.8.

[terryjreedy]

New changeset 178f09f by Terry Jan Reedy (Miss Islington (bot)) in branch '3.8':
bpo-27452: IDLE: Cleanup config.py code (GH-14577) (GH-14802)
178f09f

[terryjreedy]

New changeset efd23a1 by Terry Jan Reedy (Miss Islington (bot)) in branch '3.7':
bpo-27452: IDLE: Cleanup config.py code (GH-14577) (GH-14803)
efd23a1

[terryjreedy]

Thanks for preparing the PR.