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

Lint: prepare lib/utilities for shellcheck #1933

Merged
merged 13 commits into from
Sep 28, 2021

Conversation

gaelicWizard
Copy link
Contributor

@gaelicWizard gaelicWizard commented Sep 9, 2021

Description

This is a partial PR covering only the lib/utilities portion of #1917 in support of #1696.

Motivation and Context

My shellcheck branch (#1917) got a bit large, so I'm splitting it up. Each change is split out in to its own commit, so it should be easy to review that way. There are no intended behavior changes whatsoever; this is almost entirely quoting things, plus a few improvements to functions.

How Has This Been Tested?

The full test suite and shellcheck have run on #1917 and this is part of my main branch.

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.

@gaelicWizard gaelicWizard force-pushed the utilities branch 2 times, most recently from 8cf2192 to c2d0a0c Compare September 9, 2021 05:50
@gaelicWizard gaelicWizard mentioned this pull request Sep 9, 2021
6 tasks
@gaelicWizard gaelicWizard force-pushed the utilities branch 9 times, most recently from 545932f to fa017bf Compare September 17, 2021 20:00
@gaelicWizard gaelicWizard force-pushed the utilities branch 2 times, most recently from e61622e to a891206 Compare September 18, 2021 20:24
@gaelicWizard gaelicWizard marked this pull request as ready for review September 19, 2021 07:03
@gaelicWizard gaelicWizard changed the title DRAFT: Lint: prepare lib/utilities for shellcheck Lint: prepare lib/utilities for shellcheck Sep 19, 2021
@gaelicWizard
Copy link
Contributor Author

@cornfeedhobo another PR if you're looking for something to read 😆

@gaelicWizard gaelicWizard force-pushed the utilities branch 2 times, most recently from 3f40cb4 to 9bb19f7 Compare September 20, 2021 05:42
@NoahGorny
Copy link
Member

I like this, this requires a rebase, but looks great overall. You can also add some tests to utilities.bats when you are at it- that would be amazing ❤️

@gaelicWizard
Copy link
Contributor Author

Rebased to current master and all tests pass, including shfmt. I'm working on a branch with a bunch of BATS improvements but that will be later

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.

Great work @gaelicWizard!
Looking forward for a bats PR 😄

@gaelicWizard gaelicWizard force-pushed the utilities branch 2 times, most recently from bc700d4 to 3adb998 Compare September 22, 2021 06:25
Account for plugins with names that contain periods.
Use fewer subprocesses and newline-delimited not space-delimited.
Use fewer subprocesses and return newline-delimited, not space-delimited.
Use `sort -u` instead of `sort | uniq`
Use fewer subprocesses, return newline-delimited instead of space-delimited, and use `sort -u` instead of `uniq | sort`
Use fewer subprocesses, return newline-delimited instead of space-delimited, and use `sort -u` instead of `uniq | sort`
1. Executing `'/usr/bin/grep'` does *not* return the path to grep...
2. use `type -p` instead of external binary `which`.
3. Simplify parameter definition.
4. Why was there a space after `%s`?
Alsö, lose a spurious `cat`
My apologies to future `git blame` hunters ♥
Use `${BASH_IT_LOAD_PRIORITY_SEPARATOR}`
@NoahGorny NoahGorny merged commit 3eed0f0 into Bash-it:master Sep 28, 2021
# filename without path
filename="${1##*/}"
# filename without path or priority
filename="${filename##*${BASH_IT_LOAD_PRIORITY_SEPARATOR?}}"
Copy link
Member

Choose a reason for hiding this comment

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

seems like this generates SC2295 warning, somehow this slipped here @gaelicWizard

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Go figure! I've got a few tiny changes to utilities pending, so I'll throw that in there.

@gaelicWizard gaelicWizard deleted the utilities branch September 29, 2021 22:47
ira-bv pushed a commit to seefood/bash-it that referenced this pull request Oct 10, 2021
* master: (113 commits)
  skip go tests when go is not available
  plugins: Add ble.sh plugin
  clean up pyenv plugin
  Lint: prepare `lib/utilities` for `shellcheck` (Bash-it#1933)
  plugin/base: improvements (Bash-it#1930)
  plugins/percol: `bind`
  completion/git: `shfmt` && `shellcheck`
  completion/git: expand search range
  plugin/percol: `shellcheck` & `shfmt`
  plugins/percol: use `_command_exists`
  completion/pip: simplify code flow
  plugin/less-pretty-cat: remove `|| cat`
  completion/wpscan: simplify code flow (whitespace)
  plugins/less-pretty-cat: simplify code flow
  plugins/less-pretty-cat: use `_command_exists`
  lib/helpers: cite `_bash-it-find-in-ancestor()`
  gradle: adopt `_bash_it_find_in_ancestor()`
  lib/helpers: new function `_bash-it-find-in-ancestor()`
  completion/laravel: simplify code flow
  plugin/ruby: add missing parameter error message
  ...
ira-bv pushed a commit to seefood/bash-it that referenced this pull request Oct 10, 2021
* master: (113 commits)
  skip go tests when go is not available
  plugins: Add ble.sh plugin
  clean up pyenv plugin
  Lint: prepare `lib/utilities` for `shellcheck` (Bash-it#1933)
  plugin/base: improvements (Bash-it#1930)
  plugins/percol: `bind`
  completion/git: `shfmt` && `shellcheck`
  completion/git: expand search range
  plugin/percol: `shellcheck` & `shfmt`
  plugins/percol: use `_command_exists`
  completion/pip: simplify code flow
  plugin/less-pretty-cat: remove `|| cat`
  completion/wpscan: simplify code flow (whitespace)
  plugins/less-pretty-cat: simplify code flow
  plugins/less-pretty-cat: use `_command_exists`
  lib/helpers: cite `_bash-it-find-in-ancestor()`
  gradle: adopt `_bash_it_find_in_ancestor()`
  lib/helpers: new function `_bash-it-find-in-ancestor()`
  completion/laravel: simplify code flow
  plugin/ruby: add missing parameter error message
  ...
ira-bv pushed a commit to seefood/bash-it that referenced this pull request Oct 11, 2021
…autosave-history-plml

* 'master' of https://github.com/bash-it/bash-it: (114 commits)
  ci: Bump go to 1.17 from 1.14
  skip go tests when go is not available
  plugins: Add ble.sh plugin
  clean up pyenv plugin
  Lint: prepare `lib/utilities` for `shellcheck` (Bash-it#1933)
  plugin/base: improvements (Bash-it#1930)
  plugins/percol: `bind`
  completion/git: `shfmt` && `shellcheck`
  completion/git: expand search range
  plugin/percol: `shellcheck` & `shfmt`
  plugins/percol: use `_command_exists`
  completion/pip: simplify code flow
  plugin/less-pretty-cat: remove `|| cat`
  completion/wpscan: simplify code flow (whitespace)
  plugins/less-pretty-cat: simplify code flow
  plugins/less-pretty-cat: use `_command_exists`
  lib/helpers: cite `_bash-it-find-in-ancestor()`
  gradle: adopt `_bash_it_find_in_ancestor()`
  lib/helpers: new function `_bash-it-find-in-ancestor()`
  completion/laravel: simplify code flow
  ...
catull pushed a commit to catull/bash-it that referenced this pull request Oct 21, 2021
* lib/utilities: shellcheck

SC2059

* lib/utilities: fix `_bash-it-get-component-type-from-path()`

Account for plugins with names that contain periods.

* lib/utilities: fix `_bash-it-array-dedup()`

Use fewer subprocesses and newline-delimited not space-delimited.

* lib/utilities: fix `_bash-it-component-list()`

Use fewer subprocesses and return newline-delimited, not space-delimited.

* lib/utilities: fix `_bash-it-component-list-matching()`

Use `sort -u` instead of `sort | uniq`

* lib/utilities: fix `_bash-it-component-list-enabled()`

Use fewer subprocesses, return newline-delimited instead of space-delimited, and use `sort -u` instead of `uniq | sort`

* lib/utilities: fix `_bash-it-component-list-disabled()`

Use fewer subprocesses, return newline-delimited instead of space-delimited, and use `sort -u` instead of `uniq | sort`

* lib/utilities: fix `_bash-it-grep()`

1. Executing `'/usr/bin/grep'` does *not* return the path to grep...
2. use `type -p` instead of external binary `which`.
3. Simplify parameter definition.
4. Why was there a space after `%s`?

* lib/utilities: use `_bash-it-grep`

Alsö, lose a spurious `cat`

* lib/utilities: lint `_bash-it-component-help`

* lib/utilities: lint `_bash-it-component-cache-file()`

* lib/utilities: `shfmt`

My apologies to future `git blame` hunters ♥

* lib/helpers: fix `_bash-it-get-component-name-from-path()`

Use `${BASH_IT_LOAD_PRIORITY_SEPARATOR}`
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.

2 participants