Rev 1cabf32 demonstrates, I think, that I now understand every changed line
of the v-lines branch. This rev adds needed comments about what v._sync
means. It also simplifies v.hasBody in a straightforward way.
To state my conclusion first: It would be intolerable to break existing
code *of which we have no knowledge*. That code might use v._bodyString
*without* checking v._sync. In fact, Leo's own code uses v._bodyString
without checking v._sync. There is no guarantee that user scripts don't. It
seems that the only way forward will be to deprecate v._bodyString entirely
in Leo 5.6. This would allow us to use this branch in Leo 5.7.
The rest of this post gives important details. Please read them carefully
before commenting.
I think this branch is, *as far as it goes*, reasonable, straightforward
*and* sound. But the *local *code is not the real issue!
Vitalije's attention to detail finally gave a name to my "nameless dread".
This branch adds a single new line to v.copyTree, namely,
v2._sync = v._sync
As soon as I saw this, I asked myself, "are there other places in Leo's
code that would require changes?
Alas yes. A cff on _bodyString yields these tests in various VNode methods:
g.is_special(self._bodyString, 0, "@all")
g.is_special(self._bodyString, 0, "@ignore")
g.is_special(self._bodyString, 0, "@others")
These probably should use self.bodyString() instead of self._bodyString.
>From a design sense, these are *not *serious bugs because they are
encapsulated in the VNode class. The real worries are bugs that might
appear *outside* of the VNode class.
Looking wider afield, there are several references to v._bodyString in the
Importer class in leo/plugins/importers/linescanner.py. I suspect they too
should be changed to v.bodyString(), but *this would not be an easy change
to make*. The import code uses a completely different (and temporary) ivar,
namely v.import_lines.
The code could probably stay as it is, *provided* that Importer code
ensures that v._bodyString is valid (v._sync != 'body') during
initialization. The alternative would be to rewrite the Importer API to use
the capabilities provided by the v-lines branch. It's reasonable, but
non-trivial.
*Summary*
The new VNode code itself will probably be sound, once three VNode bugs are
fixed.
The v-lines branch provides useful new capabilities. But it would be
intolerable to break existing code that uses v._bodyString *without*
checking v._sync. The only way forward seems to be to deprecate
v._bodyString in Leo 5.6 and add the new capabilities in Leo 5.7. I'm not
even sure I am comfortable with that...
The importer code might have to be revised in non-trivial ways. The
importer unit tests may not be a guarantee that all is well.
Your comments, please. My opinions have changed several times while writing
this post. They might change again.
Edward
--
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 [email protected].
To post to this group, send email to [email protected].
Visit this group at https://groups.google.com/group/leo-editor.
For more options, visit https://groups.google.com/d/optout.
