Skip to content

bpo-30779: IDLE -- Factor ConfigChanges class from configdialog, put in config; test. #2612

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Jul 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 119 additions & 21 deletions Lib/idlelib/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def __init__(self, cfgFile, cfgDefaults=None):
"""
cfgFile - string, fully specified configuration file name
"""
self.file = cfgFile
self.file = cfgFile # This is currently '' when testing.
ConfigParser.__init__(self, defaults=cfgDefaults, strict=False)

def Get(self, section, option, type=None, default=None, raw=False):
Expand Down Expand Up @@ -73,7 +73,8 @@ def GetOptionList(self, section):

def Load(self):
"Load the configuration file from disk."
self.read(self.file)
if self.file:
self.read(self.file)

class IdleUserConfParser(IdleConfParser):
"""
Expand Down Expand Up @@ -130,21 +131,22 @@ def RemoveFile(self):
def Save(self):
"""Update user configuration file.

Remove empty sections. If resulting config isn't empty, write the file
to disk. If config is empty, remove the file from disk if it exists.
If self not empty after removing empty sections, write the file
to disk. Otherwise, remove the file from disk if it exists.

"""
if not self.IsEmpty():
fname = self.file
try:
cfgFile = open(fname, 'w')
except OSError:
os.unlink(fname)
cfgFile = open(fname, 'w')
with cfgFile:
self.write(cfgFile)
else:
self.RemoveFile()
fname = self.file
if fname:
if not self.IsEmpty():
try:
cfgFile = open(fname, 'w')
except OSError:
os.unlink(fname)
cfgFile = open(fname, 'w')
with cfgFile:
self.write(cfgFile)
else:
self.RemoveFile()

class IdleConf:
"""Hold config parsers for all idle config files in singleton instance.
Expand All @@ -158,7 +160,7 @@ class IdleConf:
(user home dir)/.idlerc/config-{config-type}.cfg
"""
def __init__(self):
self.config_types = ('main', 'extensions', 'highlight', 'keys')
self.config_types = ('main', 'highlight', 'keys', 'extensions')
self.defaultCfg = {}
self.userCfg = {}
self.cfg = {} # TODO use to select userCfg vs defaultCfg
Expand Down Expand Up @@ -766,7 +768,6 @@ def SaveUserCfgFiles(self):

idleConf = IdleConf()


_warned = set()
def _warn(msg, *key):
key = (msg,) + key
Expand All @@ -778,9 +779,100 @@ def _warn(msg, *key):
_warned.add(key)


class ConfigChanges(dict):
"""Manage a user's proposed configuration option changes.

Names used across multiple methods:
page -- one of the 4 top-level dicts representing a
.idlerc/config-x.cfg file.
config_type -- name of a page.
section -- a section within a page/file.
option -- name of an option within a section.
value -- value for the option.

Methods
add_option: Add option and value to changes.
save_option: Save option and value to config parser.
save_all: Save all the changes to the config parser and file.
delete_section: Delete section if it exists.
clear: Clear all changes by clearing each page.
"""
def __init__(self):
"Create a page for each configuration file"
self.pages = [] # List of unhashable dicts.
for config_type in idleConf.config_types:
self[config_type] = {}
self.pages.append(self[config_type])

def add_option(self, config_type, section, item, value):
"Add item/value pair for config_type and section."
page = self[config_type]
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't know if we needed to add a check for config_type to make sure it's valid before accessing it.

Copy link
Member Author

@terryjreedy terryjreedy Jul 7, 2017

Choose a reason for hiding this comment

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

What would add_option do if it did? It expects new sections. New config-types are a bug. A test of the user, config dialog, should fail. General principle: only check first or catch after if one has an alternative action in mind that is better than letting an exception bubble up.

value = str(value) # Make sure we use a string.
if section not in page:
page[section] = {}
page[section][item] = value

@staticmethod
def save_option(config_type, section, item, value):
"""Return True if the configuration value was added or changed.

Helper for save_all.
"""
if idleConf.defaultCfg[config_type].has_option(section, item):
if idleConf.defaultCfg[config_type].Get(section, item) == value:
# The setting equals a default setting, remove it from user cfg.
return idleConf.userCfg[config_type].RemoveOption(section, item)
# If we got here, set the option.
return idleConf.userCfg[config_type].SetOption(section, item, value)

def save_all(self):
"""Save configuration changes to the user config file.

Then clear self in preparation for additional changes.
"""
idleConf.userCfg['main'].Save()
for config_type in self:
cfg_type_changed = False
page = self[config_type]
for section in page:
if section == 'HelpFiles': # Remove it for replacement.
idleConf.userCfg['main'].remove_section('HelpFiles')
cfg_type_changed = True
for item, value in page[section].items():
if self.save_option(config_type, section, item, value):
cfg_type_changed = True
if cfg_type_changed:
idleConf.userCfg[config_type].Save()
for config_type in ['keys', 'highlight']:
# Save these even if unchanged!
idleConf.userCfg[config_type].Save()
self.clear()
Copy link
Contributor

@mlouielu mlouielu Jul 7, 2017

Choose a reason for hiding this comment

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

Should the result be clear after save_all?
If should, then the docstring need to note it.

But I think it should not clear after save_all, this should let dev to do when needed

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, it should be clear after save_all, that mean the changed has been apply. Maybe this side-affect need to be documented.

Copy link
Member Author

@terryjreedy terryjreedy Jul 7, 2017

Choose a reason for hiding this comment

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

By our current standard, yes. Changed.

# ConfigDialog caller must add the following call
# self.save_all_changed_extensions() # Uses a different mechanism.

def delete_section(self, config_type, section):
"""Delete a section from self, userCfg, and file.

Used to delete custom themes and keysets.
"""
if section in self[config_type]:
del self[config_type][section]
configpage = idleConf.userCfg[config_type]
configpage.remove_section(section)
configpage.Save()

def clear(self):
"""Clear all 4 pages.

Called in save_all after saving to idleConf.
XXX Mark window *title* when there are changes; unmark here.
"""
for page in self.pages:
page.clear()


# TODO Revise test output, write expanded unittest
#
if __name__ == '__main__':
def _dump(): # htest # (not really, but ignore in coverage)
from zlib import crc32
line, crc = 0, 0

Expand All @@ -790,10 +882,10 @@ def sprint(obj):
line += 1
crc = crc32(txt.encode(encoding='utf-8'), crc)
print(txt)
#print('***', line, crc, '***') # uncomment for diagnosis
#print('***', line, crc, '***') # Uncomment for diagnosis.

def dumpCfg(cfg):
print('\n', cfg, '\n') # has variable '0xnnnnnnnn' addresses
print('\n', cfg, '\n') # Cfg has variable '0xnnnnnnnn' address.
for key in sorted(cfg.keys()):
sections = cfg[key].sections()
sprint(key)
Expand All @@ -808,3 +900,9 @@ def dumpCfg(cfg):
dumpCfg(idleConf.defaultCfg)
dumpCfg(idleConf.userCfg)
print('\nlines = ', line, ', crc = ', crc, sep='')

if __name__ == '__main__':
import unittest
unittest.main('idlelib.idle_test.test_config',
verbosity=2, exit=False)
#_dump()
Loading