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

Chore: Add semantic highlighting to bash variables and commands #238

Conversation

idillon-sfl
Copy link
Member

@idillon-sfl idillon-sfl commented Jun 7, 2024

In the following example, it adds highlighting to the command do_stuff and the variable $FOO, which were not handled before.

Screenshot from 2024-06-07 03-16-01

@deribaucourt
Copy link
Member

In $FOO, the $ sign should also be highlighted in blue

Copy link
Contributor

@WilsonZiweiWang WilsonZiweiWang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@idillon-sfl
Copy link
Member Author

In $FOO, the $ sign should also be highlighted in blue

I found the default highlighting of Shell code in VS Code quite inconsistent for variables. Here's an example in a shell file:
Screenshot from 2024-06-12 14-16-54

For $FOO and "$FOO", the dollar sign has the same color as the variable name. However, for "${FOO}" it has the color of string, and for ${FOO} it still has different colors. Maybe all of this has some meaning, but I failed to find any sense out of it.

In comparison, here's what we have with this PR:
Screenshot from 2024-06-12 14-16-05

So I've been thinking of various approaches:

  1. We consider the current behavior just as good (or bad?) as the default handling of Shell code and keep it as is.
  2. We consider the behavior in shell files makes sense and we try to do the exact same thing.
  3. We have both $ and ${ the same color as the variable name.
  4. We have both $ and ${ the same color, but distinct to the variable name.
  5. We have $ the same color as the variable name, but ${ with a color distinct to the variable name.

@deribaucourt
Copy link
Member

  1. rent behavior just as good (or bad?) as the default handling of Shell code and keep it as is.

We could report this slight misbehavior in the appropriate repository, this looks like an external issue.

@idillon-sfl
Copy link
Member Author

I suggest we keep it in mind and handle $ and ${ in a separate PR, both the same color, maybe as operators.

@idillon-sfl idillon-sfl merged commit 0b4d55a into yoctoproject:staging Jun 14, 2024
5 checks passed
@idillon-sfl idillon-sfl deleted the Bug-14096-incorrect-highlighting-of-bash-variable-reference-without-curly-braces branch June 14, 2024 14:07
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

Successfully merging this pull request may close these issues.

3 participants