Vitalije and I are having a discussion about programming style as part of
#862 <https://github.com/leo-editor/leo-editor/issues/862>. Here, I'd like
to explain the programming style that I use throughout Leo. It's possible
that this style will change significantly as the result of our discussion,
but I doubt it.
Leo has two related commands: paste-node and paste-retaining-clones. Here
is the actual code.
@g.commander_command('paste-retaining-clones')
def pasteOutlineRetainingClones(self, event=None):
'''Paste an outline into the present outline from the clipboard.
Nodes *retain* their original identify.'''
c = self
return c.pasteOutline(reassignIndices=False)
# To cut and paste between apps, just copy into an empty body first, then
copy to Leo's clipboard.
@g.commander_command('paste-node')
def pasteOutline(self, event=None,
reassignIndices=True,
redrawFlag=True,
s=None,
tempOutline=False, # True: don't make entries in the gnxDict.
undoFlag=True
):
'''
Paste an outline into the present outline from the clipboard.
Nodes do *not* retain their original identify.
'''
c = self
if s is None:
s = g.app.gui.getTextFromClipboard()
pasteAsClone = not reassignIndices
# commenting following block fixes #478
#if pasteAsClone and g.app.paste_c != c:
# g.es('illegal paste-retaining-clones', color='red')
# g.es('only valid in same outline.')
# return
undoType = 'Paste Node' if reassignIndices else 'Paste As Clone'
c.endEditing()
if not s or not c.canPasteOutline(s):
return # This should never happen.
isLeo = g.match(s, 0, g.app.prolog_prefix_string)
vnodeInfoDict = computeVnodeInfoDict(c) if pasteAsClone else {}
# create a *position* to be pasted.
if isLeo:
pasted = c.fileCommands.getLeoOutlineFromClipboard(s,
reassignIndices, tempOutline)
if not pasted:
# 2016/10/06:
# We no longer support pasting MORE outlines. Use import-MORE-files
instead.
return None
if pasteAsClone:
copiedBunchList = computeCopiedBunchList(c, pasted, vnodeInfoDict)
else:
copiedBunchList = []
if undoFlag:
undoData = c.undoer.beforeInsertNode(c.p,
pasteAsClone=pasteAsClone, copiedBunchList=copiedBunchList)
c.validateOutline()
if not tempOutline:
# Fix #427: Don't check for duplicate vnodes.
c.checkOutline()
c.selectPosition(pasted)
pasted.setDirty()
c.setChanged(True, redrawFlag=redrawFlag) # Prevent flash when fixing
#387.
# paste as first child if back is expanded.
back = pasted.back()
if back and back.hasChildren() and back.isExpanded():
# 2011/06/21: fixed hanger: test back.hasChildren().
pasted.moveToNthChildOf(back, 0)
if pasteAsClone:
# Set dirty bits for ancestors of *all* pasted nodes.
# Note: the setDescendentsDirty flag does not do what we want.
for p in pasted.self_and_subtree():
p.setAllAncestorAtFileNodesDirty(
setDescendentsDirty=False)
if undoFlag:
c.undoer.afterInsertNode(pasted, undoType, undoData)
if redrawFlag:
c.redraw(pasted)
c.recolor()
return pasted
This isn't the easiest code to understand, but imo the *details don't
matter*. What *does* matter is that:
1. Each command handles *everything* about the command, including undo and
redraw.
2. The meaning of each kwarg is clear:
- reassignIndices: distinguishes between paste-node and paste-retaining
clones.
- redrawFlag: suppresses calls to c.redraw when False.
- s: The to-be-pasted text. Get the text from the clipboard if None.
- tempOutline: don't make entries in the gnxDict.
- undoFlag: Handle undo unless False.
I admit, the tempOutline flag is a bit mysterious. In fact, it can be
removed. See below.
cff shows only three calls to c.pasteOutline in Leo's core:
1. From c_oc.pasteOutlineRetainingClones (shown above)
return c.pasteOutline(reassignIndices=False)
2. From abbrev.paste_tree:
p = c.pasteOutline(s=s, redrawFlag=False, undoFlag=False)
3. From LM.openEmptyWorkBook:
p = c.pasteOutline()
Note: unit tests call only c.pasteOutline() and
c.pasteOutlineRetainingClones().
>From my point of view, the only important question is: *are these calls
understandable in context*?
And clearly, the answer is, *unequivocally yes*. Indeed, the only
"difficult" call is from abbrev.paste_tree, but *in context* it is obvious
that the to-be-pasted text comes from the abbreviation node, not the
clipboard.
*Removing kwargs*
Vitalije's suggested alternative, if I understand him fully, is to remove
*all* kwargs! The post script shows the refactored c_oc.pasteOutline and
c_oc.pasteOutlineRetainingClones, and a third method which would also be
needed. Imo, this refactoring would be catastrophic, for the following
reasons:
1. There is extreme duplication of code, to no purpose whatsoever.
2. There is no easy way to compare and contrast the similarities and
differences between the two commands.
3. The only way to support abbrev.paste_tree would be to create a third
method, c.pasteOutlineFromString, also shown in the post script. Indeed,
*all* minor variants of code require completely new versions of code,
duplicating all common code, no matter how complex.
*Summary*
The coding pattern just discussed forms the basis for all of Leo's commands.
kwargs are intended to be a way of making *small, easily understood*
modifications to existing code.
I see no reason to enforce a faux-functional programming style on all of
Leo's code base. To the contrary, kwargs reduce code duplication and
explicitly show the differences between minor variants of code.
Unnecessary kwargs should be removed when possible. It looks like the
tempOutline kwarg could be removed. That might break existing scripts, but
that's unlikely. Removing this kwarg could have the happy side effect of
simplifying fc.getLeoOutlineFromClipboard.
Edward
P. S. Here are the refactored (untested) versions of c_oc.pasteOutline,
c_oc.pasteOutlineRetainingClones, and the newly-necessary
c.pasteOutlineFromString. Imo, this kind of code duplication should be
avoided at all costs:
@g.commander_command('paste-node')
def pasteOutline(self, event=None):
'''
Paste an outline into the present outline from the clipboard.
Nodes do *not* retain their original identify.
'''
c = self
s = g.app.gui.getTextFromClipboard()
undoType = 'Paste Node'
c.endEditing()
if not c.canPasteOutline(s):
return # This should never happen.
isLeo = g.match(s, 0, g.app.prolog_prefix_string)
vnodeInfoDict = {}
# create a *position* to be pasted.
if isLeo:
pasted = c.fileCommands.getLeoOutlineFromClipboard(s,
reassignIndices)
if not pasted:
# We no longer support pasting MORE outlines. Use import-MORE-files
instead.
return None
copiedBunchList = []
undoData = c.undoer.beforeInsertNode(c.p,
pasteAsClone=False, copiedBunchList=copiedBunchList)
c.validateOutline()
c.checkOutline()
c.selectPosition(pasted)
pasted.setDirty()
c.setChanged(True)
# paste as first child if back is expanded.
back = pasted.back()
if back and back.hasChildren() and back.isExpanded():
# 2011/06/21: fixed hanger: test back.hasChildren().
pasted.moveToNthChildOf(back, 0)
if pasteAsClone:
# Set dirty bits for ancestors of *all* pasted nodes.
# Note: the setDescendentsDirty flag does not do what we want.
for p in pasted.self_and_subtree():
p.setAllAncestorAtFileNodesDirty(
setDescendentsDirty=False)
c.undoer.afterInsertNode(pasted, undoType, undoData)
c.redraw(pasted)
c.recolor()
return pasted
@g.commander_command('paste-retaining-clones')
def pasteOutlineRetainingClones(self, event=None):
'''
Paste an outline into the present outline from the clipboard.
Nodes do *not* retain their original identify.
'''
c = self
s = g.app.gui.getTextFromClipboard()
undoType = 'Paste As Clone'
c.endEditing()
if not s or not c.canPasteOutline(s):
return # This should never happen.
isLeo = g.match(s, 0, g.app.prolog_prefix_string)
vnodeInfoDict = computeVnodeInfoDict(c)
# create a *position* to be pasted.
if isLeo:
pasted = c.fileCommands.getLeoOutlineFromClipboard(s,
reassignIndices)
if not pasted:
# 2016/10/06:
# We no longer support pasting MORE outlines. Use import-MORE-files
instead.
return None
copiedBunchList = computeCopiedBunchList(c, pasted, vnodeInfoDict)
undoData = c.undoer.beforeInsertNode(c.p,
pasteAsClone=True, copiedBunchList=copiedBunchList)
c.validateOutline()
c.checkOutline()
c.selectPosition(pasted)
pasted.setDirty()
c.setChanged(True, redrawFlag=redrawFlag) # Prevent flash when fixing
#387.
# paste as first child if back is expanded.
back = pasted.back()
if back and back.hasChildren() and back.isExpanded():
# 2011/06/21: fixed hanger: test back.hasChildren().
pasted.moveToNthChildOf(back, 0)
if pasteAsClone:
# Set dirty bits for ancestors of *all* pasted nodes.
# Note: the setDescendentsDirty flag does not do what we want.
for p in pasted.self_and_subtree():
p.setAllAncestorAtFileNodesDirty(
setDescendentsDirty=False)
c.undoer.afterInsertNode(pasted, undoType, undoData)
c.redraw(pasted)
c.recolor()
return pasted
def pasteOutlineFromString(self, s):
'''
Paste an outline into the present outline from the clipboard.
Nodes do *not* retain their original identify.
'''
c = self
c.endEditing()
if not c.canPasteOutline(s):
return
isLeo = g.match(s, 0, g.app.prolog_prefix_string)
vnodeInfoDict = {}
# create a *position* to be pasted.
if isLeo:
pasted = c.fileCommands.getLeoOutlineFromClipboard(s)
if not pasted:
# We no longer support pasting MORE outlines. Use import-MORE-files
instead.
return None
c.validateOutline()
c.checkOutline()
c.selectPosition(pasted)
pasted.setDirty()
c.setChanged(True, redrawFlag=False)
# paste as first child if back is expanded.
back = pasted.back()
if back and back.hasChildren() and back.isExpanded():
# 2011/06/21: fixed hanger: test back.hasChildren().
pasted.moveToNthChildOf(back, 0)
return pasted
EKR
--
You received this message because you are subscribed to the Google Groups
"leo-editor" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to leo-editor+unsubscr...@googlegroups.com.
To post to this group, send email to leo-editor@googlegroups.com.
Visit this group at https://groups.google.com/group/leo-editor.
For more options, visit https://groups.google.com/d/optout.
