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

Proposal: Allow messages after directives #1771

Closed
3 of 4 tasks
Kreyren opened this issue Dec 11, 2019 · 4 comments
Closed
3 of 4 tasks

Proposal: Allow messages after directives #1771

Kreyren opened this issue Dec 11, 2019 · 4 comments

Comments

@Kreyren
Copy link

Kreyren commented Dec 11, 2019

For bugs

  • Rule ID: SC1072
  • My shellcheck version: 0.7.0
  • The rule's wiki page does not already cover this (e.g. https://shellcheck.net/wiki/SC2086)
  • I tried on shellcheck.net and verified that this is still a problem on the latest commit

For new checks and feature suggestions

Here's a snippet or screenshot that shows the problem:

#!/bin/sh 
debug() {
	message="$1"

	# shellcheck disable=SC2154 - Variable 'debug' is set by the end-user
	[ -n "$debug" ] && printf 'DEBUG: %s\n' "$message"

	unset message
}

Here's what shellcheck currently says:

Expected '=' after directive key. Fix any mentioned problems and try again. [SC1072]

Here's what I wanted or expected to see:

Expecting highlighted message to be valid for shellcheck

# shellcheck disable=SC2154 - Variable 'debug' is set by the end-user
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Reasoning: Less multi-line comments with reasoning to use disable directive compared to:

#!/bin/sh
debug() {
	message="$1"

	# Variable 'debug' is set by the end-user
	# shellcheck disable=SC2154
	[ -n "$debug" ] && printf 'DEBUG: %s\n' "$message"

	unset message
}

Which i believe makes the code less readable especially when there is more comments above the code.

@matthewpersico
Copy link

matthewpersico commented Dec 12, 2019

#!/bin/sh 
debug() {
	message="$1"

	# shellcheck disable=SC2154 # Variable 'debug' is set by the end-user
	[ -n "$debug" ] && printf 'DEBUG: %s\n' "$message"

	unset message
}

$ shellcheck myscript
No issues detected!

Use another comment (#) character. That is what I do.

@Kreyren
Copy link
Author

Kreyren commented Dec 14, 2019

Resolved by @matthewpersico's solution, thanks!

@Kreyren Kreyren closed this as completed Dec 14, 2019
@Kreyren
Copy link
Author

Kreyren commented Mar 4, 2020

Apparently this no longer works?

See https://github.com/gitpod-io/dockerfreeze/pull/54/checks?check_run_id=485963696

In tools/dockerfreeze line 6:
    # shellcheck disable=SC2034 # FIXME: Are cmds used?
                                ^-- SC1107: This directive is unknown. It will be ignored.
                                  ^-- SC1107: This directive is unknown. It will be ignored.
                                         ^-- SC1107: This directive is unknown. It will be ignored.
                                             ^-- SC1107: This directive is unknown. It will be ignored.
                                                  ^-- SC1107: This directive is unknown. It will be ignored.

@Kreyren Kreyren reopened this Mar 4, 2020
Kreyren pushed a commit to Kreyren/dockerfreeze that referenced this issue Mar 4, 2020
@Kreyren
Copy link
Author

Kreyren commented Mar 4, 2020

Resolved: Github is using version: 0.4.6 which apparently does not support this

@Kreyren Kreyren closed this as completed Mar 4, 2020
JesterOrNot pushed a commit to Kreyren/dockerfreeze that referenced this issue Mar 5, 2020
test

test2

parse known issue

final?

final2?

final3?

sfdhgfsdh

sdkgsfh

sagasdg

gjfghj

dsgasd

dgasdg

dasgsag

fdhdfsh

fdhsfd

dgadg

rgfjdfgj

dsgas

dsgasdg

dsagsdg

Finally

Resolve shellcheck

hotfix for koalaman/shellcheck#1771

added bug(?) tracking

Merge from investigation branch

* sagsgda

* test

* sagsad

* fdhs

* dsgadsg

* sagasdg

* fhfgh

* safa

* sagsag

* dsagasdg

* ghdfg

* adgsdg

* ghgkhj

* dsgasd

* agsd

* dgs

* dsgasdg

* dafsd

* dsgasdg

* dasgasdg

* dasgasd

sasag

SAGSDAG

gdfgj

dgasdg

asgasg

dgasdg

safasg

gasdga

gasdga

sadsga

dsgasdg

dsgasdg

reenable tests

Removed cmds
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

No branches or pull requests

2 participants