nmeum / android-tools

Unoffical CMake-based build system for android command line utilities
Apache License 2.0
191 stars 57 forks source link

bash completions are missing check_type function #22

Closed tgurr closed 3 years ago

tgurr commented 3 years ago
$ fastboot <tab>
bash: check_type: Command not found.

Adding function check_type() { type -t "$1"; } as found on https://www.programmersought.com/article/6648134593/#AndroidP_29 appears to work.

anatol commented 3 years ago

The completions coming from upstream. Could you please send a cl to aosp project?

nmeum commented 3 years ago

I personally don't use shell completions. If they are broken we should either remove or fix them. In addition to reporting this breakage upstream. I have no idea how this stuff works, but maybe check_type() is explicitly not defined to allow using the same completion file from both zsh and bash? At least that's what the blog post you linked seems to suggest.

anatol commented 3 years ago

It looks like this check_type function comes from https://android-review.googlesource.com/c/platform/system/core/+/706789 as a way to make the same completion work both at bash and zsh..

And here is an example how android envsetup uses it https://android-review.googlesource.com/c/platform/build/+/706773/11/envsetup.sh

# Zsh needs bashcompinit called to support bash-style completion.
function add_zsh_completion() {
    autoload -U compinit && compinit
    autoload -U bashcompinit && bashcompinit
}

function validate_current_shell() {
    local current_sh="$(ps -o command -p $$)"
    case "$current_sh" in
        *bash*)
            function check_type() { type -t "$1"; }
            ;;
        *zsh*)
            function check_type() { type "$1"; }
            add_zsh_completion ;;
        *)
            echo -e "WARNING: Only bash and zsh are supported.\nUse of other shell would lead to erroneous results."
            ;;
    esac
}

so I wonder if android-tools should add this check_type() function implementation. It should add completion for zsh as well. In this case bash/zsh will have different check_type() implementations.

nmeum commented 3 years ago

I think there are two/three ways to resolve this:

  1. We create dedicated completions files for both zsh and bash at compile-time (by concatenating some shell-specific template with bash.adb and fastboot.adb) and install them to the appropriate locations.
  2. We install adb.bash and fastboot.bash to /usr/share/android-tools/completions-common (or something along those lines), create bash/zsh completions files which provide the appropriate check_type function (also the autoload stuff for zsh) and source the corresponding common files in completions-common.
  3. We remove bash completions again.

I personally don't use completion files and thus I am not highly motivated to work on this at the moment. If no one is else is interested in having working bash completions for adb and fastboot I would suggest removing them.

anatol commented 3 years ago

Completions is a useful thing and the project should provide it. I prefer option 2.

nmeum commented 3 years ago

Did some tests of my own and published a new tarball :)