-
Notifications
You must be signed in to change notification settings - Fork 380
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
refactor: refactor _comp_{get_first_word,count_args}
#1036
Merged
Merged
Conversation
This file contains 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
changed the title
[Waiting for #1033 and #1034] refactor: refactor
[Waiting for #1033 and #1034] refactor: refactor Aug 13, 2023
_get_first_word
and _count_args
_comp_{get_first_word,count_args}
akinomyoga
force-pushed
the
refactor-api-5
branch
from
August 13, 2023 01:20
177470b
to
94d63ee
Compare
akinomyoga
force-pushed
the
refactor-api-5
branch
from
August 26, 2023 12:06
94d63ee
to
2f4fd2f
Compare
akinomyoga
changed the title
[Waiting for #1033 and #1034] refactor: refactor
[Waiting for #1033] refactor: refactor Aug 26, 2023
_comp_{get_first_word,count_args}
_comp_{get_first_word,count_args}
akinomyoga
force-pushed
the
refactor-api-5
branch
2 times, most recently
from
September 2, 2023 20:07
7f818a2
to
83ba88f
Compare
In the current implementation, to count the number of arguments, we reassemble `cword` and `words` by specified exclude chars (i.e., characters that should not be considered word breaks even though they are in COMP_WORDBREAKS). The reassembly by the top-level `_comp_initialize` always specify `<>&` as exclude chars, but the current implementaion of `_comp_count_args` do not specify them. This patch specifies `<>&` for `_comp_count_args` in accordance with `_comp_initialize`.
In most of the contexts calling `_comp_count_args` in the current codebase, `cword` and `words` are already initialized with the same set of exclude chars (-n chars) by _comp_initialize, so there seems to be no need to again reassemble `cword` and `words`. This patch removes the redundant initialization of `cword` and `words`. * When `_comp_initialize` is called without `-s` or `-n chars` and `_comp_count_args` is called with an empty $1, they are compatible so `cword` and `words` outside can be directly used. * `7z` and `nslookup` specify the same set of exclude chars as `_comp_initialize`, so existing `cword` and `words` are compatible. Nevertheless, we need to reinitialize `cword` and `words` when the specified exclude chars are different from those specified to _comp_initialize. * The `ssh` completion calls `_comp_count_args` with excluding `=` while it calls `_comp_initialize` with `-n :`. * The `chown` completion calls `_comp_count_args` with excluding `:` while it calls `_comp_initialize` with `-sn :`. * The `nc` completions calls `_comp_count_args` without exclude chars while it calls `_comp_initialize` with `-n :`. * The completions for `chmod`, `cryptsetup`, `hcitool`, `mkinitrd`, `patch`, `quota`, and `zopflipng` does not specify exclude chars to `_comp_count_args` while they specify `-s` to `_comp_initialize`. It is not clear whether these discrepancies are intended ones are not, but this patch tries to keep the current behavior by explicitly specifying the original exclude chars.
These completions induce reassembling `cword` and `words` inside `_comp_count_args`, but they are likely to be intended to be the same as outside `cword` and `words`. We just skip the reassembling by dropping `-n ""` or `-n :`.
akinomyoga
force-pushed
the
refactor-api-5
branch
from
September 2, 2023 21:07
83ba88f
to
3127703
Compare
akinomyoga
changed the title
[Waiting for #1033] refactor: refactor
refactor: refactor Sep 2, 2023
_comp_{get_first_word,count_args}
_comp_{get_first_word,count_args}
scop
approved these changes
Sep 12, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍
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.
Waiting for #1033 and #1034
There are many handwritten for-loops in the codebase that can be replaced by these functions with small adjustments. This is the first step:
<>&
and skippingcword
/words
re-assembling that seems redundant). These need discussions.