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

bat panics on simple Vue file #915

Closed
sharkdp opened this issue Apr 12, 2020 · 19 comments · Fixed by #917 or #2181
Closed

bat panics on simple Vue file #915

sharkdp opened this issue Apr 12, 2020 · 19 comments · Fixed by #917 or #2181
Labels
bug Something isn't working syntax-highlighting
Milestone

Comments

@sharkdp
Copy link
Owner

sharkdp commented Apr 12, 2020

When calling bat on this Vue file

<style lang="stylus">
</style>

it panics with:

thread 'main' panicked at
  'Can only call resolve on linked references:
    ByScope { scope: <source.stylus>, sub_context: None }',
  …/syntect-4.1.0/src/parsing/syntax_definition.rs:188:18

found with the Python script in #913.

@sharkdp sharkdp added bug Something isn't working syntax-highlighting upstream-error A bug in an upstream component labels Apr 12, 2020
sharkdp added a commit that referenced this issue Apr 12, 2020
sharkdp added a commit that referenced this issue Apr 12, 2020
@sharkdp sharkdp removed the upstream-error A bug in an upstream component label Apr 12, 2020
@sharkdp
Copy link
Owner Author

sharkdp commented Apr 12, 2020

This happens because we didn't have a syntax for "Stylo" source code, which is included from the Vue syntax when using <style lang="stylus">.

@nemu626
Copy link

nemu626 commented May 8, 2020

thread 'main' panicked at 'Can only call resolve on linked references: ByScope { scope: <text.pug>, sub_context: None }', /home/travis/.cargo/registry/src/github.com-1ecc6299db9ec823/syntect-3.3.0/src/parsing/syntax_definition.rs:192:18
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I meet same error with single Vue component file with pug template syntax.

@sharkdp
Copy link
Owner Author

sharkdp commented May 10, 2020

Thank you for reporting this. That's unfortunate.

It should be possible to somehow verify that syntaxes do not include broken references when building the binary assets. This might need to be fixed upstream (in syntect).

@sharkdp sharkdp reopened this May 10, 2020
@Enselic
Copy link
Collaborator

Enselic commented Dec 5, 2020

I don't understand how to reproduce the problem. Can someone please provide a Vue component file with pug template syntax that can be used to reproduce the panic? Maybe you @nemu626 ?

@sharkdp
Copy link
Owner Author

sharkdp commented Dec 6, 2020

I think you can take the one from the first post and replace stylus with one of the other languages supported by the syntax file, e.g. Pug

@Enselic
Copy link
Collaborator

Enselic commented Dec 15, 2020

Turns out it was necessary to also change style to template. Here is how to reproduce with a one-liner:

% echo "<template lang='pug'>\n</template>" > /tmp/crash.vue && bat /tmp/crash.vue
[...]
thread 'main' panicked at 'Can only call resolve on linked references: ByScope { scope: <text.pug>, sub_context: None }', [...]

@keith-hall
Copy link
Collaborator

keith-hall commented Apr 20, 2021

It should be possible to somehow verify that syntaxes do not include broken references when building the binary assets. This might need to be fixed upstream (in syntect).

I've created a PR which will add a new API to syntect for this, but I'm sure it could be improved: trishume/syntect#332

See also #1492

@sharkdp
Copy link
Owner Author

sharkdp commented May 11, 2021

I've created a PR which will add a new API to syntect for this, but I'm sure it could be improved: trishume/syntect#332

Fantastic - thank you! If we implement #1616, we would need to add the necessary "child" syntaxes to further include the Vue syntax, right?

@keith-hall
Copy link
Collaborator

If we implement #1616, we would need to add the necessary "child" syntaxes to further include the Vue syntax, right?

Yep, that's right :)

@keith-hall
Copy link
Collaborator

keith-hall commented Jun 30, 2021

I started work on #1616, and realised the output from syntect wasn't helpful enough, so I created trishume/syntect#338

Linking to this "version" of syntect, now when building assets in bat, we get:

Some referenced contexts could not be found!
- Syntax 'Vue Component' with scope 'text.html.vue' has unresolved context reference ByScope { scope: <source.sss>, sub_context: None }
- Syntax 'Vue Component' with scope 'text.html.vue' has unresolved context reference ByScope { scope: <text.slm>, sub_context: None }
- Syntax 'Vue Component' with scope 'text.html.vue' has unresolved context reference ByScope { scope: <source.livescript>, sub_context: None }
- Syntax 'Vue Component' with scope 'text.html.vue' has unresolved context reference ByScope { scope: <source.postcss>, sub_context: None }
- Syntax 'Vue Component' with scope 'text.html.vue' has unresolved context reference ByScope { scope: <text.pug>, sub_context: None }
- Syntax 'Vue Component' with scope 'text.html.vue' has unresolved context reference ByScope { scope: <text.jade>, sub_context: None }
- Syntax 'Svelte' with scope 'text.html.svelte' has unresolved context reference ByScope { scope: <source.postcss>, sub_context: None }
- Syntax 'Svelte' with scope 'text.html.svelte' has unresolved context reference ByScope { scope: <source.livescript>, sub_context: None }

@sharkdp
Copy link
Owner Author

sharkdp commented Jul 25, 2021

What does it take to integrate this into bat? Do we need additional code or really just a syntect update?

@keith-hall
Copy link
Collaborator

A tiny bit of additional code (to print the warnings to the console at asset building time) - I have already made the necessary changes locally, just waiting for a syntect release before I make a PR. Or is it okay to depend on the syntect git repository directly?

@sharkdp
Copy link
Owner Author

sharkdp commented Jul 25, 2021

just waiting for a syntect release before I make a PR.

Ah, right.

Or is it okay to depend on the syntect git repository directly?

Unfortunately, no. We can do this on master, but we won't be able to release a version on crates.io by referring to a Git hash directly. We need an actual syntect release.

@hongqn
Copy link

hongqn commented Sep 16, 2021

syntect released on Aug 2: https://github.com/trishume/syntect/releases/tag/v4.6.0

@Enselic
Copy link
Collaborator

Enselic commented Oct 24, 2021

Here is a working prototype that should fix this and all other panics in the same family, namely panics caused by attempts to embed syntaxes that are missing: Enselic#52

The proposed solution makes syntect fall back to Plain Text syntax for embedded syntaxes that are missing, which seems to work well. This also seems to be what Sublime Text is doing in cases like these.

The prototype includes a commit that adjust the Vue syntax regression test to also include a snippet of Pug:

<template lang='pug'>
  #container.col
    p This shall be formated as Plain Text as long as a Pug syntax definition is missing
</template>

Current bat crashes in Vue files with the above, but the prototype handles it well (continues to highlight subsequent Vue code as before).

Further progress on this currently blocked mostly by trishume/syntect#382 which the syntect code is based on to reduce future conflicts. But as soon as that is merged I plan on resuming work on this.

If you have any concerns with regards to falling back to Plain Text syntax in cases like these, I would love to hear them!

@sharkdp
Copy link
Owner Author

sharkdp commented Nov 22, 2021

@Enselic can this be closed with #1915 merged?

@sharkdp sharkdp added this to the v0.19 milestone Nov 22, 2021
@Enselic
Copy link
Collaborator

Enselic commented Nov 22, 2021

@sharkdp Nope, Vue still has unresolved syntaxes (

- Syntax 'Vue Component' with scope 'text.html.vue' has unresolved context reference ByScope { scope: <source.postcss>, sub_context: None }
- Syntax 'Vue Component' with scope 'text.html.vue' has unresolved context reference ByScope { scope: <source.sss>, sub_context: None }
- Syntax 'Vue Component' with scope 'text.html.vue' has unresolved context reference ByScope { scope: <text.jade>, sub_context: None }
- Syntax 'Vue Component' with scope 'text.html.vue' has unresolved context reference ByScope { scope: <text.pug>, sub_context: None }
- Syntax 'Vue Component' with scope 'text.html.vue' has unresolved context reference ByScope { scope: <text.slm>, sub_context: None }

)

I am still convinced that my proposed approach to solve it "once and for all" (#915 (comment)) is the right one, but I actually think we should wait with fixing this until after 0.19.0. It is a bit tricky to get the code right in syntect, and trishume seems busy these days, so there is a risk that that work would delay a release several months more in the worst case.

@sharkdp sharkdp removed this from the v0.19 milestone Nov 22, 2021
@sharkdp
Copy link
Owner Author

sharkdp commented Nov 22, 2021

Understood, thank you. Removed the milestone (which - anyway - was only a suggestion).

@Enselic
Copy link
Collaborator

Enselic commented Mar 12, 2022

I finally got around polishing the prototype code, and opened a PR with production ready code for syntect that fixes this whole class of bugs: trishume/syntect#427

(Before this is fixed in bat, syntect needs to release 5.0.0 and bat needs to start depending on that release of course, so it might still take a while before this is fixed for bat end-users)

@Enselic Enselic added this to the v0.21.0 milestone Mar 29, 2022
Enselic added a commit to Enselic/bat that referenced this issue May 4, 2022
Enselic added a commit that referenced this issue May 7, 2022
* Bump to syntect 5.0.0 to e.g. start lazy-loading themes

Closes #915
Closes #951
Closes #1846
Closes #1854

* Typo fix formated -> formatted

* Update CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working syntax-highlighting
Projects
None yet
5 participants