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.

Sign up for GitHub

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

Jump to bottom

plugin/base: improvements #1930

Merged
merged 16 commits into from
Sep 28, 2021
Merged

plugin/base: improvements #1930

merged 16 commits into from
Sep 28, 2021

Conversation

gaelicWizard
Copy link
Contributor

@gaelicWizard gaelicWizard commented Sep 7, 2021 •
edited
Loading

Description

I've run shellcheck on base.plugin.bash and made a bunch of tweaks to the functions. No behavior changes are intended except the removal of banish-cookies() and command_exists().

Motivation and Context

Just trying to clean up.

How Has This Been Tested?

Tested locally, and all tests pass.

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 plugin-base branch 4 times, most recently from 33d4a07 to 73b59f4 Compare September 9, 2021 01:59
@gaelicWizard gaelicWizard marked this pull request as ready for review September 9, 2021 01:59
@gaelicWizard gaelicWizard changed the title DRAFT: Base plugin improvements Base plugin improvements Sep 9, 2021
@gaelicWizard gaelicWizard force-pushed the plugin-base branch 3 times, most recently from 6f7b2de to 454be11 Compare September 9, 2021 07:31
@gaelicWizard
Copy link
Contributor Author

Is anything else needed for this PR?

NoahGorny
NoahGorny requested changes Sep 10, 2021
View reviewed changes
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.

Wow @gaelicWizard, nice cleanup!

I have left a couple of comments. Also I think that this file is full of random stuff that can be moved to some other place, but lets keep this for another PR

plugins/available/base.plugin.bash Outdated Show resolved Hide resolved
plugins/available/base.plugin.bash Outdated Show resolved Hide resolved
plugins/available/base.plugin.bash Outdated Show resolved Hide resolved
@gaelicWizard
Copy link
Contributor Author

gaelicWizard commented Sep 10, 2021 •
edited
Loading

I'm leaning more and more towards lots of small files with only a few or even just one function, but that's a whole other PR!

@NoahGorny
Copy link
Member

I'm leaning more and more towards lots of small files with only a few or even just one function, but that's a whole other PR!

Yap, that should be a part of #1640

NoahGorny
NoahGorny requested changes Sep 11, 2021
View reviewed changes
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.

I have only one comment 😄

plugins/available/base.plugin.bash Outdated Show resolved Hide resolved
@gaelicWizard gaelicWizard force-pushed the plugin-base branch 6 times, most recently from 73a47e3 to bf9490b Compare September 12, 2021 04:36
NoahGorny
NoahGorny approved these changes Sep 12, 2021
View reviewed changes
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.

Nice job @gaelicWizard !

NoahGorny
NoahGorny requested changes Sep 16, 2021
View reviewed changes
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.

Hi @gaelicWizard, I just noticed- could you add base.plugin.bash to clean_files.txt now that you linted it so nicely?
This will prevent us from mistakenly breaking it again 😃

@gaelicWizard
Copy link
Contributor Author

Hi @gaelicWizard, I just noticed- could you add base.plugin.bash to clean_files.txt now that you linted it so nicely?
This will prevent us from mistakenly breaking it again 😃

One of my goals is to get rid of that file by linting everything! But for the immediate moment, it's kinda hard for me to add things to it since I've got so many open PR's many of which are based on each other and some of which directly change the listing rules, if approved and merged.

@NoahGorny
Copy link
Member

Hi @gaelicWizard, I just noticed- could you add base.plugin.bash to clean_files.txt now that you linted it so nicely?
This will prevent us from mistakenly breaking it again smiley

One of my goals is to get rid of that file by linting everything! But for the immediate moment, it's kinda hard for me to add things to it since I've got so many open PR's many of which are based on each other and some of which directly change the listing rules, if approved and merged.

I dont quite understand, if you add this file here- this makes sure it will always be consistent. If we won't add it, we will need another PR to fix it when we will get rid of clean_files.txt

@gaelicWizard
Copy link
Contributor Author

Rebased on current master and all tests passing. 👍

NoahGorny
NoahGorny approved these changes Sep 21, 2021
View reviewed changes
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.

Nice work 😄

@gaelicWizard
plugins/base: code style improvements
63869e0 
Quote variables, use $@ and $array[@] instead of $*, typeset some integers, remove unneccesasary binary invocation, use shell features when possible, remove `eval`, &c.
@gaelicWizard
plugins/base: conditional function definitions
5cb7522 
Instead of functions failing when required tools aren't installed, just don't define the function.
Alsö, don't redefine del() if it already exists.
@gaelicWizard
plugins/base: rewrite usage()
c3f45b2 
Reimplement disk usage function using Bash syntax and simpler layout, without having to invoke an external binary.
@gaelicWizard
plugins/base: revamp quiet()
41ef92b 
New implementation that is even quieter.
@gaelicWizard
plugins/base: myip()
c975f31
@gaelicWizard
plugins/base: pickfrom()
c8ed9a9
@gaelicWizard
plugins/base: passgen()
ea84771 
Fix `passgen()` to not need `tr`, remove one subshell, and eliminate a useless `echo`.
@gaelicWizard
plugins/base: mkcd()
9ba76b6
@gaelicWizard
plugins/base: mkiso()
49c9c43
@gaelicWizard
plugins/base: remove banish-cookies()
423729e 
Adobe Flash is gone with the wind. Alsö, this would be something someone would do *once* and shouldn't be a function...
@gaelicWizard
plugins/base: lsgrep is SC2010
8d3e81a 
The `lsgrep()` function is *itself* explicitly forbidden by `shellcheck` rule SC2010.

Alsö, s/`$*`/`$@`
@gaelicWizard
plugins/base: mkiso()
96239a2 
Expressly handle unbound parameters.
@gaelicWizard
plugins/base: remove command_exists
280eaf3
@gaelicWizard
plugin/base: lint SC2154 && SC2144
bf5670a 
Newly undisabled `shellcheck` rules
@gaelicWizard
plugin/base: import libs for tests
45bf719
@gaelicWizard
plugin/base: shfmt
e66019d 
Apply `shfmt` using current project settings. My apologies to future `git blame` hunters. ♥
@NoahGorny NoahGorny merged commit c2c76a3 into Bash-it:master Sep 28, 2021
@gaelicWizard gaelicWizard deleted the plugin-base branch September 29, 2021 22:59
ira-bv pushed a commit to seefood/bash-it that referenced this pull request Oct 10, 2021
@seefood
Merge branch 'master' into ira/fix-knife-load
eaf7ff4 
* 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
@seefood
Merge branch 'master' into ira/cleanup
96dbd38 
* 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
@seefood
Merge branch 'master' of https://github.com/bash-it/bash-it into ira/…
d6239cf 
…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
@gaelicWizard @catull
plugin/base: improvements (Bash-it#1930)
86115db 
* plugins/base: code style improvements

Quote variables, use $@ and $array[@] instead of $*, typeset some integers, remove unneccesasary binary invocation, use shell features when possible, remove `eval`, &c.

* plugins/base: conditional function definitions

Instead of functions failing when required tools aren't installed, just don't define the function.
Alsö, don't redefine del() if it already exists.

* plugins/base: rewrite `usage()`

Reimplement disk usage function using Bash syntax and simpler layout, without having to invoke an external binary.

* plugins/base: revamp `quiet()`

New implementation that is even quieter.

* plugins/base: `myip()`

* plugins/base: `pickfrom()`

* plugins/base: `passgen()`

Fix `passgen()` to not need `tr`, remove one subshell, and eliminate a useless `echo`.

* plugins/base: `mkcd()`

* plugins/base: `mkiso()`

* plugins/base: remove `banish-cookies()`

Adobe Flash is gone with the wind. Alsö, this would be something someone would do *once* and shouldn't be a function...

* plugins/base: `lsgrep` is SC2010

The `lsgrep()` function is *itself* explicitly forbidden by `shellcheck` rule SC2010.

Alsö, s/`$*`/`$@`

* plugins/base: `mkiso()`

Expressly handle unbound parameters.

* plugins/base: remove `command_exists`

* plugin/base: lint SC2154 && SC2144

Newly undisabled `shellcheck` rules

* plugin/base: import libs for tests

* plugin/base: `shfmt`

Apply `shfmt` using current project settings. My apologies to future `git blame` hunters. ♥
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NoahGorny NoahGorny NoahGorny approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@gaelicWizard @NoahGorny