scop / bash-completion

Programmable completion functions for bash
GNU General Public License v2.0
2.8k stars 376 forks source link

_split_longopt rename is breaking downstream (3rd party) completion scripts #1135

Open jasonkarns opened 3 months ago

jasonkarns commented 3 months ago

Describe the bug

The removal of the function _split_longopt in https://github.com/scop/bash-completion/commit/19a3798a0bf46ad9e39a055c4113eb5e9e9cb98b (released in 2.12.0) is causing completions for the-silver-searcher (ag) to fail

To reproduce

  1. with bash-completion 2.12.0 installed
  2. and silver searcher installed with completions (both the above installed via homebrew)
  3. ag SEARCHQUERY app<TAB>

get:

$ ag SEARCHQUERY app-bash: _split_longopt: command not found

Expected behavior

I'm not the author of ag's completion script so I'm not entirely sure what they expect that function to do. But as the consumer, of course, I expect ag's completion script to work. :D I opened this as a bug against ag's completion as well, but wanted to get guidance here first. Is this function not intended to be public? If not, I'm not certain how to remedy this in ag's completion. If it is then I suspect there needs to be some migration period where the function is aliased to allow updating of downstream completion scripts?

Versions (please complete the following information)

Additional context

I recognize that it's possible (probable even?) that this function should have been considered an internal private function and thus was never intended for completion scripts to use.

However, other completion tooling is also being affected by this change (in addition to silver-searcher):

FWIW, i manually edited ag's completion script to use _comp__split_longopt instead, and all is well. So i think the real question is a matter of "public api management" and "semver". (in scare quotes because of the complexity of package version management with shell utilities like this)

Debug trace

I suspect tracing isn't necessary in this case because the root cause is known? (The function rename.)

akinomyoga commented 3 months ago

The function was removed in d659507f8052151e9435706e5eee29676bc980f4

akinomyoga commented 3 months ago

FWIW, i manually edited ag's completion script to use _comp__split_longopt instead, and all is well.

_comp__split_longopt is not a part of public API and can be removed in the future at any time.

So i think the real question is a matter of "public api management" and "semver".

Any functions and variables whose name contains __ as a substring are not public stable interfaces and considered subject to change.

For the semver, we might have kept the old names.

I actually had a doubt about removing the old names even if they become a part of private APIs. But I haven't tried to keep it because a reasoning was given in the commit message, and also _split_longopt was technically not a public API because we hadn't specified the range of the public API at all until 2.11.

akinomyoga commented 3 months ago

I've checked the ag completion. The ag completion seems to be based on bash-completion 1 API, which is the reason that it directly depends on _split_longopt. The ag completion also depends on the API _get_cword, which is explicitly deprecated in bash-completion 2.0.

I think the ag completion needs to switch to the bash-completion 2 API.

akinomyoga commented 3 months ago

I thought about submitting a PR to the upstream ag, but the mentioned repository https://github.com/ggreer/the_silver_searcher seems stale; there doesn't seem to be any activities for four years. Are there any repository of ag that is actively maintained?

Instead, there seem to be many alternative tools [1]. For example, the latest version of ripgrep (rg 14.0.0) seems to offer a completion setting for Bash [2].

scop commented 3 months ago

JFYI/aside rg 3rd party loader in https://github.com/scop/bash-completion/pull/1148