-
Notifications
You must be signed in to change notification settings - Fork 392
refactor: rename completion functions #1037
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
Conversation
d23c9a8
to
ce7d657
Compare
ce7d657
to
be90a55
Compare
Not sure about this one; it's certainly clearer this way, but if we go with this, it kind of asks renaming of |
Hmm, that is a good point. Do you have any other ideas? The completion functions (which call For consistency, the current way is already not so consistent; while the completion function for a specific command is named as
Currently, I feel options 1 and 2 would be realistic. Considering the fact that we already have a separation in the naming rule of functions in the main file In any case, I should probably update |
52d126f
to
45ceab4
Compare
45ceab4
to
db6cbb3
Compare
db6cbb3
to
bae62b3
Compare
doc/api-and-naming.md
Outdated
| private non-local mutable variables | `_comp__*_mut_*` | `_comp_cmd_${Command}__mut_${Data}` | | ||
| exporter function local variables | `_*` (not `_comp*`) | `_*` (not `_comp*`) | | ||
| public/exported functions | `_comp_*` | `_comp_xfunc_${Command}_${Utility}` (functions for use with `_comp_xfunc`) | | ||
| - completors (for `complete -F`) | `_comp_complete_*` | `_comp_cmd_${Command}` | |
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.
After this, _comp_complete_load
is in an incorrect namespace as it does something else, we should look into renaming it to something else.
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.
In my thinking, _comp_complete_load
is still in the same class of "completion functions" in the sense that _comp_complete_load
is intended to be specified to complete -F
.
Actually, when I introduced the name _comp_complete_load
in d9082d2
(#1032), I already anticipated the changes of this PR and made consistent with this PR. This is also one of the reasons that I split _completion_loader
into two functions with different purposes, _comp_load
(for a utility to purely load a completion) and _comp_complete_load
(for complete -F
) (as in comment d9082d2#r1295226734).
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.
Ah, indeed. No problem then, merging. Thanks!
Co-authored-by: Ville Skyttä <ville.skytta@iki.fi>
b9f27b8
to
7f8d3c0
Compare
squashed typo fix |
For some reason GH is stalling in "Checking for ability to merge automatically…" for me, feel free to merge at your convenience if it works for you. |
Thank you! |
This is a suggestion for the naming of completion functions (i.e., the functions that are supposed to be specified to
complete -F
) defined inbash_completion
: In this PR, the completion functions inbash_completion
have the names of the form_comp_complete_*
.