Skip to content

bpo-36096: IDLE: Refactor class variables in colorizer #12002

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 1 commit into from
Feb 27, 2019
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
26 changes: 15 additions & 11 deletions Lib/idlelib/colorizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,26 +55,35 @@ def color_config(text):
class ColorDelegator(Delegator):
"""Delegator for syntax highlighting (text coloring).

Class variables:
after_id: Identifier for scheduled after event.
Instance variables:
delegate: Delegator below this one in the stack, meaning the
one this one delegates to.

Used to track state:
after_id: Identifier for scheduled after event, which is a
timer for colorizing the text.
allow_colorizing: Boolean toggle for applying colorizing.
colorizing: Boolean flag when colorizing is in process.
stop_colorizing: Boolean flag to end an active colorizing
process.
close_when_done: Widget to destroy after colorizing process
completes (doesn't seem to be used by IDLE).

Instance variables:
delegate: Delegator below this one in the stack, meaning the
one this one delegates to.
"""

def __init__(self):
Delegator.__init__(self)
self.init_state()
self.prog = prog
self.idprog = idprog
self.LoadTagDefs()

def init_state(self):
"Initialize variables that track colorizing state."
self.after_id = None
self.allow_colorizing = True
self.stop_colorizing = False
self.colorizing = False

def setdelegate(self, delegate):
"""Set the delegate for this instance.

Expand Down Expand Up @@ -134,11 +143,6 @@ def delete(self, index1, index2=None):
self.delegate.delete(index1, index2)
self.notify_range(index1)

after_id = None
allow_colorizing = True
stop_colorizing = False
colorizing = False

def notify_range(self, index1, index2=None):
"Mark text changes for processing and restart colorizing, if active."
self.tag_add("TODO", index1, index2)
Expand Down
52 changes: 42 additions & 10 deletions Lib/idlelib/idle_test/test_colorizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,46 +100,78 @@ def test_color_config(self):
eq(text['inactiveselectbackground'], 'gray')


class ColorDelegatorTest(unittest.TestCase):
class ColorDelegatorInstantiationTest(unittest.TestCase):

@classmethod
def setUpClass(cls):
requires('gui')
root = cls.root = Tk()
root.withdraw()
text = cls.text = Text(root)
cls.percolator = Percolator(text)
# Delegator stack = [Delagator(text)]

@classmethod
def tearDownClass(cls):
cls.percolator.redir.close()
del cls.percolator, cls.text
del cls.text
cls.root.update_idletasks()
cls.root.destroy()
del cls.root

def setUp(self):
self.color = colorizer.ColorDelegator()
self.percolator.insertfilter(self.color)
# Calls color.setdelagate(Delagator(text)).

def tearDown(self):
self.color.close()
self.percolator.removefilter(self.color)
self.text.delete('1.0', 'end')
self.color.resetcache()
del self.color

def test_init(self):
color = self.color
self.assertIsInstance(color, colorizer.ColorDelegator)
# The following are class variables.

def test_init_state(self):
# init_state() is called during the instantiation of
# ColorDelegator in setUp().
color = self.color
self.assertIsNone(color.after_id)
self.assertTrue(color.allow_colorizing)
self.assertFalse(color.colorizing)
self.assertFalse(color.stop_colorizing)


class ColorDelegatorTest(unittest.TestCase):

@classmethod
def setUpClass(cls):
requires('gui')
root = cls.root = Tk()
root.withdraw()
text = cls.text = Text(root)
cls.percolator = Percolator(text)
# Delegator stack = [Delegator(text)]

@classmethod
def tearDownClass(cls):
cls.percolator.redir.close()
del cls.percolator, cls.text
cls.root.update_idletasks()
cls.root.destroy()
del cls.root

def setUp(self):
self.color = colorizer.ColorDelegator()
self.percolator.insertfilter(self.color)
# Calls color.setdelegate(Delegator(text)).

def tearDown(self):
self.color.close()
self.percolator.removefilter(self.color)
self.text.delete('1.0', 'end')
self.color.resetcache()
del self.color

def test_setdelegate(self):
# Called in setUp.
# Called in setUp when filter is attached to percolator.
color = self.color
self.assertIsInstance(color.delegate, colorizer.Delegator)
# It is too late to mock notify_range, so test side effect.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Refactor class variables to instance variables in colorizer.
Copy link
Member

Choose a reason for hiding this comment

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

For the present, I will take care of the idlelib news.txt entries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should have asked this earlier, but once you approve a PR, should I then merge it? Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

If I check the []Approve box for a Review, and if you agree that it is ready, and accept the responsibility of doing the merge, yes. For most IDLE issues though, I consider the followup responsibility to be shared. If you prefer me to merge, say so. If a PR is actually a joint product, or if it is blocking doing another, I might merge it, but should say so first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, that's very helpful. I think this one looks OK and you've approved it, so I'll merge it soon.