Skip to content
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

fix: improve the completion settings suggested in aqua completion --help #3297

Merged
merged 3 commits into from
Nov 27, 2024

Conversation

akinomyoga
Copy link
Contributor

@akinomyoga akinomyoga commented Nov 27, 2024

Check List

Three fixes are included in this PR.

(1) .bashrc and .zshrc 4e21f1d

The current help text suggests adding the completion settings in .bash_profile and .zprofile, but .bash_profile and .zprofile are loaded only in login shell sessions. They are not loaded in (non-login) interactive sessions started inside the login shells, and thus the completion would be disabled in non-login interactive sessions.

Actual

$ cat .bash_profile
if command -v aqua &> /dev/null; then
  source <(aqua completion bash)
fi
$ bash
$ complete -p aqua
<nothing is output>

Expected

$ cat .bashrc
if command -v aqua &> /dev/null; then
  source <(aqua completion bash)
fi
$ bash
$ complete -p aqua
complete -o bashdefault -o default -o nospace -F _cli_bash_autocomplete aqua

In general, .bash_profile and .zprofile should only contain the settings that are automatically inherited by the child processes or the global user settings in the operating system. The interactive settings should be put in .bashrc and .zshrc.

In this commit, we suggest .bashrc and .zshrc instead of .bash_profile and .zprofile, respectively.

(2) Fix for Bash 3.* 8cbabba

The currently suggested setting does not work for Bash 3.* because the source builtin in Bash 3.* does not support reading from a pipe and source <(command) fails silently. Instead, eval "$(command)" works in all versions of Bash.

Actual

$ bash-3.2
$ source <(aqua completion bash)
$ complete -p aqua
<nothing is output>

Expected

$ bash-3.2
$ eval "$(aqua completion bash)"
$ complete -p aqua
complete -o bashdefault -o default -o nospace -F _cli_bash_autocomplete aqua

(3) Style improvement 64ad3b4

This is an appearance improvement for the examples of different shells, and the change is just as you see in the diff.

The current help text suggests adding the completion settings in
.bash_profile and .zprofile, but .bash_profile and .zprofile are
loaded only in login shell sessions. They are not loaded in
(non-login) interactive sessions started inside the login shells, and
thus the completion would be disabled in non-login interactive
sessions.

In general, .bash_profile and .zprofile should only contain the
settings that are automatically inherited by the child processes or
the global user settings in the operating system.  The interactive
settings should be put in .bashrc and .zshrc.

In this patch, we suggest .bashrc and .zshrc instead of .bash_profile
and .zprofile, respectively.
The source builtin in Bash 3.* does not support reading from a pipe,
so `source <(command)` fails silently.  `eval "$(command)"` works in
all versions of Bash.
@akinomyoga akinomyoga changed the title Improve the completion settings suggested in aqua completion --help fix: improve the completion settings suggested in aqua completion --help Nov 27, 2024
@suzuki-shunsuke
Copy link
Member

Thank you for your contribution!

@suzuki-shunsuke suzuki-shunsuke added this to the v2.38.1 milestone Nov 27, 2024
@suzuki-shunsuke suzuki-shunsuke merged commit 46013b8 into aquaproj:main Nov 27, 2024
15 checks passed
@akinomyoga akinomyoga deleted the completion-bash-example branch November 27, 2024 22:52
akinomyoga added a commit to akinomyoga/aquaproj.github.io that referenced this pull request Nov 27, 2024
This patch reflects the changes in the main repository aquaprpj/aqua
applied by PR aquaproj/aqua#3297 [1] to the online documentation.

[1] aquaproj/aqua#3297
akinomyoga added a commit to akinomyoga/aquaproj.github.io that referenced this pull request Nov 27, 2024
This patch reflects the changes in the main repository aquaprpj/aqua
applied by PR aquaproj/aqua#3297 [1] to the online documentation.

[1] aquaproj/aqua#3297
@suzuki-shunsuke
Copy link
Member

suzuki-shunsuke added a commit to aquaproj/aquaproj.github.io that referenced this pull request Nov 27, 2024
* fix: reflect changes in aquaproj/aqua for shell setup example

This patch reflects the changes in the main repository aquaprpj/aqua
applied by PR aquaproj/aqua#3297 [1] to the online documentation.

[1] aquaproj/aqua#3297

* fix: `cmdx usage`

---------

Co-authored-by: Shunsuke Suzuki <suzuki.shunsuke.1989@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants