Title: IDLE: Cleanup config code
Type: enhancement Stage: test needed
Components: IDLE Versions: Python 3.6
Status: open Resolution:
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: python-dev, serhiy.storchaka, terry.reedy
Priority: normal Keywords: patch

Created on 2016-07-04 20:09 by terry.reedy, last changed 2017-06-19 18:44 by terry.reedy.

File name Uploaded Description Edit
config-clean.diff terry.reedy, 2016-07-04 20:09 review
Messages (8)
msg269809 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-07-04 20:09
The config module code can be improved even without changing behavior. While working on #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?
msg269852 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-07-05 20:55
If 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.
msg269855 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-07-05 22:07
How can you get a relative path in 3.6 (or other current Python)? To test, I wrote containing "print(__file__)".  Executing from IDLE, with "python path/to/", "path/to/", "" 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 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.
msg269856 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-07-05 22:24
$ ./python -m file
$ ./python
msg269859 - (view) Author: Roundup Robot (python-dev) Date: 2016-07-06 00:12
New changeset da83e115afea by Terry Jan Reedy in branch '2.7':
Issue #27452: add line counter and crc to IDLE configHandler test dump.

New changeset 127569004538 by Terry Jan Reedy in branch '3.5':
Issue #27452: add line counter and crc to IDLE configHandler test dump.

New changeset c2e21bc83066 by Terry Jan Reedy in branch 'default':
Issue #27452: add line counter and crc to IDLE config test dump.
msg269864 - (view) Author: Roundup Robot (python-dev) Date: 2016-07-06 01:52
New changeset adbeef96ae95 by Terry Jan Reedy in branch 'default':
Issue #27452: make command line idle-test> python work.
msg269865 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-07-06 01:55
Ah, the invocation I did not test ;-).  It does not matter in this case because "os.path.dirname('')" is '' and the join leaves relative names for the config files that work fine for opening them.


 {'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. 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.
msg269870 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-07-06 07:59
In this case it works, because os.join('', filename) returns filename.
Date User Action Args
2017-06-19 18:44:33terry.reedysetcomponents: + IDLE
2016-07-06 07:59:07serhiy.storchakasetmessages: + msg269870
2016-07-06 01:55:37terry.reedysetmessages: + msg269865
2016-07-06 01:52:17python-devsetmessages: + msg269864
2016-07-06 00:12:03python-devsetnosy: + python-dev
messages: + msg269859
2016-07-05 22:24:08serhiy.storchakasetmessages: + msg269856
2016-07-05 22:07:47terry.reedysetmessages: + msg269855
2016-07-05 20:55:28serhiy.storchakasetmessages: + msg269852
2016-07-04 20:09:51terry.reedycreate