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

fix(snippets): fixed existence-check snippets #1160

Merged
merged 1 commit into from
May 13, 2024

Conversation

alexanderwoehler
Copy link

@alexanderwoehler alexanderwoehler commented May 10, 2024

Snippet-Fix

As of now, the "if-defined" and "if-not-defined" snippets are missing ; then after the condition, which results in broken snippets (see below)

$ cat test.sh
# if-defined
if [[ -n "${variable+x}" ]]
    command ...
fi

# if-not-defined
if [[ -z "${variable+x}" ]]
    command ...
if

$ shellcheck test.sh

In test.sh line 2:
if [[ -n "${variable+x}" ]]
^-- SC1049 (error): Did you forget the 'then' for this 'if'?
^-- SC1073 (error): Couldn't parse this if expression. Fix to allow more checks.


In test.sh line 4:
fi
^-- SC1050 (error): Expected 'then'.
  ^-- SC1072 (error): Unexpected keyword/token. Fix any mentioned problems and try again.

For more information:
  https://www.shellcheck.net/wiki/SC1049 -- Did you forget the 'then' for thi...
  https://www.shellcheck.net/wiki/SC1050 -- Expected 'then'.
  https://www.shellcheck.net/wiki/SC1072 -- Unexpected keyword/token. Fix any...

@alexanderwoehler alexanderwoehler changed the title fix(snippets): fixed existence-check snippets and added parsing snippet fix(snippets): fixed existence-check snippets and added getopts snippet May 10, 2024
@skovhus skovhus enabled auto-merge May 11, 2024 20:04
@skovhus skovhus disabled auto-merge May 11, 2024 20:04
@skovhus
Copy link
Collaborator

skovhus commented May 11, 2024

Thanks. Can you run pnpm lint?

@alexanderwoehler
Copy link
Author

done

$ pnpm lint

> @ lint .../bash-language-server
> pnpm lint:bail --fix


> @ lint:bail .../bash-language-server
> eslint . --ext js,ts,tsx --cache "--fix"

Fixed a typo and pnpm lint reformatted the if-defined and if-not-defined snippets. I "fixupped" these formatting changes together with my initial fix, I hope thats fine :)

@Shane-XB-Qian
Copy link
Contributor

It would be nice to have a snippet for argument parsing using geopts. There already is a PR (see #937) which covers this, but there has not been any activity for months and I do not know why.

please check/refer #897
// you can either help to refine this had been merged snips here,
// or if you were vimer, there local snipmate etc plugins as well, or vsc i guess there were similar ones too.

1 similar comment
@Shane-XB-Qian
Copy link
Contributor

It would be nice to have a snippet for argument parsing using geopts. There already is a PR (see #937) which covers this, but there has not been any activity for months and I do not know why.

please check/refer #897
// you can either help to refine this had been merged snips here,
// or if you were vimer, there local snipmate etc plugins as well, or vsc i guess there were similar ones too.

@alexanderwoehler alexanderwoehler changed the title fix(snippets): fixed existence-check snippets and added getopts snippet fix(snippets): fixed existence-check snippets May 12, 2024
@alexanderwoehler
Copy link
Author

@Shane-XB-Qian i removed the new snippet for getopts so this PR only includes the bugfixes for the two snippets "if-defined" and "if-not-defined"

@Shane-XB-Qian
Copy link
Contributor

um... i was just giving you the background information, if you thought your version of 'getopts' was better then i supposed it is ok to add it still, but just saying there other alternatives for snips, and of course to fix the current had merged is good.

@skovhus skovhus enabled auto-merge May 13, 2024 06:59
@skovhus skovhus merged commit 2d51e28 into bash-lsp:main May 13, 2024
5 checks passed
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