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

Revert most of "Fix linting errors that snuck in." #2000

Merged
merged 1 commit into from
Jan 1, 2022

Conversation

gaelicWizard
Copy link
Contributor

@gaelicWizard gaelicWizard commented Dec 28, 2021

Description

This reverts most of commit 2c8ee40.

Motivation and Context

  • Shellcheck documentation for the source-path directive indicates this is correct usage. We're sourceing the bash-preexec.sh file from inside the vendor/github.com/rcaloras/bash-preexec directory. If we used the source directive, then the full complete path to the file itself would need to be specified.
  • Fix disable=1090 to disable=SC1090 and remove duplicate lines since this shellcheck directive will apply to the entire if-ladder.
  • Disabling SC2154 is almost never appropriate. In this case, several _git_bash_completion* variables are expressly assigned in this file, so there is no "unknown" to ignore.

Aside: the ${!_git_bash_completion@} construct will expand to all variables starting with the prefix _git_bash_completion, so this line is just a shorthand way to clear all our variables concisely without forgetting any.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • If my change requires a change to the documentation, I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • If I have added a new file, I also added it to clean_files.txt and formatted it using lint_clean_files.sh.
  • I have added tests to cover my changes, and all the new and existing tests pass.

This reverts commit 2c8ee40.

- _Shellcheck_ documentation for the [`source-path`]( https://github.com/koalaman/shellcheck/wiki/Directive#source-path ) directive indicates this is correct usage. We're `source`ing the `bash-preexec.sh` file from inside the `vendor/github.com/rcaloras/bash-preexec` directory. If we used the [`source`]( https://github.com/koalaman/shellcheck/wiki/Directive#source ) directive, then the full complete path to the file itself would need to be specified.
- Fix `disable=1090` to `disable=SC1090` and remove duplicate lines since this `shellcheck` directive will apply to the entire if-ladder.
- Disabling `SC2154` is almost never appropriate. In this case, several `_git_bash_completion*` variables are expressly assigned in this file, so there is no "unknown" to ignore.

Aside: the `${!_git_bash_completion@}` construct will expand to all variables starting with the previx `_git_bash_completion`, so this line is just a shorthand way to clear all our variables concisely without forgetting any.
@gaelicWizard
Copy link
Contributor Author

@cornfeedhobo, you may have an old or broken shellcheck on your system! ♥

@gaelicWizard gaelicWizard marked this pull request as ready for review December 28, 2021 00:24
Copy link
Member

@NoahGorny NoahGorny left a comment

Choose a reason for hiding this comment

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

Hmm, seems like we should always check with the newest shellcheck (or the one we use in the CI at least)

@NoahGorny NoahGorny merged commit 2e51e92 into Bash-it:master Jan 1, 2022
@gaelicWizard gaelicWizard deleted the lint branch January 1, 2022 22:00
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.

None yet

2 participants