Skip to content
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

Leo crashes with unusual combination of @clean and .leo file #289

Closed
edreamleo opened this issue Jun 16, 2016 · 5 comments
Closed

Leo crashes with unusual combination of @clean and .leo file #289

edreamleo opened this issue Jun 16, 2016 · 5 comments
Assignees
Milestone

Comments

@edreamleo
Copy link
Member

edreamleo commented Jun 16, 2016

Reported by Laurent Steffan, lmsteffan@gmail.com

Given this external file:

class myClass:
    def __init__(self, myArg=    10
):
            pass

with myClass(myArg=10
) as essai:
    pass

And this .leo file:

<?xml version="1.0" encoding="utf-8"?>
<!-- Created by Leo: http://leoeditor.com/leo_toc.html -->
<leo_file xmlns:leo="http://leoeditor.com/namespaces/leo-python-editor/1.1" >
<leo_header file_format="2" tnodes="0" max_tnode_index="0" clone_windows="0"/>
<globals body_outline_ratio="0.5" body_secondary_ratio="0.5">
    <global_window_position top="50" left="50" height="500" width="700"/>
    <global_log_window_position top="0" left="0" height="0" width="0"/>
</globals>
<preferences/>
<find_panel_settings/>
<vnodes>
<v t="laurent.20160602153419.1" a="E"><vh>Dummy</vh>
<v t="laurent.20160602105120.1" a="E"><vh>@clean essai.py</vh>
<v t="laurent.20160602105120.3" a="E"><vh>class dummy</vh>
<v t="laurent.20160602105120.6" a="E"><vh>__init__</vh>
<v t="laurent.20160602113739.1"><vh>&lt;&lt;argument&gt;&gt;</vh></v>
</v>
</v>
</v>
</v>
</vnodes>
<tnodes>
<t tx="laurent.20160602105120.1">@language python
@tabwidth -4
@others

with myClass(myArg=&lt;&lt;argument&gt;&gt;) as essai:
    pass
</t>
<t tx="laurent.20160602105120.3">class myClass:
    @others
</t>
<t tx="laurent.20160602105120.6">def __init__(self, myArg=&lt;&lt;argument&gt;&gt;):
        pass</t>
<t tx="laurent.20160602113739.1">10</t>
<t tx="laurent.20160602153419.1"></t>
</tnodes>
</leo_file>

Let me make this clearer: the original body text of the top-level node is:

@language python
@tabwidth -4
@others

with myClass(myArg=<<argument>>
) as essai:
    pass

Leo crashes with the message "AssertionError: newLevel == oldLevel + 1" generated in at.changeLevel.

This is an interesting, worthy, bug, not necessarily because it will be hard to fix, but because it reveals some interesting edge cases with @clean (and @shadow) and also the new (v5) sentinels. I'll fix this asap, after discussing the details.

@edreamleo edreamleo added the Bug label Jun 16, 2016
@edreamleo edreamleo added this to the 5.4 milestone Jun 16, 2016
@edreamleo edreamleo self-assigned this Jun 16, 2016
@edreamleo
Copy link
Member Author

edreamleo commented Jun 17, 2016

I said earlier that this was an interesting, worthy, bug. Here are the reasons:

Point 1: This bug doesn't happen with @file instead of @clean. However, when reading the file, the outline (with @file) becomes:

with myClass(myArg=
<<argument>>
) as essai:
    pass

That is, the section reference gets transferred to a new line. The so-called "new" sentinel scheme does not use @nl and @nonl sentinels, so there is no way to suppress the newline that starts the #@+<<argument>> sentinel line.

Point 2: Things are even more "interesting" with @clean. The @clean (Mulder-Ream) update algorithm uses a line-by-line diff. (This will never change). Therefore, update algorithm can only assign whole lines to section definitions. Trying to assign "snippets" of a line will never work, even after writing the .leo file to (supposedly) correct the assignment of characters to section definitions.

In short, there is a hidden boundary condition of both @clean and @file: section definitions can only contain whole lines.

Point 3: The proximate cause of the crash lies elsewhere, namely an assertion that fails in the changeLevel method:

if newLevel > oldLevel:
    assert newLevel == oldLevel + 1, 'newLevel == oldLevel + 1'

A possible fix is simply to do this:

if newLevel > oldLevel:
    newLevel = oldLevel + 1

The corresponding code makes no real sense to me. I have no memory of the code or why it is written the way it was. I can deduce the the code is the way it is because the new @+node sentinels don't tell explicitly where nodes end, so some trickery is required, but the actual code (involving the at.endSentinelStack, at.endSentinelNodeStack and at.endSentinelLevelStack) seems bizarre.

Happily, the clone-find commands (combined with traces) will likely show what is happening. It's possible that the code can be improved. We shall see.

Point 4. There is no way that I shall ever change the Mulder-Ream update algorithm to allow section references to be "embedded" in a line. This would require the worst kind of "if" statements to sully the algorithm. I would no more tolerate such "if" statements in the @clean logic than the Python devs would tolerate extra "if" statements in Python's garbage collector. (In fact, there are none!)

Summary

The quick fix will be to replace the failing assert by code that guarantees the condition being asserted. Whether this a proper fix in all cases remains to be seen. I'll be studying the code closely to find out.

This bug has revealed some hidden, minor, limitations in section references in both @file and @clean files. These limitations are baked into Leo and are not likely ever to be removed. I'll reject any requests to remove them without further discussion.

edreamleo added a commit that referenced this issue Jun 17, 2016
More work is needed to see if the change makes sense in all cases.  All tests pass.
@edreamleo
Copy link
Member Author

Rev 014b763 makes the change discussed above. I'll leave this issue open until I study the code further and document what I find here.

@jurov
Copy link
Contributor

jurov commented Jun 27, 2016

If << node reference >> is mixed with other text on one line, can Leo please output a warning that it won't work on reading? It is probably one of the first creative things people try with node references, at first it appears to work but breaks badly when rereading external change.

@edreamleo
Copy link
Member Author

@jurov I agree, something should be done. I'm surprised at the present behavior, which happens even with @file, so the @clean update algorithm is not (entirely) to blame. I suspect that Leo handled section references differently (and better) with old-style sentinels, but the advantages of new-style sentinels far outweigh the problem under discussion.

The workaround is to do, for example, something like:

if (
    << a test >>
):
    << body >>

which does indeed work properly, with both @file and @clean, as I have just verified.

I agree that some warning would be good. I'll look into it.

@edreamleo
Copy link
Member Author

I am going to split this issue into two parts:

  1. The issue with crashes appears to be fixed.
  2. The issue re: warnings for section references with leading or trailing text should be considered an enhancement.

I'll close this issue and create another asking for warnings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants