I ran ./lint_clean_files.sh and fixed the errors

[matchuek]

Motivation and Context

#1696

How Has This Been Tested?

I don't see necessity for testing these changes.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

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.

[NoahGorny]

Hi @matchuek, I am delighted that you opened up a PR in an effort to clean up Bash-it 😄

I see that the main difference is replacing which with command -v. I did not see any shellcheck warnings on this one, can you send here the error that you received when running lint_clean_files.sh?

[matchuek]

Hello @NoahGorny,

here is the output of ./lint_clean_files.sh:

Trim Trailing Whitespace....................................................Passed
Fix End of Files............................................................Passed
Check for merge conflicts...................................................Passed
Mixed line ending...........................................................Passed
Check for added large files.................................................Passed
Check for conflict markers and core.whitespace errors.......................Passed
Test shell scripts with shellcheck..........................................Failed
- hook id: shellcheck
- exit code: 1

In themes/brainy/brainy.theme.bash line 139:
		|| [ -z "$(which todo.sh)" ] && return
                           ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.

For more information:
  https://www.shellcheck.net/wiki/SC2230 -- which is non-standard. Use builti...

In themes/base.theme.bash line 88:
GIT_EXE=$(which git 2> /dev/null || true)
          ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.


In themes/base.theme.bash line 89:
P4_EXE=$(which p4 2> /dev/null || true)
         ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.


In themes/base.theme.bash line 90:
HG_EXE=$(which hg 2> /dev/null || true)
         ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.


In themes/base.theme.bash line 91:
SVN_EXE=$(which svn 2> /dev/null || true)
          ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.


In themes/base.theme.bash line 403:
	if which rvm &> /dev/null; then
           ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.


In themes/base.theme.bash line 412:
	if which rbenv &> /dev/null; then
           ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.

For more information:
  https://www.shellcheck.net/wiki/SC2230 -- which is non-standard. Use builti...

In completion/available/consul.completion.bash line 6:
	complete -C "$(which consul)" consul
                       ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.

For more information:
  https://www.shellcheck.net/wiki/SC2230 -- which is non-standard. Use builti...

In themes/atomic/atomic.theme.bash line 151:
		|| [ -z "$(which todo.sh)" ] && return
                           ^---^ SC2230: which is non-standard. Use builtin 'command -v' instead.

For more information:
  https://www.shellcheck.net/wiki/SC2230 -- which is non-standard. Use builti...

Check shell style with shfmt................................................Passed
CRLF end-lines remover......................................................Passed
Check .sh files against bash-it requirements................................Passed
Check .bash files against bash-it requirements..............................Passed
Check that clean_files.txt is sorted alphabetically.........................Passed

[gaelicWizard]

I've never once used command -v; I always used type -t...but I don't know the difference! Which one is better?

[NoahGorny]

Hi @matchuek
I left a couple of comments. It seems like you are using an outdated shellcheck and this is why it generated all of those errors. It still great that you opened a PR, because there is still interesting cleanup things to do here!

Don't feel intimidated by the review! If you have any questions, feel free to comment and I will gladly help!

Good luck!

[NoahGorny]

I've never once used command -v; I always used type -t...but I don't know the difference! Which one is better?

I actually prefer type, it seems to be safer all around. We actually have an helper function for this in Bash-it, called _function_exists/_binary_exists etc that use type

[matchuek]

That's OK for me!

[NoahGorny]

That's OK for me!

All right, whenever you have the time feel free to update the PR and I will take another look 😄

Thank you for contributing ❤️

[matchuek]

Hi @NoahGorny ,

update the PR

what do you mean with that? I can't acknowledge your change request, github says:

Pull request authors can’t approve their own pull request.

[NoahGorny]

Hi @NoahGorny ,

update the PR

what do you mean with that? I can't acknowledge your change request, github says:

Pull request authors can’t approve their own pull request.

Hi @matchuek, I did not add the changes to your PR, I only suggested them. You need to commit them yourself, and push to this PR- than I can approve and merge 😄

[matchuek]

I restarted with shellcheck 0.7.2 - everything passed so far!
I will retry on some of the many other files actually not in /clean_files.txt