Skip to content

Sourcing: "strip one leading, dynamic section before trying to locate the rest" #926

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

Closed
aarondill opened this issue Aug 3, 2023 · 2 comments · Fixed by #1086
Closed
Labels
enhancement New feature or request

Comments

@aarondill
Copy link
Contributor

What is the problem this feature will solve?

When writing code like this:

here="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
. "$here/config.sh"
. "$here/utils.sh"

shellcheck will understand that it should search the script dir for the files, but bash lsp doesn't.

What is the feature you are proposing to solve the problem?

To support the common pattern of . "$CONFIGDIR/mylib.sh", ShellCheck strips one leading, dynamic section before trying to locate the rest. That means that ShellCheck will look for config.sh and utils.sh in the same directory as this script when it is being checked:

So, let's strip one leading, dynamic section before trying to locate the rest 😄

This can work with #811 by only taking effect when the variable is truly dynamic, instead using the value if it's constant (ie set earlier in the script)

What alternatives have you considered?

# shellcheck source=./.utils.d/output.sh - this gets quite frustrating when shellcheck understands the location of the file, but bash-lsp doesn't

@aarondill aarondill added the enhancement New feature or request label Aug 3, 2023
@aarondill
Copy link
Contributor Author

See here for shellcheck's documentation about this pattern.

aarondill added a commit to aarondill/bash-language-server that referenced this issue Jan 21, 2024
This does the same thing that ShellCheck does ([here](https://github.com/koalaman/shellcheck/blob/ba86c6363c30a5dbefd0b8b9a7c5f4ab0478dc91/src/ShellCheck/Parser.hs#L2277-L2283)).

`To support the common pattern of . "$CONFIGDIR/mylib.sh", ShellCheck strips one leading, dynamic section before trying to locate the rest.`

fixes bash-lsp#926, fixes bash-lsp#659
aarondill added a commit to aarondill/bash-language-server that referenced this issue Feb 6, 2024
This does the same thing that ShellCheck does ([here](https://github.com/koalaman/shellcheck/blob/ba86c6363c30a5dbefd0b8b9a7c5f4ab0478dc91/src/ShellCheck/Parser.hs#L2277-L2283)).

`To support the common pattern of . "$CONFIGDIR/mylib.sh", ShellCheck strips one leading, dynamic section before trying to locate the rest.`

fixes bash-lsp#926, fixes bash-lsp#659
@justinTM
Copy link

this would be huge

aarondill added a commit to aarondill/bash-language-server that referenced this issue Apr 13, 2025
This does the same thing that ShellCheck does ([here](https://github.com/koalaman/shellcheck/blob/ba86c6363c30a5dbefd0b8b9a7c5f4ab0478dc91/src/ShellCheck/Parser.hs#L2277-L2283)).

`To support the common pattern of . "$CONFIGDIR/mylib.sh", ShellCheck strips one leading, dynamic section before trying to locate the rest.`

fixes bash-lsp#926, fixes bash-lsp#659

fix: handle leading dynamic source paths when concatenating

This fixes `${var}/path` and `"${var}"/path` as well as countless variations of these patterns

Note: this turned out to be rather complex task, so it's broken out into a seperate function, which allows for early returns

refactor: move resolveStaticString to util/tree-sitter.ts

This seemed the most appropriate place for it.

tests: fix existing snapshot and add test for dynamic source paths

Adds a simple test for dynamic source paths and updates the snapshot to match new tests / behaivor.

Note: all the line numbers in the snapshot increased by 2 because I added two new lines (the test and a blank one)

tests: add new test in fixtures/sourcing.sh

I added this to the bottom of the file to minimize snapshot diffs. they're large enough as it is.
aarondill added a commit to aarondill/bash-language-server that referenced this issue Apr 13, 2025
This does the same thing that ShellCheck does ([here](https://github.com/koalaman/shellcheck/blob/ba86c6363c30a5dbefd0b8b9a7c5f4ab0478dc91/src/ShellCheck/Parser.hs#L2277-L2283)).

`To support the common pattern of . "$CONFIGDIR/mylib.sh", ShellCheck strips one leading, dynamic section before trying to locate the rest.`

fixes bash-lsp#926, fixes bash-lsp#659

fix: handle leading dynamic source paths when concatenating

This fixes `${var}/path` and `"${var}"/path` as well as countless variations of these patterns

Note: this turned out to be rather complex task, so it's broken out into a seperate function, which allows for early returns

refactor: move resolveStaticString to util/tree-sitter.ts

This seemed the most appropriate place for it.

tests: fix existing snapshot and add test for dynamic source paths

Adds a simple test for dynamic source paths and updates the snapshot to match new tests / behaivor.

Note: all the line numbers in the snapshot increased by 2 because I added two new lines (the test and a blank one)

tests: add new test in fixtures/sourcing.sh

I added this to the bottom of the file to minimize snapshot diffs. they're large enough as it is.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants