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

Pass --external-sources to shellcheck #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rasky
Copy link

@rasky rasky commented Aug 14, 2019

Many shell scripts use "source" to include other scripts. This option
tells shellcheck to parse those external scripts, so that the remainder
of the script can be properly linted.

Many shell scripts use "source" to include other scripts. This option
tells shellcheck to parse those external scripts, so that the remainder
of the script can be properly linted.
@kaste
Copy link
Contributor

kaste commented Aug 14, 2019

Can't find when they added this flag nor any further descriptions 🙄 Any downsides to just add unconditional for everyone? Otherwise makes sense and lgtm.

@braver
Copy link
Member

braver commented Aug 14, 2019

It's a flag to enable this behavior, so to me that says it's not supposed to be on all the time. I don't feel like having to figure out if there are downsides, or having to take it out again at a later stage because it causes problems for someone. If we bake it in like this, you can't take it out. There is also no need to bake it in: if you want this behavior you can configure args for the linter. So I don't really see a good reason to merge this PR. If I'm wrong, please let me know!

@braver braver closed this Aug 14, 2019
@stdedos
Copy link

stdedos commented Sep 7, 2021

Can't find when they added this flag nor any further descriptions 🙄 Any downsides to just add unconditional for everyone? Otherwise makes sense and lgtm.

See koalaman/shellcheck#902.
Easier and more admin-friendly, as always, is the .shellcheckrc file (but it does not work for a long time, koalaman/shellcheck#1811)

@kaste
Copy link
Contributor

kaste commented Aug 26, 2022

Since for example ale and other IDEs set this as explained in koalaman/shellcheck#902 we can do this too. Though instead of hard-coding it into cmd, doing it via defaults users have the chance to opt-out. For example

defaults = {
	...
	"--external-sources": True,
}

is probably the better patch.

@kaste kaste reopened this Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants