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

refactor: prefix command to the awk command #1072

Merged
merged 9 commits into from
Nov 28, 2023
Merged

Conversation

akinomyoga
Copy link
Collaborator

@akinomyoga akinomyoga commented Nov 27, 2023

As discussed in #1069 (comment).

In this PR, I change awk to command awk where Solaris awk works (65ea699).

However, another possibility is that we entirely switch to _comp_awk so that we do not have to care about the feature set of Solaris awk. If you like this direction, I can adjust the PR.

There are additional changes:

@akinomyoga akinomyoga added this to In progress in 2.12 release Nov 27, 2023
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.

Left a couple of comments, pre-approved with them addressed.

bash_completion Outdated Show resolved Hide resolved
completions/chronyc Outdated Show resolved Hide resolved
To avoid caring about the limited feature set of Solaris awk compared
to POSIX awk, we always use `_comp_awk` from now on.
In Bash 5.0, a backslash in an unquoted variable expansion $var
induces a pathname expansion, and the backslash is removed.  In
addition, the execution fails when `shopt -s failglob` is set and
there are no filenames matching the pattern.  The raw variable
expansion $var can also be subject to unexpected word splitting with a
custom IFS.  One should store command words in an array or should
`eval` the command stored in a scalar variable.  This PR uses an array
variable.

Note: This Bash behavior was required by the literal interpretation of
the POSIX wording.  After Bash 5.0 implemented this literal
interpretation, a discussion on the POSIX interpretation arose.
Finally the POSIX wording was modified to match the behavior of Bash <
5.0.  Then, the Bash behavior was reverted in Bash 5.1.  For this
reason, this behavior of the pathname expansion is only observed in
Bash 5.0.
The POSIX character classes of the form [[:space:]] or [[:lower:]] are
not supported by mawk < 1.3.3-20090727.  For example, this old version
of mawk is still shipped with Ubuntu 18.04 LTS, which offers extended
support until 2028-04.
\s and \S are GNU extensions.  We need to replace them with
[[:space:]] and [^[:space:]], respectively, in sed.  Some awk
implementations do not support the POSIX character classes of the form
[[:space:]], so \s in awk needs to be replaced with the literal space
and tab, $'[ \t]'.
Flag i for the case-insensitive matching is a GNU extension and thus
should be avoided.

The mock completion contains s/.../.../i while the rpm completion also
contains a similar code but without flag i.  The related sed code in
the rpm completion was introduced in commit 14c9f5b (2009-05-23),
and that in the mock completion was introduced in commit 508a88e
(2009-05-31).  The former did not have flag i while the latter had
flag i from the beginning.

In this patch, we assume the case-insensitive flag was intended for
both cases and update the regular expression so that it matches in a
case-insensitive manner also in the rpm completion.
We have been using the connected form of the `-F` option to the `awk`
command because Solaris awk does not support the separate form of the
option argument `-F <char>`.  Nevertheless, there are some cases of
the separate form in the codespace, though the majority is in the
connected form.  Since we now switch to `_comp_awk` for POSIX awk, we
unify them to the separate form `-F <char>`.
@akinomyoga
Copy link
Collaborator Author

As mentioned in #1072 (comment), there is a new commit 9ec2e9e.

@scop scop merged commit b545d7d into scop:master Nov 28, 2023
7 checks passed
@scop
Copy link
Owner

scop commented Nov 28, 2023

Thanks!

@scop scop moved this from In progress to Done in 2.12 release Nov 28, 2023
@akinomyoga akinomyoga deleted the command-awk branch November 28, 2023 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants