Skip to content

fix: fix uses of sed/awk scripts#1566

Merged
akinomyoga merged 4 commits intoscop:mainfrom
akinomyoga:sed-2
Feb 3, 2026
Merged

fix: fix uses of sed/awk scripts#1566
akinomyoga merged 4 commits intoscop:mainfrom
akinomyoga:sed-2

Conversation

@akinomyoga
Copy link
Collaborator

These are broken regardless of POSIX.

akinomyoga and others added 4 commits February 3, 2026 07:12
`$3 = "="` always evaluates to true regardless of whether $3 is
actually "=".  This results in printing always $2.  In this patch, we
fix the condition to `$3 == "="`.
In the current implementation, we try to filter out the elements of
"${options[@]}" that already appear in "${words[@]}".  The strategy is
to generate

a. "${options[@]}" and
b. the intersect of "${words[@]}" and "${options[@]}"

in a single list and to pick up only unique elements.  However, the
current implementation does not correctly generate "b. the intersect".
It also contains elements of words that contain any of "${options[@]}"
as a substring.  This possibly results in generating some words in the
command line as options.

scop#1564 (comment)

In this patch, we rather implement the filtering using `_compgen -X`.
The filtering attempted in the iptables completion have not taken
effect because `s/.../.../p` (contained in "$chain") has been
performed before the filtering by "s/<chains-to-remove>//".  In this
perspective, the ebtables completion did the correct thing by first
performing the filtering and then performing `s/.../.../p`.

However, even with the above issue fixed, both the iptables and
ebtables completion may wrongly reduce a part of chain names, such as
`DOCKER-FORWARD` to `DOCKER-`.  The filtering should be done by the
exact match.

One solution is to split the "sed" invocation into "sed" and "grep -v"
and perform the filtering of the exact match in the "grep" command as
"grep -vEx 'INPUT|OUTPUT|...'".  In this patch, we instead use the
option `-X '@(INPUT|OUTPUT|...)'` of `_comp_compgen_split`.

scop#1564 (comment)

Co-authored-by: Ville Skyttä <ville.skytta@iki.fi>
Copy link
Owner

@scop scop left a comment

Choose a reason for hiding this comment

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

LGTM on a quick read, thanks!

GH actions is not feeling great at the moment, good to merge after we get a clean/green CI run, https://www.githubstatus.com/incidents/xwn6hjps36ty

@akinomyoga
Copy link
Collaborator Author

GH actions is not feeling great at the moment, good to merge after we get a clean/green CI run, https://www.githubstatus.com/incidents/xwn6hjps36ty

Oh, OK. Thanks for the information.

@akinomyoga akinomyoga merged commit ca7a5c1 into scop:main Feb 3, 2026
9 of 23 checks passed
@akinomyoga akinomyoga deleted the sed-2 branch February 3, 2026 03:50
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