refactor: public config variable naming changes #678

scop · 2022-01-02T10:02:12Z

Implementation of variable naming changes discussed in #537 and #539

scop · 2022-01-02T10:08:11Z

Here's a few initial bits to start the work and to see how it'd look like.

This does not address the publicity part yet; arguably at least the _comp_test_* env state saving variables should be marked as private the way we end up wanting private ones to be marked.

This also does not mark any of the changes implemented yet as breaking even though strictly speaking, the xinetd and tar ones kind of would be. But those were never intended to be user settable under normal operation, and I doubt renaming them would cause any real world problems. Then again if we end up making some "really" breaking changes elsewhere which would warrant major version bump per semver, we could mark these as such as well.

scop · 2022-04-09T09:53:36Z

Should be rebased on top of #728 when that's in.

This was never intended to be a public user settable variable, but exists just for test suite mocking purposes.

This is not intended for users to set in order to control completion behavior, but rather for internal/dev-like testing. Therefore `_comp_cmd_*` seems more appropriate.

For consistency with other internal state variables.

For consistency. Previous names were `BASHCOMP_*`, but that prefix is on its way out.

The old variable name is still used as the (undocumented) fallback. It is now made sure to be at the default in the test suite, too.

The old name still serves as the (undocumented) fallback. While at it, verify both names are at their defaults for tests.

The old name still serves as the (undocumented) fallback.

`COMP_TAR_INTERNAL_PATHS` still serves as the (undocumented) fallback.

`COMP_KNOWN_HOSTS_WITH_HOSTFILE` still serves as the (undocumented) fallback.

`COMP_KNOWN_HOSTS_WITH_AVAHI` still serves as the (undocumented) fallback.

Per naming guideline for non-local internal variables. #539 (comment)

scop · 2022-04-10T11:43:41Z

This is now in sync with the current proposal at #539 (comment). Target branch is docs/markdown, with the intent that this will be merged to master after the markdown branch is in, not to the markdown branch before that.

Therefore draft status for now, although I consider this complete for changes touching public configuration/control variables, internal ones should be done in another PR to keep size at bay.

akinomyoga · 2022-04-11T12:44:24Z

bash_completion

-    # `COMP_KNOWN_HOSTS_WITH_HOSTFILE' is set to an empty value.
-    if [[ -n ${COMP_KNOWN_HOSTS_WITH_HOSTFILE-1} ]]; then
+    # `BASH_COMPLETION_KNOWN_HOSTS_WITH_HOSTFILE' is set to an empty value.
+    if [[ -n ${BASH_COMPLETION_KNOWN_HOSTS_WITH_HOSTFILE-${COMP_KNOWN_HOSTS_WITH_HOSTFILE-1}} ]]; then


Suggested change

if [[ -n ${BASH_COMPLETION_KNOWN_HOSTS_WITH_HOSTFILE-${COMP_KNOWN_HOSTS_WITH_HOSTFILE-1}} ]]; then

if [[ ${BASH_COMPLETION_KNOWN_HOSTS_WITH_HOSTFILE-${COMP_KNOWN_HOSTS_WITH_HOSTFILE-1}} ]]; then

I think this is more consistent with other cases.

akinomyoga · 2022-04-11T12:47:15Z

completions/cvs

                # passwordless access to the remote repository
-                if [[ -n ${COMP_CVS_REMOTE:-} ]]; then
+                if [[ -n ${BASH_COMPLETION_CMD_CVS_REMOTE-${COMP_CVS_REMOTE-}} ]]; then


Suggested change

if [[ -n ${BASH_COMPLETION_CMD_CVS_REMOTE-${COMP_CVS_REMOTE-}} ]]; then

if [[ ${BASH_COMPLETION_CMD_CVS_REMOTE-${COMP_CVS_REMOTE-}} ]]; then

akinomyoga · 2022-04-11T12:48:32Z

bash_completion

@@ -634,7 +635,8 @@ _filedir()
        IFS=$'\n'

        # Try without filter if it failed to produce anything and configured to
-        [[ -n ${COMP_FILEDIR_FALLBACK-} && -n $arg && ${#toks[@]} -lt 1 ]] && {
+        [[ ${BASH_COMPLETION_FILEDIR_FALLBACK-${COMP_FILEDIR_FALLBACK-}} &&
+            -n $arg && ${#toks[@]} -lt 1 ]] && {


Suggested change

-n $arg && ${#toks[@]} -lt 1 ]] && {

$arg && ${#toks[@]} -lt 1 ]] && {

akinomyoga · 2022-04-11T12:51:21Z

bash_completion

@@ -623,7 +623,8 @@ _filedir()
        # the fallback condition with the "plus" dirs.
        local opts=(-f -X "$xspec")
        [[ $xspec ]] && plusdirs=(-o plusdirs)
-        [[ ${COMP_FILEDIR_FALLBACK-} || -z ${plusdirs-} ]] ||
+        [[ ${BASH_COMPLETION_FILEDIR_FALLBACK-${COMP_FILEDIR_FALLBACK-}} ||
+            -z ${plusdirs-} ]] ||


Suggested change

-z ${plusdirs-} ]] ||

! ${plusdirs-} ]] ||

It seems there are more cases [[ ! $word ]] (5 cases in bash_completion) than [[ -z $word ]] (2 cases). Another instance of -z is in line bash_completion:1990 (_cd).

scop · 2022-04-12T15:45:04Z

Thanks for having a look and good point on the suggestions. However, we have a lot of similar cases in completions/*, so it seems it'd be good to address them in one go and add a style guide entry for it, too. I opened #740 for that and chose to leave those changes out from this one.

akinomyoga · 2022-04-12T20:56:14Z

Thanks!

scop marked this pull request as draft January 2, 2022 10:02

scop mentioned this pull request Jan 4, 2022

Remove redundante variable docs from README #681

Merged

scop force-pushed the refactor/variable-renames branch from bd67d58 to 2bf536d Compare February 20, 2022 07:38

scop mentioned this pull request Apr 9, 2022

docs: convert to markdown, remove some no longer relevant sections #728

Merged

scop added 12 commits April 10, 2022 14:06

refactor(xinetd): rename BASHCOMP_XINETDDIR to _comp_test_xinetd_dir

072e969

This was never intended to be a public user settable variable, but exists just for test suite mocking purposes.

refactor(tar): rename BASHCOMP_TAR_OPT_DEBUG to _comp_cmd_tar_debug

e1039c2

This is not intended for users to set in order to control completion behavior, but rather for internal/dev-like testing. Therefore `_comp_cmd_*` seems more appropriate.

refactor(test): use _comp_test_ prefix for saved env variables

2850363

For consistency with other internal state variables.

refactor(test): use BASH_COMPLETION_TEST_ prefix for control variables

74d5ffc

For consistency. Previous names were `BASHCOMP_*`, but that prefix is on its way out.

refactor: rename COMP_IWLIST_SCAN BASH_COMPLETION_CMD_IWCONFIG_SCAN

f404a7b

The old variable name is still used as the (undocumented) fallback. It is now made sure to be at the default in the test suite, too.

refactor: COMP_FILEDIR_FALLBACK to BASH_COMPLETION_FILEDIR_FALLBACK

5f941d4

The old name still serves as the (undocumented) fallback. While at it, verify both names are at their defaults for tests.

refactor: COMP_CONFIGURE_HINTS to `BASH_COMPLETION_CMD_CONFIGURE_HINTS

ffc55ab

The old name still serves as the (undocumented) fallback.

refactor: COMP_CVS_REMOTE to BASH_COMPLETION_CMD_CVS_REMOTE

6389ead

The old name still serves as the (undocumented) fallback.

refactor: BASH_COMPLETION_CMD_TAR_INTERNAL_PATHS

2b36939

`COMP_TAR_INTERNAL_PATHS` still serves as the (undocumented) fallback.

refactor: BASH_COMPLETION_KNOWN_HOSTS_WITH_HOSTFILE

1fbe22e

`COMP_KNOWN_HOSTS_WITH_HOSTFILE` still serves as the (undocumented) fallback.

refactor: BASH_COMPLETION_KNOWN_HOSTS_WITH_AVAHI

e67ac07

`COMP_KNOWN_HOSTS_WITH_AVAHI` still serves as the (undocumented) fallback.

docs(environment): group variables to general, then command specific

2070c2a

scop force-pushed the refactor/variable-renames branch from fdf8f55 to 2070c2a Compare April 10, 2022 11:16

scop changed the base branch from master to docs/markdown April 10, 2022 11:18

scop changed the title ~~Namespacing/publicity variable naming changes~~ refactor: public config variable naming changes Apr 10, 2022

scop added 3 commits April 10, 2022 14:41

refactor(xinetd): _comp_test_xinetd_dir to _comp__test_xinetd_dir

a491111

Per naming guideline for non-local internal variables. #539 (comment)

refactor(tar): _comp_cmd_tar_debug to _comp_cmd_tar__debug

0c7dd3a

Per naming guideline for non-local internal variables. #539 (comment)

refactor(test): _comp_test_* to _comp__test_*

a2a1459

Per naming guideline for non-local internal variables. #539 (comment)

Base automatically changed from docs/markdown to master April 10, 2022 16:54

scop marked this pull request as ready for review April 10, 2022 16:54

akinomyoga approved these changes Apr 11, 2022

View reviewed changes

scop mentioned this pull request Apr 12, 2022

style: prefer empty/! over -n/-z #740

Merged

scop merged commit 8181dab into master Apr 12, 2022

scop deleted the refactor/variable-renames branch April 12, 2022 15:43

akinomyoga mentioned this pull request Apr 17, 2022

Disable /etc/bash_completion.d/redefine_filedir #667

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: public config variable naming changes #678

refactor: public config variable naming changes #678

scop commented Jan 2, 2022

scop commented Jan 2, 2022

scop commented Apr 9, 2022

scop commented Apr 10, 2022

akinomyoga Apr 11, 2022

akinomyoga Apr 11, 2022

akinomyoga Apr 11, 2022

akinomyoga Apr 11, 2022

scop commented Apr 12, 2022

akinomyoga commented Apr 12, 2022

	if [[ -n ${BASH_COMPLETION_KNOWN_HOSTS_WITH_HOSTFILE-${COMP_KNOWN_HOSTS_WITH_HOSTFILE-1}} ]]; then
	if [[ ${BASH_COMPLETION_KNOWN_HOSTS_WITH_HOSTFILE-${COMP_KNOWN_HOSTS_WITH_HOSTFILE-1}} ]]; then

	if [[ -n ${BASH_COMPLETION_CMD_CVS_REMOTE-${COMP_CVS_REMOTE-}} ]]; then
	if [[ ${BASH_COMPLETION_CMD_CVS_REMOTE-${COMP_CVS_REMOTE-}} ]]; then

	-n $arg && ${#toks[@]} -lt 1 ]] && {
	$arg && ${#toks[@]} -lt 1 ]] && {

refactor: public config variable naming changes #678

refactor: public config variable naming changes #678

Conversation

scop commented Jan 2, 2022

scop commented Jan 2, 2022

scop commented Apr 9, 2022

scop commented Apr 10, 2022

akinomyoga Apr 11, 2022

Choose a reason for hiding this comment

akinomyoga Apr 11, 2022

Choose a reason for hiding this comment

akinomyoga Apr 11, 2022

Choose a reason for hiding this comment

akinomyoga Apr 11, 2022

Choose a reason for hiding this comment

scop commented Apr 12, 2022

akinomyoga commented Apr 12, 2022