-
Notifications
You must be signed in to change notification settings - Fork 392
refactor: API/naming of functions and a variable in bash_completion
#1032
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
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
7f59fea
refactor: rename `_comp{ => _compgen}_selinux_users`
akinomyoga dd6c14a
refactor(mussh): clean up option `-a`
akinomyoga 6589b06
refactor: rename `{ => _comp_}_included_ssh_config_files`
akinomyoga 5c924c9
refactor: rename `{ => _comp__init}_install_xspec`
akinomyoga 9efc596
refactor: rename `_{ => comp}_load_completion`
akinomyoga 4f95ab6
refactor: rename variable `{ => _comp}_xspecs`
akinomyoga d9082d2
refactor: rename `_completion_loader => _comp_complete_load`
akinomyoga File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if this is something we want/need to expose as a public function? External completions shouldn't need to use it I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you.
Hmm,
ble.shactually wants to use it._completion_loader(which is also used bygit-completion.bashfrom the Git project) sets_minimalup when none is found, butble.shwants to fall back into its own completion if none is found. Another option is to make_completion_loaderaccept an option that suppresses the fallback to_minimal.I now noticed that I overlooked
_completion_loaderin renaming functions. I think_completion_loadershould be renamed to_comp_loadinstead of__load_completion. I'll fix it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggested this, but
_completion_loaderis used as a completion function forcomplete -F, so I realized it causes a problem when a user inputs a command name starting with-in the command line. So we should prepare two separate functions forcomplete -Fand for a feature to load the completion from a script.Also, as far as I search on GitHub,
__load_completionis directly used by some users (regardless of whether it was originally intended by us).The function
_completion_loadercurrently seems to be intended for two different roles. One is as a function that is specified tocomplete -F. For example, returning status124on successful loading is a part of this use case. The other is as a function that can be called by a user to load a completion for a specific command. Here, I suggest splitting_completion_loaderinto two: 1)_comp_xxx "$cmd"that can be used to load a completion for the specified command, and 2)_comp_yyythat can be specified tocomplete -F.Then, I would also like to have an option that does not define the fallback completion
_minimalwhen the specific completion is not found. I use it for ble.sh. There are also other scripts that use__load_completion(though I'm not sure if they recognize_completion_loader). There are two ways:_minimal--_comp_xxx -- "$cmd"defines the fallback setting_minimalby default, and_comp_xxx -R "$cmd"does not._minimal--_comp_xxx -- "$cmd"does not define_minimal, and_comp_xxx -D "$cmd"defines_minimal.The opt-in behavior seems natural to me as defining
_minimalis an additional behavior on top of just loading a specific completion. Actually, the opt-in behavior is mostly the same as the current implementation of__load_completion, so my current suggestion is to merge the feature_comp_xxx(feature 1 of_completion_loader) into__load_completionas an option-D(defining the default setting_minimal) and to make it a public interface under the name_comp_load.The feature
_comp_yyyis named as a function_comp_complete_loadin accordance with the naming convention suggested in #1037. I can adjust the naming.I've pushed those adjustments in commit 77497bc.