On Tue, Sep 1, 2020 at 10:55 AM vitalije <[email protected]> wrote:
> I don't fully understand your previous post. ... > I am pretty sure that ExpandController is more immune to the outline changes than the v.expandedPositions method. We agree that positions, no matter how defined, do not necessarily survive changes in the outline. That's true of Leo's existing positions, and it's true of the ExpandController class. In my previous post I hinted that a *complete *fix to #1631 <https://github.com/leo-editor/leo-editor/issues/1631> would involve *updating *all saved positions (using either v.expandedPosition or your proposed ExpandController class) whenever the outline changes. Imo, this is not worth doing. It's a lot of work for virtually no gain. One could even say that contracting a moved position is a *good* thing. Yes, the present behavior confused Félix, but he's a special case :-) He was paying much closer attention to Leo's behavior than the average user, including me! That's because he was taking the implementor's view, not the user's view. *Unexpected difficulties* Here are some details that indicate the difficulties in a complete fix: I tried defining c.shouldBeExpanded as in the bug-1631 branch. The proposed code does not use any data structures at all. Alas, the new code fails in difficult-to-understand ways. The attached code fails in the bug-1631 branch. As noted in the "Readme" node, the a1 node does not always show that it has children. (You may have to insert or delete the "a" clone to see this.) As I said, the reasons for these problems are difficult to understand. They involve Leo's complex tree redrawing code. Here are some of the difficulties with the redraw code: 1. Redraw code happens while changing nodes, so c.p is not guaranteed to be what one might naively think it is. 2. There are at least three paths through the tree code relating to expansion and contraction: - The user clicks the expand/contract icon, thereby possibly selecting a new node. - The user uses an arrow key in the tree pane (or an alt-arrow key anywhere) to expand or contract a node, and possibly move to a new node. - A script modifies the expansion state of a node (vnode) and then calls c.redraw(). At present, Leo handles all these details properly. The new code doesn't. Yes, there could be a bug lurking in Leo's Qt tree code that the new code exposes. *Summary* I am willing to consider fixing #1631 only if the following conditions are met: 1. The Qt code does not change in any way, except possibly for bug fixes that stand on their own merits. 2. All positions, no matter how represented, are *always* updated properly when the outline changes. In particular, positions must be updated when undoing a delete operation, which seems like the acid test. Imo, this is just too much work to be worthwhile. I am about to close #1631. I will consider PR's that meet the above requirements, but I advise against further work on this issue. 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 view this discussion on the web visit https://groups.google.com/d/msgid/leo-editor/CAMF8tS3CLtsxwfExN7OOjdC%2BB3_AONagZByyJkKR9TjyLhbQ3g%40mail.gmail.com.
test-bug-1631.leo
Description: Binary data
