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

The first command is not parsed correctly when using from other grammars #79

Closed
Robot-Inventor opened this issue Mar 11, 2024 · 5 comments

Comments

@Robot-Inventor
Copy link

Robot-Inventor commented Mar 11, 2024

When you import better-shell-syntax from other grammars using include, the first command is not parsed correctly.

The issue was originally reported in shikijs/shiki#616 by @sventec. Upon investigation, we found that it is a grammar issue. (I'm not familiar with TextMate so sorry if my thoughts are wrong.)

This problem occurs when better-shell-syntax is included from the shellsession grammer. On investigation, the shellsession grammar does not seem to have the problem.

For example, if the following code is passed to the shellsession grammar,

user@test$ echo -n $EDITOR | grep -E "vim"

the following code is passed to better-shell-syntax, and the first command echo -n is not parsed correctly.

echo -n $EDITOR | grep -E "vim"

image

echo -n should be split into multiple tokens as shown in the following image, but is actually processed as a single token.

image

@jeff-hykin
Copy link
Owner

jeff-hykin commented Mar 11, 2024

I'm not familiar with TextMate

Yeah dont worry. I'm pretty sure the guy that implemented Textmate wasn't familiar with Textmate.

The reason the first command isn't parsed correctly is because the syntax has to use a bunch of hacks to make anything work. One of those hacks is detecting the start by "looking behind". In normal shell files, at the top level (e.g. not in a comment or if statement etc), a command can only have a few chars come before it (ex: && || ; !) so when it looks behind and sees $ it thinks "ah this can't be a command, it must be an argument"

In its current state, changing that behavior would break the normal shell grammar.

Ideally when grammar1 tells grammar2 to run on some text, it would make pre-text (like user@test$) completely invisible to grammar2. Unfortunately Textmate sucks, so that's absolutely not what happens, and instead we get problems like these.

I also made a Docker grammar, which includes a shell grammar. I had to basically fork my own shell grammar, make some small changes, to create a shell-for-docker grammar, then direftly include the shell-for-docker grammar in the docker grammar. Its not great, but there isn't really another option thanks to the crappy parsing engine.

@Robot-Inventor
Copy link
Author

Thank you for your quick and thoughtful reply.

Am I correct in understanding that due to TextMate's specifications, it is not possible to fix this issue without breaking the normal shell syntax?

It's pretty tricky not being able to hide the pre-text from subsequent grammars...

I'll see what other solutions I can come up with, such as forking the shell syntax. Thank you very much!

@jeff-hykin
Copy link
Owner

it is not possible to fix this issue without breaking the normal shell syntax?

Hard to say its impossible, but it certainly would make the shell syntax harder to maintain.

I'll see what other solutions I can come up with

If you fork, open up the main.rb and Ctrl+f for possible_pre_command_characters. I bet adding $ to that, and then running project build will generate a json grammar that will work for your case

@jeff-hykin
Copy link
Owner

Oh and also here's the docs for setting up the project: https://github.com/jeff-hykin/better-shell-syntax/blob/master/documentation/setup.md

@Robot-Inventor
Copy link
Author

It worked perfectly! Thank you so much for taking the time to provide such detailed instructions. I really appreciate your help. I'm going to close this issue now since the problem has been resolved.

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

No branches or pull requests

2 participants