Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Allow ShellCheck source= directive to be relative to project root instead of file path #124

Merged
merged 6 commits into from
Aug 23, 2018

Conversation

tbehling
Copy link
Contributor

This PR introduces a new config setting for linter-shellcheck called useProjectCwd, which controls whether the paths used by ShellCheck's source= directive are relative to the project root or the file.

Background & Context

The ShellCheck tool supports a linter directive called source= that works like the following:

# shellcheck source=src/examples/config.sh
. "$(locate_config)"

In the current implementation of linter-shellcheck, the shellcheck command executes with a "current working directory" based on the file's path. The outcome is all source= paths must be relative to the file's directory.

While working with ShellCheck on a large project, I found it more convenient to reference absolute file paths from the source= directive. This is particularly helpful when running the shellcheck command from CI scripts. Without this ability, developers see different ShellCheck errors in Atom vs. CI results or a command line linter invocation.

To provide an option to homogenize the ShellCheck experience between Atom and typical CLI usage, this PR does the following:

  • Introduces a new boolean config flag, useProjectCwd.
    • This defaults to false to maintain the existing behavior (file-relative paths).
    • Users can opt-in by checking this box to gain project-relative paths.
  • Alters the "working directory" behavior when executing shellcheck to use either the project root or the file's directory.
  • Adds specs and needed fixtures to exercise both the existing and new behavior.
  • Updates the project README.

Copy link
Member

@Arcanemagus Arcanemagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor change to how the configuration is specified, other than that this looks good!

package.json Outdated
},
"useProjectCwd": {
"type": "boolean",
"title": "Run Shellcheck using project root as the working directory (false for file-relative)",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be the description, although you may still want a title attribute to override the default of "Use Project Cwd" due to the "Cwd" part.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this feedback Landon, great point. I've updated the metadata and here's what it looks like now:

screen shot 2018-08-23 at 2 52 12 pm

Copy link
Member

@Arcanemagus Arcanemagus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Arcanemagus Arcanemagus merged commit 9fc415f into AtomLinter:master Aug 23, 2018
@tbehling tbehling deleted the offer-project-cwd branch August 23, 2018 20:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants