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

Consider using a better maintained shellscript grammar #77675

Closed
mk-pmb opened this issue Jul 19, 2019 · 19 comments
Closed

Consider using a better maintained shellscript grammar #77675

mk-pmb opened this issue Jul 19, 2019 · 19 comments
Assignees
Labels
feature-request Request for new features or functionality grammar Syntax highlighting grammar upstream Issue identified as 'upstream' component related (exists outside of VS Code) verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@mk-pmb
Copy link

mk-pmb commented Jul 19, 2019

Please search existing issues to avoid creating duplicates.

Duplicate of locked #40956 , which redirected the discussion to atom/language-shellscript#100 , where I was told it's no longer their problem:

VSCode is still using the old TextMate grammar, which the Atom team no longer maintains as we have moved many grammars (including this one) over to Tree-sitter.

·

Also please test using the latest insiders build to make sure your issue has not already been fixed:

Can't do that right now, maybe next week. In case it was really fixed, please consider this issue as a request to annotate the locked thread with updated info.

  • VSCode Version: 1.36.1
  • OS Version: Ubuntu 16.04 LTS (xenial)

Steps to Reproduce:

  1. Create a bash file moo.sh:
#!/bin/bash
nl <(sed -nre 's!^\s+!!p' <<'  __FOO__'
  ########################
  ## Dummy Message      ##
  ########################
  __FOO__
  )
echo bar
  1. Compare with my screenshot.
  2. Run it and observe that bash considers "echo" a command of its own.
  3. Unident __FOO__ to see the flipside of this bug: VSCode now thinks everything is ok.
  4. Run again, see that in reality, unindenting broke it:
./moo.sh: line 2: unexpected EOF while looking for matching `)'
./moo.sh: line 9: syntax error: unexpected end of file

Does this issue occur when all extensions are disabled?: Yes

@alexr00 alexr00 added the grammar Syntax highlighting grammar label Jul 29, 2019
@alexr00 alexr00 added this to the Backlog milestone Jul 29, 2019
@alexr00
Copy link
Member

alexr00 commented Jul 29, 2019

Leaving open to check on if we decided to update the old shell script grammar.

@marcelbrueckner
Copy link

Probably the same issue here. I have an intended Heredoc whose delimiter isn't recognized correctly because of indentation. Although I'm using the <<- syntax as stated in man bash

If the redirection operator is <<-, then all leading tab characters are stripped from input lines and the line containing delimiter. This allows here-documents within shell scripts to be indented in a natural fashion.

if true; then
    cat <<-'EOF' > /tmp/file
        # My script here
    EOF
fi

@mk-pmb
Copy link
Author

mk-pmb commented Aug 28, 2019

Sorry for my wrong gist a moment ago. I then noticed it's limited to tabs.

@robmonte
Copy link

robmonte commented Oct 2, 2019

I have found that it seems to break with the standard EOF too:

echo "here is some $SYNTAX coloring"
cat << EOF > myfile.txt
multiple line
output into
a file
EOF
echo "here is some more $SYNTAX coloring"

As you can see, this works on Github itself. While it works here, this breaks in VSCode:
image

@lucasbasquerotto
Copy link

lucasbasquerotto commented Nov 5, 2020

Related: #102958 (Wrong syntax highlighting when using \. in regex inside HEREDOC)

@alexr00 alexr00 added the feature-request Request for new features or functionality label Nov 9, 2020
@Lat31320
Copy link

Hi,

Please consider that type of code too (var=` ... )

#!/bin/sh
if [ -d "$directory" ]
    then
# syntax broken after
        mount_script=`/usr/bin/osascript <<EOF
            tell application "Finder"
            activate
            mount volume "smb://$myshare"
            end tell
            EOF`
    else
        echo blah
fi
echo another code here
exit 0

The script is running but it's a pain to working on it (the real one counts ~400 lines). The only fix I found (during script editing only) is to move "EOF`" at the very begin of the line AND remove the ending "`" char.

#!/bin/sh
if [ -d "$directory" ]
    then
        mount_script=`/usr/bin/osascript <<EOF
            tell application "Finder"
            activate
            mount volume "smb://$myshare"
            end tell
EOF
    else
        echo blah
fi
echo another code here
exit 0

Despite the coloration is broken, VSCODE is correctly find the <<EOF .... EOF delimiters as it highlights the both when I select one.

VS Code version: Code 1.52.1 (ea3859d, 2020-12-16T16:30:02.420Z)
OS version: Darwin x64 19.6.0

System Info
Item Value
CPUs Intel(R) Core(TM) i7-5650U CPU @ 2.20GHz (4 x 2200)
GPU Status 2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
oop_rasterization: enabled
opengl: enabled_on
protected_video_decode: unavailable_off
rasterization: enabled
skia_renderer: disabled_off_ok
video_decode: enabled
webgl: enabled
webgl2: enabled
Load (avg) 2, 2, 3
Memory (System) 8.00GB (0.08GB free)
Process Argv --crash-reporter-id 81a3ae87-315d-47fd-b117-7193c28612c0
Screen Reader no
VM 0%
Extensions (6)
Extension Author (truncated) Version
xml Dot 2.5.1
shell-format fox 7.0.1
bash-extension-pack liz 0.0.4
shellman Rem 4.9.0
bash-debug rog 0.3.9
shebang-snippets rpi 0.1.4
A/B Experiments
vsliv368cf:30146710
vsreu685:30147344
openlogontheside:30221877
python383:30185418
vspor879:30202332
vspor708:30202333
vspor363:30204092
vswsl492cf:30211402
wsl2prompt:30224612
vstry914:30244301
pythonvsdeb440:30242242
unusedpromptcf:30224611
folderexplorercf:30224615
openfilemenu:30224647
pythonvsded773:30236629
pythonvspyt600:30241727

@rightaway
Copy link

In this script $variable1 and $variable2 should have the same color as other variables. And /filename should have the same color as it would in echo text > $variable/filename. Currently it all has the same color as the body of the HEREDOC.

cat <<- HEREDOC > $variable1/filename
  text="some=$variable2"
HEREDOC

@fabriciomurta
Copy link

One more to add to the list of issues with bash syntax highlighting:

close_t='"2011-04-22T13:33:48Z"'
if [[ "${close_t}" =~ ^\"([0-9]{4}-[0-9]{1,2}-[0-9]{1,2})T(([0-9]{1,2}:){2}[0-9]{1,2}Z)\" ]]; then
  echo "It is in expected format."
else
  echo "It is not in the expected format."
fi

But it breaks highlight just the next line in vscode. The broken behavior is triggered by the (( in the regex.

GitHub's syntax highlight goes a bit farther and breaks the syntax highlighting in the remainder of the code block. :)

This is how it looks in VSCode 1.59.1:
image

Just in case, vim's sh syntax handles this case well (tested version 189 of that file, it is currently at 198).

@tddschn
Copy link

tddschn commented Sep 11, 2021

One more to add to the list of issues with bash syntax highlighting:

CleanShot-2021-09-11T23-06-28@2x

Code:

header <wine-pca.txt -a pc1,pc2 |
	paste -d, - <(xsv select type wine-scaled.csv) |
	tee wine-pca.csv | csvlook

All codes after this command lose syntax highlighting (shown as the same color as comments).

How it is supposed to look like:
CleanShot-2021-09-11T23-08-54@2x

@taqtiqa-mark
Copy link

taqtiqa-mark commented Jun 13, 2022

Another more current example:

image

function get_source() {
    if [[ -z ${RTM_TPL_SOURCE-} ]]
    then
        RTM_TPL_SOURCE="$(enquirer select --cancel --default --selected 1 --message 'Install RTM from?' -- crates.io github.com)"
        export RTM_TPL_SOURCE
    fi
    verbose_print "RTM source: ${RTM_TPL_SOURCE}"
}

Version: 1.68.0
Commit: 4af164e
Date: 2022-06-08T11:49:57.055Z
Electron: 17.4.7
Chromium: 98.0.4758.141
Node.js: 16.13.0
V8: 9.8.177.13-electron.0
OS: Linux x64 5.4.0-117-generic

@lucasbasquerotto
Copy link

That seems an issue with the select in the string, it seems to (wrongly) mess things up.

It happens even here in github:

function get_source() {
    if [[ -z ${RTM_TPL_SOURCE-} ]]
    then
        RTM_TPL_SOURCE="$(a select b c)"
        export RTM_TPL_SOURCE
    fi
    verbose_print "RTM source: ${RTM_TPL_SOURCE}"
}

@taqtiqa-mark
Copy link

taqtiqa-mark commented Jun 13, 2022

@alexr00 not sure if this helps or is noise, but it appears the only bash language server implementation points to the tree-sitter grammar for bash.

That grammar repository does appear to be maintained, but there would appear to be some setup required to test if that resolves the issues reported here.

Its not clear to me what additional VSCode setup would be required....

Maybe all this?

https://github.com/sourcegraph/lsp-adapter/blob/master/dockerfiles/bash/run-bash-language-server.sh

@alexr00
Copy link
Member

alexr00 commented Jun 14, 2022

@taqtiqa-mark I appreciate the pointer, but we don't currently have syntax highlighting support with treesitter: #50140

sei-mkaar added a commit to cmu-sei/foundry-appliance that referenced this issue Aug 17, 2022
Rewrite code to add root CA certificate to Helm values files.

Fixes longstanding issue where VSCode does not syntax highlight subsequent
lines properly:
microsoft/vscode#77675
sei-mkaar added a commit to cmu-sei/foundry-appliance that referenced this issue Aug 17, 2022
Rewrite code to add root CA certificate to Helm values files.

Fixes longstanding issue where VSCode does not syntax highlight subsequent
lines properly:
microsoft/vscode#77675
sei-mkaar added a commit to cmu-sei/foundry-appliance that referenced this issue Aug 17, 2022
Rewrite code to add root CA certificate to Helm values files.

Fixes longstanding issue where VSCode does not syntax highlight subsequent
lines properly:
microsoft/vscode#77675
@alexr00
Copy link
Member

alexr00 commented Nov 22, 2022

@jeff-hykin do you plan to continue to maintain https://github.com/jeff-hykin/better-shell-syntax? I'm considering trying it out as the default grammar for shell script as the current one hasn't been touched in a long time.

@jeff-hykin
Copy link

@alexr00 over the last week (in response to this) I cleaned up the better shell syntax and stablized it.

@people-on-this-thread please do install/test the extension. It should ready for VS Code insiders, but it'd be nice to get some feedback to confirm there are no regressions. There's known bugs, but currently no known regressions.

#156054 is already fixed
#152327 is already fixed
#166905 is already fixed
#139913 is already fixed (hi @lilyball 👋 , good to see you outside nix)
#147293 is already fixed
The select issue mentioned a few comments above is fixed

#147673 is not
#77675 (code example at top of this issue) is not

@alexr00
Copy link
Member

alexr00 commented Dec 14, 2022

@jeff-hykin this is great news, thank you!

I'll pull your grammar into VS Code now. We're not having a December release so it will have lots of time in VS Code insiders for folks to notice if there are regressions before we release.

@alexr00
Copy link
Member

alexr00 commented Dec 14, 2022

You can see a visual comparison of the differences in the new grammar in #169118.
Specific token differences can be seen in test_sh.json.

@alexr00 alexr00 modified the milestones: Backlog, January 2023 Dec 14, 2022
alexr00 added a commit that referenced this issue Dec 14, 2022
weartist pushed a commit to weartist/vscode that referenced this issue Dec 15, 2022
@alexr00
Copy link
Member

alexr00 commented Jan 6, 2023

The new grammar has been in VS Code insiders for 3 weeks with no issues reported. Closing as fixed!

@alexr00 alexr00 closed this as completed Jan 6, 2023
@alexr00
Copy link
Member

alexr00 commented Jan 23, 2023

This has been verified by the integration tests.

@alexr00 alexr00 added verified Verification succeeded verification-needed Verification of issue is requested labels Jan 23, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Feb 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality grammar Syntax highlighting grammar upstream Issue identified as 'upstream' component related (exists outside of VS Code) verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests