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

embed causing recursion #2252

Closed
borela opened this issue Apr 2, 2018 · 11 comments
Closed

embed causing recursion #2252

borela opened this issue Apr 2, 2018 · 11 comments
Labels

Comments

@borela
Copy link

borela commented Apr 2, 2018

Summary

I am not sure if this is a core problem or just the syntax, but some friends and users are reporting recursion in the LaTeX syntax which does not happen in the build I use (3143).

borela/naomi#117

Expected behavior

To work as before or better error messages.

Actual behavior

Recursion and no explanation.

From the diffs, the syntax suffered many changes but I am thinking that the issue is the embed action as it is the only major change I saw.

Steps to reproduce

  1. Use a dev build;
  2. Install Naomi;
  3. Try to use the builtin LaTeX syntax;
@deathaxe
Copy link
Collaborator

deathaxe commented Apr 3, 2018

As using the LaTeX package from ST3143 in ST3160 doesn't "fix the issue" a change in core logic might trigger or IMHO reveal the issue.

@wbond
Copy link
Member

wbond commented Apr 3, 2018

embed itself won't ever cause recursion, since embed doesn't manipulate the contexts, but instead tracks the escape pattern separately from the regex patterns used by matches in the contexts. It is designed and implemented to reduce the number of contexts a syntax has.

From your issue, the problems is that a syntax is using over 25,000 contexts. This is a sanity check, to catch situations where with_prototype actions are recursively creating new contexts.

The more discussion around the issue, the more it seems to me the possibility is that you've somehow managed to create a syntax that has 25,000 legitimate contexts. This seems relatively unlikely, but if you are creating syntaxes that are including all sorts of other syntaxes, and you are using with_prototype to do it, you are creating a full copy of every context in the syntax for each unique with_prototype. Depending on which syntaxes you are including, and if they are including other syntaxes with with_prototype, the situation can get out of hand quickly. This is why the embed action was created, to help reduce the proliferation of contexts when with_prototype is used.

Purely in and of itself, using the same packages from 3143 in 3160 should not exhibit the problem, if the number of contexts is the issue. It is possible that all of the changes and improvements to the default syntaxes in the current dev cycles has increased the number of contexts to the point where combined with Naomi hits the limit.

@borela
Copy link
Author

borela commented Apr 3, 2018

I'll make some tests by combining all files into a single one.

@Thom1729
Copy link

Thom1729 commented Apr 4, 2018

How does with_prototype work when combined with embed? Does it keep track of the added rules separately? E.g., run the escape rule, then run the with_prototype rules, then run the embedded rules.

I'm dealing with a similar issue with JS Custom (under different, more limited circumstances) and I think that embed/escape should solve it.

@rwols
Copy link

rwols commented Apr 4, 2018

Would it be possible to do a git-bisect between the tag v3143 and HEAD?

@wbond
Copy link
Member

wbond commented Apr 4, 2018

@Thom1729 If you with_prototype a context that contains an embed is converted to with_prototype itself. Since with_prototype allows for things other than escape-like patterns, we can't shoehorn it in there.

with_prototype is kind of nasty in terms of it being a simple syntax, but with potentially huge effects. I think it tends to be used more than perhaps Jon originally thought it would be.

@blake-regalia
Copy link

@wbond While I was originally looking forward to using embed, I was a bit disappointed to realize that doesn't play nice with interpolation, such as with JavaScript template literals, since you can only pop out. I understand how with_prototype can be 'nasty', but we have a legitimate use-case for it to be used with embed, namely for pushing parent contexts from within the nested syntax. See bathos/Ecmascript-Sublime#50 (comment)

@wbond
Copy link
Member

wbond commented May 9, 2018

@blake-regalia embed is effectively an optimized version of with_prototype for exactly one purpose - wholesale embedding of one syntax in another. As soon as you want to do other things, you'll need to use with_prototype. There isn't anything wrong with that, it just results in higher memory usage, and causes a proliferation of contexts when used many times. But if you want to embed another syntax and add stuff to it, you need to use with_prototype.

An example of the issue related to with_prototype:

  • Imagine writing syntax for FooLang
  • FooLang has heredocs that support variables
  • You want to include support for HTML, Markdown, JS, CSS in the heredocs
  • You use with_prototype to add heredocs syntax highlighting so variables are supported
  • Markdown includes a whole bunch of other syntaxes in fenced code blocks
  • Now instead of 300 contexts for FooLang, you have 2x all of the contexts for HTML, JS, CSS, Markdown, Python, SQL, XML, Graphviz, JSON, Java, C#, ShellScript, R, PHP, Go, Ruby, Objective-C, Objective-C++, C, C++ and Regular Expressions. So now the syntax is easily 10k+ contexts.

@blake-regalia
Copy link

@wbond Thanks for the reply. I actually did not realize that set also supports scope:source.foolang, which is awesome! The only other qualm I had was that I assumed escape was only allowed to be used with embed, which I have now discovered is not the case. I guess my assumptions mislead me, and perhaps it could be more clear in the docs. I think these are terrific features! Thanks

@blake-regalia
Copy link

Actually, it seems that even though Sublime does not raise an error, escape has no effect when used with set. Is this true?

@deathaxe
Copy link
Collaborator

escape is dedicated function for use with embed.

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

No branches or pull requests

8 participants