naomi icon indicating copy to clipboard operation
naomi copied to clipboard

Naomi breaks syntax highlighting for LaTeX and MarkdownEditing

Open hsiktas opened this issue 6 years ago • 37 comments

Hi,

as soon as I enable the Naomi package I get bombarded by multiple error windows like this and the corresponding syntax highlightings stop working:

image image

In order to get back syntax highlighting for LaTeX and Markdown files I have to disable the Naomi package. Unfortunately, I could not find any additional information in the Sublime Text Console. :(

hsiktas avatar Mar 28 '18 22:03 hsiktas

I believe I found the error to be related to styled components or template strings. Do you have a sample of a working Latex code that have embedded javascript?

borela avatar Mar 29 '18 17:03 borela

Sorry I have never used embedded JavaScript in my LaTeX files. Do you believe that this happens only when we embed JS into the other syntaxes?

hsiktas avatar Mar 30 '18 13:03 hsiktas

This bug was puzzling, the only time I was able to reproduce it was in a dev build I used on a friend's PC; I use the 3143 and the recursion bug does not happen there, so it's a regression in dev build.

borela avatar Mar 30 '18 15:03 borela

These error messages normally tell you some scopes not being popped off correctly, causing the scope stack to be flooded. To prevent overflow and app crash no more than 25000 levels are allowed. I realized these issues several times during developing my CNC syntaxes.

An example of code which causes issues might be helpful for detailed debugging.

It's not quite obvious how MarkdownEditing and LaTeX which seem to trigger the error are associated with Naomi. Opening several .md files didn't cause such issues here.

Disabling MardownEditing and opening a .md file using Naomi's syntax seemed not to highlight anything at all.

The .md was a readme created by create-react-app. It's quite long.

Without detailed info about the combination of languages triggering the issue I guess it will be hard to find the culprit. I guess if lang1 includes lang2 and lang2 includes lang1 for some reason, a recursion could quickly arise.

deathaxe avatar Apr 02 '18 18:04 deathaxe

@deathaxe Most of the contexts pop after matching, the biggest issue is that it does not happen in the build 3143, the LaTeX works, embedded javascript uses the Naomi's version as expected.

This is what puzzled me for a while, I don't use the dev builds, when I got to a computer with it, all I need to do for the error to appear is select the LaTeX syntax.

borela avatar Apr 02 '18 18:04 borela

@deathaxe Another option that could be added to the core, is a option to print the trace of the supposed recursion.

borela avatar Apr 02 '18 18:04 borela

The initial post doesn't state anything about LaTeX with embedded JavaScript but Markdown and a LaTeX file.

I actually opened a couple of LaTeX files and Markdowns with Naomi enabled on ST3160 without any issue.

As I am not a LaTeX expert I can't judge which constructs may trigger the issue. An example to reproduce might therefore help finding the culprit.

EDIT: Need to correct myself. Just set up a vanilla 3160 and hit the issue.

deathaxe avatar Apr 02 '18 19:04 deathaxe

The culprit seems to be the naomi.fjsx15.sublime-syntax.

What I did

Open LaTeX file from http://www.electronics.oulu.fi/latex/examples/example_1/example1.html

Step 1 - renaming one naomi syntax after another

Step 2 - commenting out some lines in syntaxes/naomi.fjsx15.sublime-syntax

name: JavaScript
scope: source.js

file_extensions:
  - js
  - jsx
  - mjs

contexts:
  main:
    - match: ""
      push: [ meta, entry ]

  entry:
    - include: Packages/Naomi/syntaxes/fjsx15/comment.sublime-syntax
    # - include: Packages/Naomi/syntaxes/fjsx15/statement.sublime-syntax
    # - include: Packages/Naomi/syntaxes/fjsx15/expression.sublime-syntax

  meta:
    - meta_scope: meta.naomi
    - match: ""
      pop: true

Step 3 - following the comment breadcrumps via syntaxes/fjsx15/comments.sublime-syntax to syntaxes/flow1/comments.sublime-syntax

Step 4 - commenting out all the includes within syntaxes/flow1/comments.sublime-syntax

hidden: true
scope: ...

contexts:
  main:
    - match: "/\\*\\s*::"
      scope: punctuation.definition.comment.begin.flow
      set: block-end
    - match: /\*(?=\s*:)
      scope: punctuation.definition.comment.begin.flow
      set: block-end

  block-end:
    - meta_scope: comment.block.flow
    - match: \*/
      scope: punctuation.definition.comment.end.js.fjsx15
      pop: true
    - match: (?=\S)
      push: type-statement-or-expression

  type-statement-or-expression:
    - match: (;)
      captures:
        1: keyword.other.terminator.flow
      pop: true
    # - include: Packages/Naomi/syntaxes/flow1/associated-type.sublime-syntax
    # - include: Packages/Naomi/syntaxes/flow1/declare.sublime-syntax
    # - include: Packages/Naomi/syntaxes/flow1/interface.sublime-syntax
    # - include: Packages/Naomi/syntaxes/flow1/type-aliasing.sublime-syntax
    # - include: Packages/Naomi/syntaxes/fjsx15/class/property.sublime-syntax
    # - include: Packages/Naomi/syntaxes/flow1/indexer.sublime-syntax
    # - include: Packages/Naomi/syntaxes/fjsx15/import.sublime-syntax
    # - include: Packages/Naomi/syntaxes/fjsx15/export.sublime-syntax

Step 5 No more errors.

Alternative

Moving the includes out of the entry into main works as well.

name: JavaScript
scope: source.js

file_extensions:
  - js
  - jsx
  - mjs

contexts:
  main:
    - include: Packages/Naomi/syntaxes/fjsx15/comment.sublime-syntax
    - include: Packages/Naomi/syntaxes/fjsx15/statement.sublime-syntax
    - include: Packages/Naomi/syntaxes/fjsx15/expression.sublime-syntax
    # - match: ""
    #   push: [ meta, entry ]

  # entry:

  meta:
    - meta_scope: meta.naomi
    - match: ""
      pop: true

Conclusion

There is at least one pop too much or too less somewhere.

Despite the fact your file structure is "unique" compared to all the other syntax projects, I am not sure whether the pop statements always work as you seem to expect them to do.

  1. I find it weird to expect a pop statement located in a main of a syntax-definition to pop off the whole syntax definition (as used in some of the included files). IMHO main is the top-level of a syntax file and therefore shouldn't be popped off stack.

  2. All the pushes from your main scopes seem strange as well. I'd expect the risk to fail a correct pop from stack to be very high by design in this package. At least for me its very hard to follow from file to file. I'd pass the question to one of the devs @wbond or @jps but I feel like ST's syntax infrastructure was never meant to be used this way, even though your modular approach is very interesting from a point of developer.

Hope it doesn't sound too negative!

As the risk of popping one stack-level too much or too less is very high, I honestly doubt the core to be blamed. As more and more syntaxes embed others the risk of stack overflow increases if at least one of them doesn't handle stack push/pop correclty. Popping off a level too much or less can cause more troubles then. I had to struggle with such issues, too, when embedding some of my CNC languages into others.

deathaxe avatar Apr 02 '18 20:04 deathaxe

@deathaxe Thank you for taking your time on this one.

I know, I went a step too far with the modularity and the combination of multiple files might be giving headaches with some new feature, but the interesting bit is that the bug does not happen in the build 3143(the one I use), I had to ask help because the bug never seam to happen on my machine.

By default all files pop from the main as it allows me to treat it like a normal context so for example, in the case of functions I could do:

set: [ body, parameters ]

And have the body and parameters in different files. This prevents the syntax from having thousands of lines, specially because I often times have lists of functions (javascript core), lists of properties (css); sublime never seam to complain about it, the - match: "" at the main fjsx15 file is just there to make it compatible with the default HTML syntax as it does not expect the the main to pop like you said.

When a main or context does not pop I add a suffix ".no-pop". But like I said, the recursion does not happen in the build 3143, so something changed since them in the dev build (which I am guessing is the embed feature).

I also suggested the stack trace because recursion bugs are always hard to track, but in this case it does look like an error in the core, otherwise it would happen in the build 3143 too.

borela avatar Apr 02 '18 21:04 borela

@deathaxe Yesterday I went to a friend's house to test the patches you showed as he uses the dev build(I can't test it on my machines right now, too many projects openned).

None of the patches worked, what is even stranger is that the bug seams random, often times I would comment parts of the syntax and it would be ok, but then if I tried just adding a dummy context, for example:

some-unused-context:
  - match: test
    scope: test

The bug would come back. It looks like a hard limit on the number of contexts or files involved when the embed feature is used, but I can't say for sure because the place that caused the bug would change from time to time (which I am guessing is a result of listing the files as the OS (Windows at least) does not garantee the files to be listed alphabetically).

borela avatar Apr 03 '18 11:04 borela

suggested the stack trace because recursion bugs are always hard to track

I agree with a function to help tracing such issues would help very much.

the bug seams random

I had same experiences yesterday. I first commented out the source.shell stuff from LaTeX and everything seemed working well. A step further it didn't.

But this is what I experienced in the past, too, when I added some unbalanced push/pops into my files. It seemed to work for smaller files, but when they got too big, I hit the sanity limit.

What I can see here feels exactly like that.

3143 vs. 3160

Loading my example1.tex to 3143 takes quite long to load. It feels like 3160 just gives up more quickly just like the sanity limit was recently added. ST3143 even got unresponsive once, when loading the file for the very first time.

Using the LaTeX package from 3143 in 3160 fails, too. So this issue is related to a change in core (added sanity limit?).

Not sure whether it is related, but I can see a couple of issues in 3143's console. Maybe they give some hints about possible reasons. Unreachable sections in a syntax file feel not too ideal.

Note: This is the only language triggering such issues and is the one, which triggers the sanity limit. :-/

rule Packages/Naomi/syntaxes/fjsx15/literal/string/template.sublime-syntax#13c4e162771b843d63c0bb268a8373dc::16d7f8c69dc5d9bad0df65b3e033ccbb::#anon_main_0 has a scope name, but is unreachable, so the name will never be used
rule scope:text.html.markdown#main has a scope name, but is unreachable, so the name will never be used
rule Packages/Naomi/syntaxes/fjsx15/literal/string/template.sublime-syntax#16d7f8c69dc5d9bad0df65b3e033ccbb::#anon_main_0 has a scope name, but is unreachable, so the name will never be used
rule Packages/Naomi/syntaxes/fjsx15/literal/string/template.sublime-syntax#13c4e162771b843d63c0bb268a8373dc::#anon_main_0 has a scope name, but is unreachable, so the name will never be used
rule Packages/Naomi/syntaxes/fjsx15/literal/string/template.sublime-syntax#da722399f83b5d5ce019cca5f4a14d31::16d7f8c69dc5d9bad0df65b3e033ccbb::#anon_main_0 has a scope name, but is unreachable, so the name will never be used
rule Packages/Naomi/syntaxes/fjsx15/literal/string/template.sublime-syntax##anon_main_0 has a scope name, but is unreachable, so the name will never be used
rule Packages/Naomi/syntaxes/fjsx15/literal/string/template.sublime-syntax#20893eebc5cc3ea617f6d7708fbf253a::16d7f8c69dc5d9bad0df65b3e033ccbb::#anon_main_0 has a scope name, but is unreachable, so the name will never be used
rule Packages/Naomi/syntaxes/fjsx15/literal/string/template.sublime-syntax#da722399f83b5d5ce019cca5f4a14d31::#anon_main_0 has a scope name, but is unreachable, so the name will never be used
rule Packages/Naomi/syntaxes/fjsx15/literal/string/template.sublime-syntax#20893eebc5cc3ea617f6d7708fbf253a::#anon_main_0 has a scope name, but is unreachable, so the name will never be used

rule Packages/Naomi/syntaxes/fjsx15/literal/string/template.sublime-syntax#13c4e162771b843d63c0bb268a8373dc::#anon_main_0 has a scope name, but is unreachable, so the name will never be used
rule Packages/Naomi/syntaxes/fjsx15/literal/string/template.sublime-syntax##anon_main_0 has a scope name, but is unreachable, so the name will never be used
rule Packages/Naomi/syntaxes/fjsx15/literal/string/template.sublime-syntax#da722399f83b5d5ce019cca5f4a14d31::#anon_main_0 has a scope name, but is unreachable, so the name will never be used
rule Packages/Naomi/syntaxes/fjsx15/literal/string/template.sublime-syntax#20893eebc5cc3ea617f6d7708fbf253a::#anon_main_0 has a scope name, but is unreachable, so the name will never be used

About embed

The LaTeX in 3143 embeds syntaxes like this

        - match: '(\{)(javascript|js)(\})'
          captures:
            1: punctuation.definition.group.brace.begin.latex
            2: variable.parameter.function.latex
            3: punctuation.definition.group.brace.end.latex
          push:
            - meta_include_prototype: false
            - meta_content_scope: meta.environment.embedded.js.latex source.js.embedded
            - match: '(?=\\end\{minted\})'
              pop: true
            - include: scope:source.js

The embed statement in 315x+ is basically a more comprehensive replacement for

        - match: '(\{)(javascript|js)(\})'
          captures:
            1: punctuation.definition.group.brace.begin.latex
            2: variable.parameter.function.latex
            3: punctuation.definition.group.brace.end.latex
          push:
            - meta_include_prototype: false
            - meta_content_scope: meta.environment.embedded.js.latex source.js.embedded
            - include: scope:source.js
          with_prototype:
            - match: '(?=\\end\{minted\})'
              pop: true

The major difference between both is the with_prototype structure, what is basically the escape can be compared to, even though it can do a little bit more.

I don't find related issues in https://github.com/SublimeTextIssues/Core/issues at the moment, but I think I can remember an issue with with_prototype not being handled by main sections the same way like with other sections in a syntax file, which might cause the issue without knowing/seeing as it is not obvious.

If I were you, I'd try to avoid including a whole syntax (including the main ) like

    - include: Packages/Naomi/syntaxes/fjsx15/comment.sublime-syntax

but rather include the stuff in dedicated sections like

    - include: Packages/Naomi/syntaxes/fjsx15.sublime-syntax#comment

This is what I did successfully in the past, when I intended to share some rules. This is what PackageDev does successfully, too.

deathaxe avatar Apr 03 '18 15:04 deathaxe

I am not tracking all of the details of this issue, but I saw I was mentioned.


One of the biggest difference between 3143 and 3160 is that we now have a global regex cache, so if the regex patterns for multiple contexts are the same, we reference a single set instead of allocating new memory for each copy. This would likely explain why 3160 would hit the recursion detection faster, since it likely would not need to re-allocate duplicate regex objects. Generally the memory usage is lower (up to 30% lower total app memory usage), and loading syntaxes is faster.


I would check and make sure the user doesn't have any overrides in places. This could easily cause funky include loops.

wbond avatar Apr 03 '18 16:04 wbond

I should note that the sanity limit of 25,000 (realized) contexts in a single syntax has been around for a long time. Due to the new embed syntax, many syntaxes will actually now have fewer contexts, since contexts don't need to be cloned to handle the embed escape pattern, like with is necessary when using with_prototype. When with_prototype is created, a complete copy of the pushed contexts is created, with the with_prototype patterns prepended. Using embed, the escape pattern is tracked separated from the contexts regex patterns.

At it's worst case, I believe the default syntaxes had a syntax (ASP) that expanded to around 8,000 contexts. Other syntaxes were much more sane, with contexts in the hundreds. I think the second highest was possibly LaTeX with around 3,500. This is largely due to embedding many other complete syntaxes.

wbond avatar Apr 03 '18 16:04 wbond

@deathaxe Those unreachable contexts seam really strange specially the name "anon_main_0", I never name it like that, from what I tested, it was related to this with_prototype, but I don't why is giving off these warnings, the code seams fine.

@wbond Yes, the limit existed before, but this time it is intermittent, sometimes commenting out parts of the syntax makes the error go away, then adding a dummy context makes the error appear again.

borela avatar Apr 03 '18 19:04 borela

Perhaps you've written a syntax with 25,000 contexts?

wbond avatar Apr 03 '18 19:04 wbond

@wbond Also, the time I tested, the sublime's installation was clean, no overrides, but it only happens in build > 3143, I wasn't able to pin point which dev build started the issue.

borela avatar Apr 03 '18 19:04 borela

The unreachable context warnings were fixed in build 3154. It happens when you push into an anonymous context combined with a with_prototype since the with_prototype creates a new copy of the context, and the original copy is never referenced anywhere.

wbond avatar Apr 03 '18 19:04 wbond

There's 467 syntax files in the repo, I rarelly go beyond 10 contexts per file and even in that case, it would be bellow the 25k limit; but like you said, the limit existed in previous builds, so it should happen in the build 3143 too.

borela avatar Apr 03 '18 19:04 borela

Also, the time I tested, the sublime's installation was clean, no overrides, but it only happens in build > 3143

A lot of the default syntaxes have had changes during the current dev cycle, especially the JS syntax. I am sure there are more contexts now than in 3143.

There's 467 files in the repo, I rarelly go beyond 10 contexts per file and even in that case, it would be bellow the 25k limit; but like you said, the limit existed in previous builds, so it should happen in the build 3143 too.

Every include copies the contexts from an external syntax into the current syntax. Every time you use with_prototype a full copy of every transitive context that is referenced from the context you are using with_prototype on is copied also.

wbond avatar Apr 03 '18 19:04 wbond

Isn't the include just a reference to the external file? I thought it was working this way because the cache files are on average ~1kb, the fjsx15 entry point is just 76kb on windows. Is there a function in the API that I can call to count the contexts?

borela avatar Apr 03 '18 19:04 borela

Every syntax is 100% standalone. It will reference the definition from another syntax, but at run time all of the contexts are compiled into the current syntax. This is for performance reasons, and because a syntax can only be used by a single view at a time. It also allows things like with_prototype to work. For instance, the HTML syntax includes the CSS syntax, but (as of build 3143) uses with_prototype to look for the end quote in the attribute containing the CSS. To do this, a copy of every context in the CSS syntax is created with a match looking for the closing quote. We obviously can't modify the original CSS contexts, since this is only supposed to apply in a specific context in the HTML syntax.

There are two cache files for syntaxes, the cache of the parsed, memory-layout of the contexts, and a cache of all of the compiled regex patterns. The context cache file is usually pretty small, but the regex cache (rcache) will get larger as the number of contexts (especially via with_prototype) is increased.

wbond avatar Apr 03 '18 19:04 wbond

Currently there is no API or functionality to count the number of contexts in a syntax

wbond avatar Apr 03 '18 19:04 wbond

I am also having same issue, installed Naomi, will break Markdown package, even the built-in Markdown package. After painful reverting clean slate and bring back packages one by one, found out uninstall Naomi will make them back.

pzgz avatar Apr 23 '18 10:04 pzgz

@pzgz The latest updates in the dev build + naomi hit the limit of 25k contexts, the stable 3143 build does not have this problem.

Currently I am writting a build system to concatenate the syntax files into 1 which will drastically reduce the amount of contexts.

borela avatar Apr 23 '18 12:04 borela

@borela Thanks a lot, I will keep watch on this, though I am also considering that error messages from ST3 is kind of confusing...

pzgz avatar Apr 23 '18 15:04 pzgz

@pzgz It puzzled me too for a while specially because I use the stable build from ST3 where It does not happen. When I wrote Naomi, I broke the syntax into multiple files and every time I needed to reuse some contexts, I just referenced that file.

The issue is that every - include in the syntax instead of just adding a reference, it copies the contexts, the dev builds had the Markdown reworked which does such includes and when combined with Naomi, goes beyond the 25k limit.

After I get the build system working, I'll be able to mantain the current structure but just publish the concatenated file which should make it compatible with the dev build syntaxes.

borela avatar Apr 23 '18 16:04 borela

@borela I'm still getting this using stable 3.1.1, Build 3176. Will this be fixed soon?

Qrokqt avatar Aug 07 '18 21:08 Qrokqt

@Qrokqt Finishing up the toolbox to simplify the development of the build system(I am using JS to develop it) and keep the repo clean from config files.

borela avatar Aug 07 '18 23:08 borela

The Java syntax has the same problem. I never edit Java in sublime text, but now Sublime Merge is giving me the same error if I work with a Java file. I really like the Naomi package so I hope this can be fixed in the future. Thanks for making a great package!

roelandxyz avatar Oct 02 '18 17:10 roelandxyz

As a workaround you can disable unused packages. I disabled most of built-in ones (about 27) and this problem with 25000 limit is gone for me, and Markdown syntax works for me then.

phts avatar Oct 02 '18 18:10 phts