Skip to content
This repository has been archived by the owner on Nov 22, 2022. It is now read-only.

refactor(commands/completion): always default to bash #507

Merged
merged 1 commit into from
Jan 4, 2021
Merged

refactor(commands/completion): always default to bash #507

merged 1 commit into from
Jan 4, 2021

Conversation

maxice8
Copy link
Collaborator

@maxice8 maxice8 commented Jan 4, 2021

No description provided.

@codecov
Copy link

codecov bot commented Jan 4, 2021

Codecov Report

Merging #507 (da5218d) into trunk (41720e8) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##            trunk     #507   +/-   ##
=======================================
  Coverage   57.80%   57.81%           
=======================================
  Files          83       83           
  Lines        5565     5561    -4     
=======================================
- Hits         3217     3215    -2     
+ Misses       2017     2016    -1     
+ Partials      331      330    -1     
Impacted Files Coverage Δ
commands/completion/completion.go 100.00% <100.00%> (+6.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41720e8...da5218d. Read the comment docs.

Copy link
Owner

@profclems profclems left a comment

Choose a reason for hiding this comment

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

Thanks for taking this on!

It is right we default to bash if no shell is specified. We should add the to the --shell flag usage info that default is bash. Since cobra already prints the default value if set, we should set the default value to bash:

	completionCmd.Flags().StringVarP(&shellType, "shell", "s", "bash", "Shell type: {bash|zsh|fish|powershell}")

@pull-request-size pull-request-size bot added size/S and removed size/XS labels Jan 4, 2021
@maxice8
Copy link
Collaborator Author

maxice8 commented Jan 4, 2021

Thanks for looking at it, I set bash as default outright and removed all the code that deals with the value being empty

@maxice8 maxice8 merged commit c2c23f5 into profclems:trunk Jan 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants