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

Improve bash_completion coding style #1816

Merged
merged 1 commit into from
May 23, 2018

Conversation

PeterDaveHello
Copy link
Collaborator

Correct the indent and make the code more readable

@@ -3,19 +3,19 @@
# bash completion for Node Version Manager (NVM)

if ! command -v nvm &> /dev/null; then
return
Copy link
Member

Choose a reason for hiding this comment

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

is editorconfig not being enforced on this file already?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope as it can't understand the syntax, it can yell when the indentation is 3 or 5 spaces, but as we set to 2 space, 4 or 6 spaces will be both fine to it.

Copy link
Member

Choose a reason for hiding this comment

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

Why can't it understand the syntax?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's not language based.

bash_completion Outdated
@@ -3,19 +3,19 @@
# bash completion for Node Version Manager (NVM)

if ! command -v nvm &> /dev/null; then
return
return
fi

__nvm_generate_completion()
{
Copy link
Member

Choose a reason for hiding this comment

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

these curly braces should all be in OTBS, not in allman style.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure

bash_completion Outdated
esac
case "${current_word}" in
-*) __nvm_options ;;
*) __nvm_generate_completion "${COMMANDS}" ;;
Copy link
Member

Choose a reason for hiding this comment

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

these really only need one space after )

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cool, I prefer that.

@PeterDaveHello PeterDaveHello force-pushed the bash_completion branch 2 times, most recently from 9690c2d to e7ade17 Compare May 22, 2018 06:17
@PeterDaveHello
Copy link
Collaborator Author

Updated. BTW, CI not triggered?

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

LGTM, but I'm looking into why CI isn't running.

@ljharb ljharb merged commit 62ee7cf into nvm-sh:master May 23, 2018
@PeterDaveHello PeterDaveHello deleted the bash_completion branch May 23, 2018 14:36
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