fix(ri): fix several problems and use _comp_split
#1083
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
akinomyoga
force-pushed
the
_comp_split-1
branch
from
7aa168b to
febe8fd
Compare
The current implementation of `_comp_cmd_ri__methods` generates methods separated by newlines as a single element of COMPREPLY, which is suspicous. This code existed since the first implementation of the "ri" completion in commit 309cf93. In this patch, we try to split the result with newlines.
The current implementation passes to `compgen` the option `-P ...` through the quoted word "$prefix" where prefix='-P ...'. However, this is interpreted by compgen as `-P' ...'` where an extra space is suffixed. It was originally specified in an unquoted form `$prefix` so that `-P` and `...` were split by the word splitting. This was broken in commit 9ba5831 [1], which added quotes based on shellcheck SC2086. In this patch, we use a different approach. We make the variable `prefix` only contains the prefix value `...` and explicitly specify the option `-P` at the calling site. [1] \ scop@9ba5831#diff-bbb83b7f6efc5ed7f97e90f27044b376b7a227f954b1ff6ff6ee0afea4bf4a36R32
The current implementation of `_comp_cmd_ri__methods` prefixes $prefix to the generated method names. However, this prefixing also affects the already generated completions in COMPREPLY because it first generates the method names by appending them to COMPRELY and then modifies all the elements of COMPREPLY. In the current usage, it did not produce the problem because prefix is empty when there are existing elements in COMPREPLY, but it's accidentally. Before refactoring the code, we want to resolve this dependency on the subtle condition.
akinomyoga
force-pushed
the
_comp_split-1
branch
from
febe8fd to
c95baa1
Compare
scop
approved these changes
Feb 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm now trying to apply
_comp_splitincompletions/*, but I decided to submit the changes oncompletions/riin a separate PR since this filerihas many fixes. See the commit message of each commit for the details.In particular, commit 8c4510d needs double-checking from others. The existing code looks like
COMPREPLY+=("$(... | sort -u)")where the entire result of the command substitution is quoted, but I suspected that the quoting is unneeded because the result should be separated. However, the quoting seems to have existed from the beginning.