Customizability and naming conventions for each completion

[akinomyoga]

In #527, I raised the following question.

For example, in #491, new variables BASH_COMPLETION_MAKE_* are added for the customization of the completion for make, but I don't see other completions use the same approach for the customization. I'm not even sure if any provides customization points. Maybe, a general design for the customization of each completion can be discussed. I also think it is good to normalize the global variable names used by each completion, such as BASH_COMPLETION_{cmdName}_{configName} or _BASHCOMP_{cmdName}_{internalDataName}.

After that I actually found the following reply in #455:

@scop commented on 23 Oct 2020 #455 (comment)

What way project intend to inform users about particular command completion features and available options?

There is no general intent to provide customizability; on the contrary actually. But in case there is some, it's likely it's going to be through env vars, and they will be documented in doc/bash_completion.txt like they currently are.

What way completion can be custmizable (specially prefixed env var, specially named file)?

Likely through env vars in case we implement some customizability. But see above.

Does it mean we don't want to introduce customizability for completions? When we received a pull request with customization options (such as #491), should we reject the PR?

Variable names

I also noticed that there are actually already existing customization through the variables of the form COMP_{CommandName}_{ConfigName} in doc/bash_completion.txt (Actually, there is only one variable, COMP_TAR_INTERNAL_PATHS, for completion for a command). Is this the naming convention for the customization of the completion for each command? Maybe we should explicitly standardize the name as COMP_CMD_{CommandName}_{ConfigName}.

Function names

I also found that the text file doc/styleguide.txt contains useful guidelines. According to this document, the new completion function is recommended to have the form _comp_cmd_{CommandName}.

Function names
--------------

Use the _comp_ prefix for all function names, and _comp_cmd_ for functions
defined in per command completion files and not anywhere else. Prefixing with
an underscore helps keep the functions out of the way for most command name
completions (except obviously ones starting with an underscore or ones that have
nothing typed in yet), and having a consistent prefix helps avoid some clashes
and gives a hint where a function originates from.

It is known that a lot of functions in the tree do not follow this practice.
This is due to backwards compatibility reasons, but all functions introduced
after version 2.11 should follow this name prefix rule.

In fact I can find the following instances:

$ grep '^_comp_cmd_' completions/*
./completions/apt-get:3:_comp_cmd_apt_get_installed_packages()
./completions/apt-mark:3:_comp_cmd_apt_mark()
./completions/avahi-browse:3:_comp_cmd_avahi_browse()
./completions/declare:3:_comp_cmd_declare()
./completions/pdftoppm:3:_comp_cmd_pdftoppm()
./completions/ps:3:_comp_cmd_ps()
./completions/sha256sum:3:_comp_cmd_sha256sum()
./completions/xev:3:_comp_cmd_xev()
./completions/xrandr:3:_comp_cmd_xrandr_outputs()
./completions/xrandr:8:_comp_cmd_xrandr_monitors()
./completions/xrandr:14:_comp_cmd_xrandr_providers()
./completions/xrandr:20:_comp_cmd_xrandr_modes()
./completions/xrandr:30:_comp_cmd_xrandr_all_modes()
./completions/xrandr:39:_comp_cmd_xrandr_help()

However, e.g., in the latest pull request #532, the function names don't follow styleguide.txt. Is the description in styleguide.txt experimental or outdated? Have you changed your mind to make the rule simpler? I actually agree that it is a good idea to prefix _comp_/_comp_cmd_ for bash_completion related functions.

[akinomyoga]

[Completed: moved to #731 (comment)] Internal variables / mutable variables (caching)

In our test suite, we test that the completion functions don't change the environment. But caching is mentioned in #455 (comment). I searched for the corresponding completion and found gradle/gradle-completion: Gradle tab completion for bash and zsh where README.md has the following description:

(Optional) Manual Completion Cache Initialization
Completion cache initialization happens the first time you invoke completion, and usually takes a few seconds, depending on the size of your project. You can manually initialize the cache and avoid interrupting your development mojo by running:

cd path/to/your-project
source ~/.zsh/gradle-completion/_gradle 1>&2 2>/dev/null; __gradle-completion-init

I think it might be also good to provide a rule for internal variables and mutable variables which can be used for caching the completion results. For example, _COMP_CMD_{CommandName}_{InternalDataName} or _COMP_CMD_MUT_{CommandName}_{CacheName} (for mutable data), etc. Maybe we don't have to distinguish two types of internal variables; We use _COMP_CMD_{CommandName}_{DataName} for both types of variables and allow the completions change these variables.

[scop]

Agree, some reserved prefix for these would be good to have. The contents are an internal implmentation detail, not something users need to be concerned with, so an underscore prefix would certainly be in order. I think I'd additionally make them lowercase to kind of more strongly convey that, and better align with the corresponding function names. Gut feeling says +1 to the "not distinguishing" approach.

[akinomyoga]

One more +1 to "not distinguishing" approach. Making the internal variables lowercase is also a good way to make it clear.

[scop]

I think we have a conclusion here and in #791. Could you document the convention in #731? TIA :)

[akinomyoga]

Thanks. Actually, I have implemented *_mut_* in #791 as just one possible option to make it work with the test framework. If you are fine with this option, I'll push commits to #731.

[scop]

Does it mean we don't want to introduce customizability for completions?

In general, I'm a believer in "if everything else fails, then make it customizable/configurable". We should strive to make things Just Work. And a lot of our stuff kind of is customizable by providing a whole implementation of an entire function. Granted, that's a tedious, big gun approach, and more surgical customizability would be better from that respect.

When we received a pull request with customization options (such as #491), should we reject the PR?

"Depends". But we should think hard before accepting or rejecting.

Maybe we should explicitly standardize the name as COMP_CMD_{CommandName}_{ConfigName}.

I'm all for consistency, and aligning with the new _comp/_comp_cmd convention would be good here, i.e. COMP_, COMP_CMD_. Also let's not break existing setups, some kind of silent deprecation/fallback to the old var name would be in order.

I also found that the text file doc/styleguide.txt contains useful guidelines.

Aside, we have some of that also in CONTRIBUTING.md. Should converge to one doc.

However, e.g., in the latest pull request #532, the function names don't follow styleguide.txt.

Nope, that's an oversight. Fixed, thanks. Will also add a PR to fix completions/gssdp-device-sniffer similarly. actually not, as it's aliased to the pre-existing _gssdp_discover.

[akinomyoga]

In general, I'm a believer in "if everything else fails, then make it customizable/configurable". We should strive to make things Just Work. And a lot of our stuff kind of is customizable by providing a whole implementation of an entire function. Granted, that's a tedious, big gun approach, and more surgical customizability would be better from that respect.

OK, I think now I roughly understand the approach of bash-completion. I can understand both ways of thinking. This is not the matter of which one is correct or incorrect.

Also let's not break existing setups, some kind of silent deprecation/fallback to the old var name would be in order.

Yes. I think the only variable that belongs to a specific command is COMP_TAR_INTERNAL_PATHS (as far as I checked last week), so we can just replace it with ${COMP_CMD_TAR_INTERNAL_PATHS:-$COMP_TAR_INTERNAL_PATHS} in the code.

[scop]

I think there are some more (was actually surprised to find some of these):

• COMP_CONFIGURE_HINTS
• COMP_CVS_REMOTE
• COMP_BLUETOOTH_SCAN
• COMP_IWLIST_SCAN
• COMP_SAMBA_SCAN
• BASHCOMP_TAR_OPT_DEBUG

Also, need to make sure not to break nounset mode for the fallback, i.e. ${FOO:-${foo-}} (or just - for the first instead of :- depending on what's semantically correct -- plain - reads cleaner to me if there's a choice).

[akinomyoga]

OK! Thank you for listing.

Also, need to make sure not to break nounset mode for the fallback, i.e. ${FOO:-${foo-}}

Oh, I see. That is also an important point.

[akinomyoga]

[Completed: moved to #731] Current proposal

	bash_completion	completions/*
public configuration variables	BASH_COMPLETION_*	BASH_COMPLETION_CMD_${Command^^}_${Config^^}
non-local internal variables	_comp__*	_comp_cmd_${Command}__${Data}
public/exported functions	_comp_*	_comp_cmd_${Command} (functions for complete -F)
private/internal functions	_comp__*	_comp_cmd_${Command}__${Utility} (utility functions)

• We usually don't provide customizability but try to provide the best behavior by default. It is also good for a uniform user experience in different environments. However, if the user configuration is necessary, we use BASH_COMPLETION_*.
• We use _comp_cmd_${Command}__${Data} for the internal variables used for the completion of the command $Command. The completion can change the variables for caching, etc. Nevertheless, the behavior observable from users should be idempotent, i.e., the completion results should be consistent and unaffected by a past call of the completion.

---

Update 2021-06-15: I have replaced "helper" with "utility".
Update 2021-12-31: The command/config names in user configuration variables should be uppercase, which is implied by the notation ${XXX^^} above.
Update 2022-01-02: Configuration for each command now has the form BASH_COMPLETION_CMD_*. The separators between the command name and the utility names are now double underscores, implying that these are private functions.
Update 2022-02-20: Now internal variables and private functions defined in bash_completion has the form _comp__* (instead of __comp_* or just _comp_*).

[scop]

I concur, let's go with _comp_xfunc_${Command}_${Utility} for this.

I guess we're finally pretty much done here 🎉 Next up would be to add the xfunc stuff to the table, find a home for the naming conclusion in our doc files and document it there.

Related to the public API, I remember we had a discussion how the public API should behave in general for "multiple return values", i.e. not emit things to stdout but rather fill an array based on some criteria. Maybe that was in some issue, it doesn't seem to be in discussions here. Anyway if we start going through our functions and deciding what's public/not and renaming/wrapping the future public API, that's relevant as it'd be great to make the behavior consistent across it while doing that.

[akinomyoga]

I guess we're finally pretty much done here 🎉 Next up would be to add the xfunc stuff to the table,

I have updated the proposal table.

find a home for the naming conclusion in our doc files and document it there.

Maybe #681 (comment) (AsciiDoc to Markdown?) is also related as we now have several documentations README, CONTRIBUTING, and doc.

Related to the public API, I remember we had a discussion how the public API should behave in general for "multiple return values", i.e. not emit things to stdout but rather fill an array based on some criteria.

The discussion was started from here: #537 (reply in thread).

[scop]

find a home for the naming conclusion in our doc files and document it there.

Maybe #681 (comment) (AsciiDoc to Markdown?) is also related as we now have several documentations README, CONTRIBUTING, and doc.

doc/ conversion in #728. Once that's in, I suppose we could document this in styleguide.md as it has some related info. Or all of this to a doc of its own.

[scop]

Public config/control var touching PR is ready for review once the markdown one is in, #678 (comment)

[scop]

Doc PR: #731

[akinomyoga]

[Completed] More flexible positions of double underscores in the names?

Maybe we have already discussed this somewhere, but I don't remember well. While implementing functions, I felt I actually wanted a finer control of scoping in an arbitrary position of double underscores as I have originally described in #539 (reply in thread):

I think we may modify the rule of #537 as "the function names that contain double underscores in any position is used for private functions". In this way, we can introduce more flexible scoping of the private functions, i.e., the scope of _a_b__c_d is _a_b and should not be used from outside _a_b but can be used from any other functions of the name _a_b_*.

• For example, I wanted to define the variables _comp_finalize__depth and _comp_finalize__original_return_trap that belong to the function _comp_finalize in feat: implement BASH_COMPLETION_FINALIZE_HOOKS #739. Or should we define the variables in the name _comp__finalize_depth and _comp__finalize_original_return_trap?
• As another example, I wanted to define the functions _comp_cmd_mutt__get_conffiles__visit that is a helper function of _comp_cmd_mutt__get_conffiles in refactor(bash_completion): refactor { => _comp_}{dequote,quote} #736. We have only mentioned the rule for _comp_cmd_mutt__get_conffiles but maybe we can allow the nested private functions to represent the different levels of privateness: In this case, the scope of _comp_cmd_mutt__get_conffiles__visit is considered _comp_cmd_mutt_get_conffiles{,_*}, and the scope of _comp_cmd_mutt__get_conffiles is _comp_cmd_mutt{_,_*}*.

[scop]

For the first example, the former seems better to me.

For the second:

the scope of _comp_cmd_mutt__get_conffiles__visit is considered _comp_cmd_mutt_get_conffiles{,_*}

Is there a second underscore missing between mutt and get in the latter, i.e. should it be _comp_cmd_mutt__get_conffiles{,_*}? Anyway this kind of leveling/scoping seems fine to me.

[akinomyoga]

Is there a second underscore missing between mutt and get in the latter, i.e. should it be _comp_cmd_mutt__get_conffiles{,_*}?

Ah, yes, right! It was a typo!

[scop]

An attempt to address this is in d92af15

[akinomyoga]

[Completed] Naming of local variables in the function that exports variables of dynamic names?

When we would like to define a function that receives a variable name in the argument and operates on the variable, naively, the local variable names defined in the function should not conflict with the variable name specified in the argument.

• One way to solve this issue is to use Freddy Vulto's trick with _upvars, but the trick is slow when we want to use it repeatedly.
• Another way is to define a naming convention of the local variables of such a function, so that the variable names cannot be specified to the argument of such a function.

In the latter case, what would be the naming convetion of such local variables?

• _comp__* as the same as the other globally-defined internal variables?---This might conflict with the global internal variables.
• _comp_local_*?---This seems a bit long for the local variables.
• __comp_*---I'm currently using this form in feat: implement BASH_COMPLETION_FINALIZE_HOOKS #739, but this doesn't have much backgrounds

[akinomyoga]

I have updated #739 to use _comp_local_*.

[scop]

Actually, it is me who misunderstood this completely, I somehow took it backwards. I do now agree that _comp_local_* is kind of too long for this purpose.

What about just an underscore (_), that's it? I suppose the cases where this would be an annoyance at all would be very rare, as the variable names passed to functions like these tend to be locals, and they rarely begin with an underscore. For the cases where it is, they could always pass a variable name not starting with an underscore and copy over to the one starting with one if they need.

[akinomyoga]

(Sorry, I've replied in the wrong thread, so let me repost this comment here)

Thank you for your reply. As an example of the single-underscore prefix, I have updated #739 in commits f1161a0 and c36cea8.

Also, I have actually already used the double-underscore prefix in _comp_expand_glob. If we are going to use the single-underscore prefix for the private local variables, we also need to update this variable name. The limitation to the operand array name should also be updated here.

Edit: But, maybe the double-underscore prefix is still a valid option because the double-underscores are used for the privateness for the other names of utility functions, config variables, and internal global variables of bash-completion.

[scop]

bd77d1a updates the naming PR/doc with some wording about this. I don't have a strong feeling about a single vs double underscore prefix, but I chose to use a single one in this version of the doc blurb simply because we don't specify anything else that would start with a double underscore. I'm fine with changing the rule to double underscore if you feel that's better.

[akinomyoga]

I agree with the single underscore, so I'll mark this sub-thread as [Completed] by manually including it in the title. [ Unfortunately, GitHub doesn't have a mechanism for "Close" or "Resolve" sub-threads in Discussions. ]

[akinomyoga]

Renaming existing functions: just prefixing _comp vs adjusting each

Naively, we could just prefix _comp to the existing functions, but some of the function names become somewhat redundant.

• For example, _init_completion becomes _comp_init_completion where "comp" are duplicate. I prefer adjusting it like _comp_initialize (this is also symmetric with _comp_finalize suggested in feat: implement BASH_COMPLETION_FINALIZE_HOOKS #739).
• Also, returning results using variables is not special anymore because we are going to change the codebase to use it by default, so the suffix _by_ref that some functions have doesn't make much sense as before.
• Another thing that I thought is that maybe we can separate the namespace of the utilities that generate candidates in COMPREPLY, such as _variables, _filedir, _signals, _ip_addresses, etc. This is good to find available generators from the function names. For example, we may prefix them as _comp_generate{_variables,_filedir,...} or _comp_gen{_variables,_filedir,...}, or the other way around is to group the other functions in _comp_util_*.

However, discussing it for each function could be another blocking factor. If we would do it, I suggest doing it all at once.

Another concern for adjusting each is that external completion writers cannot automatically update their codebase by just prefixing _comp if we adjust the names one by one, but this may not be an issue because we are going to deprecate old functions and define new functions in a new interface of return results, so the external completion writers anyway need to be careful with the new names. If necessary, we might prepare an automatic update script that contains seds to replace the old names and warns about the use of old functions that cannot be automatically updated. At least, I would create a table of renaming after completing the refactoring.

[akinomyoga]

I think I forgot to clarify my current position. Personally, I would like to adjust each name (such as the first and second points above) even though there are small possible drawbacks. I am neutral on the namespacing of candidate generators and the others (the third point above).