boltex / leojs

Leo Literate Editor with Outline in Typescript
MIT License
23 stars 0 forks source link

LeoJS does not recognize multiple @language directives #123

Closed edreamleo closed 1 week ago

edreamleo commented 2 months ago

One of my common outline patterns is:

@language rest
notes about an issue
@language python

followed by python child nodes relating to the issue.

Bug: LeoJS colorizes the children with @language rest.

Instead, ambiguous nodes (nodes containing multiple @language directives) should have no effect on the node's descendants.

This is a serious bug from my point of view. However, there are several easy and obvious workarounds.

boltex commented 2 months ago

Thanks for this report! :)

I will make a minimal Leo file with only 2 or 3 nodes and try to compare the behavior between Leo, LeoInteg and LeoJS for cases like this, or similar to this. (E.g. with an @other between the @language directives, etc... )

And I'll fix it and/or report here inconsistencies or further questions that may arise from my experiments.

edreamleo commented 2 months ago

@boltex Thanks for your comments. Imo, part 2 of this issue should be fixed for LeoJS 1.0.

Part 1: Desktop Leo can color individual lines with different language within the same body text. That feature might be difficult or impossible in vs-code.

Part 2: The logic that controls whether @language directives affect descendant nodes should have nothing to do with vs-code. I'm surprised that LeoJS has not already duplicated that logic.

edreamleo commented 2 months ago

@boltex There two workarounds for the pattern:

@language rest
notes about an issue
@language python

Workaround 1: Put the notes in a separate node, with a single @language rest directive.

Workaround 2: Eliminate both @language directives and convert the notes to Python comments.

In other words, this issue will not prevent me from using LeoJS :-)

boltex commented 2 months ago

Part 2: The logic that controls whether @language directives affect descendant nodes should have nothing to do with vs-code. I'm surprised that LeoJS has not already duplicated that logic.

You are right about this, perhaps I thought "vscode can only assign 1 language to an editor, so I might stop looking for @language when I reach the first @language directive in a body text". When in fact I should settle on the LAST @language directive in a node to decide it's language (so that it affects its descendants properly)

Thanks again for your insights !

edreamleo commented 2 months ago

@boltex I thought perhaps you had made such an invalid assumption. Restoring Leo's exact coloring code is likely the only way to fix this issue.

boltex commented 1 month ago

@edreamleo I just tested in Leo itself : Having multiple @languages in a node makes its descendant have no set language. (plain text) Is that the intended behavior? (just want to make sure before I make it so in LeoJS)

edreamleo commented 1 month ago

@boltex The intended behavior is that an ambiguous node (a node with multiple @language directives) does not affect the syntax coloring of its descendant nodes. So whether descendant nodes are colorized depends on:

a) the first (moving up the tree) non-ambiguous ancestor node that contains an @language directive. b) the default (target) language in effect for the outline.

Case a) is even more complex for cloned nodes. Imo, both LeoInteg and LeoJS should follow Leo's code as closely as possible, starting with the node BaseColorizer.updateSyntaxColorer & helpers.

Defaults

All outlines I use have a default coloring of either @language rest or @language python.

However, leoSettings.leo has a node @string target-language = plain which looks like no default language is in effect, but my myLeoSettings.leo has @string target-language = python.

In short, it's complicated :-) Please feel free to ask more questions.

boltex commented 1 month ago

@edreamleo 😄 I just looked at leoColorizer.py to figure out the way Leo actually does it, and then easily fixed it in devel branch with commit 66fc45440ded9201db209f8df1f0b910037e5401

--> Thanks for beta-testing LeoJS and finding details like this !!

boltex commented 1 month ago

@edreamleo I'll fix it in leoserver too

edreamleo commented 1 month ago

@boltex I'm glad the fix was straightforward.