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: add misc fixes before _parse_{help,usage} changes #950

Merged
merged 10 commits into from
May 3, 2023

Conversation

akinomyoga
Copy link
Collaborator

These are small but non-trivial changes/fixes that I would like to apply before we change the interface of _parse_{help,usage}.

akinomyoga added 9 commits May 2, 2023 04:54
POSIX Basic Regular Expressions (BRE) does not support \?.  Instead,
we can use \{0,1\}.  Also, `grep` and `sed` can be combined.
LC_ALL=C is already specified inside _parse_help, so we do not need to
specify LANG=C at the caller side.
\| is not supported by POSIX Basic Regular Expressions (BRE).  Another
way is to use newline to separate the two matches in `grep` pattern,
but this is not a commonly-used approach.  We instead use `sed` to
remove the two matches separately.
The word-boundary anchor \b is not supported by POSIX Extended Regular
Expressions (ERE).  Also, the previous implementation did not consider
the word-boyndary for the last element.  We here explicitly match the
trailing /[^_[:alnum:]]/ or /$/.

Another issue is that the previous implementation did not work because
of contaminated newlines in the ERE.  We now split the results by
newlines and then join the elements with IFS='|'.
# don't provide main command completions if one is
# already on the command line
[[ $COMP_LINE =~ $(tr ' ' '\b|' <<<"$main_cmd") ]] && return
local IFS='|'
local regex_main_cmd="(${main_cmd[*]})($|[^_[:alnum:]])"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this could end up as an invalid regex if _parse_help lets any strings through to main_cmd that make it so. The previous implementation had the same issue though, so not a regression. Leaving up to you to decide whether to address or merge as is.

Some array utilities would be nice to help with cases like this. I have this ages old unfinished patch for the main bash_completion in my stash, probably broken in the first place and uses namerefs so requires bash >= 4.3, but to illustrate what I was thinking back $then

+_bashcomp_uniq()
+{
+    local -n _bashcomp_uniq_array_=$1
+    local -A tmp
+    local -i i
+    for i in ${!_bashcomp_uniq_array_[*]}; do
+        (( tmp["${_bashcomp_uniq_array_[i]}"]++ > 0 )) &&
+            unset '_bashcomp_uniq_array_[i]'
+    done
+}
+
+_bashcomp_last_index() {
+    local -n _bashcomp_last_index_array_=$1 _bashcomp_last_index_ret_=$2
+    local -i i
+    for i in ${!_bashcomp_last_index_array_[*]}; do :; done
+    _bashcomp_last_index_ret_=$i
+}
+
+_bashcomp_compact() {
+    local -n _bashcomp_compact_array_=$1
+    local i j=0
+
+    for i in ${!_bashcomp_compact_array_[*]}; do
+        if (( i > j )); then
+            _bashcomp_compact_array_[j]="${_bashcomp_compact_array_[i]}"
+            unset "_bashcomp_compact_array_[i]"
+        fi
+        (( j++ ))
+    done
+}
+
+_bashcomp_index_of() {
+    # TODO getopts -> -r gets rightmost (last) index
+    # TODO getopts: -R uses regex instead of glob
+    local -n _bashcomp_index_of_array_=$1
+    local pattern=$2
+    local -n _bashcomp_index_of_ret_=$3
+
+    local -i i
+    for i in ${!_bashcomp_index_of_array_[*]}; do
+        # shellcheck disable=SC2053
+        if [[ ${_bashcomp_index_of_array_[i]} == $pattern ]]; then
+            _bashcomp_index_of_ret_=$i
+            return 0
+        fi
+    done
+
+    _bashcomp_index_of_ret_=-1
+    return 1
+}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this could end up as an invalid regex if _parse_help lets any strings through to main_cmd that make it so. The previous implementation had the same issue though, so not a regression. Leaving up to you to decide whether to address or merge as is.

OK, I think I can try to escape special characters in sed in the previous line.

Some array utilities would be nice to help with cases like this. I have this ages old unfinished patch for the main bash_completion in my stash, probably broken in the first place and uses namerefs so requires bash >= 4.3, but to illustrate what I was thinking back $then

Thank you for the suggestion. In this case, I indeed wanted to have something like remove and join (like _comp_array_remove main_cmd '--config' '--phone' and _comp_join '|' "${main_cmd[@]}"). The suggested uniq, last_index, compact, and index_of do not seem to be specifically able to be used for the present case, but I guess your idea is to give names to array manipulations as independent functions.

This might also be related to the discussion of _comp_xfunc_ARRAY_filter in #739.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a2d756e that escapes the ERE special characters.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Array utilities are moved to #953

@akinomyoga akinomyoga mentioned this pull request May 3, 2023
@akinomyoga akinomyoga merged commit a26a2a9 into scop:master May 3, 2023
@akinomyoga akinomyoga deleted the _parse_help-0 branch May 3, 2023 09:15
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