Hi Oskari, Sam,
On 3/24/23 06:07, Oskari Pirhonen wrote:
> - Some small stylistic changes such as removing trailing semicolons and
> noisy `shift` calls.
But they are sooo pretty =) That's a little OCD of mine putting
semicolons in shell scripts. Maybe I read too much C, but it feels
good to see punctuation marks at the end of sentences. I prefer to
keep semicolons.
> - Ensure pcregrep exists. It is installed as pcre2grep on my machine, so
> check for both and error out if neither is found. Prefer pcre2grep
> (installed by libpcre2) because libpcre is EOL.
Hmm, I didn't know about pcre2grep(1). I've applied the following:
commit 40049b4d55f6ba2f392aa3b48835b4555ef79ee2 (HEAD -> main, alx/main)
Author: Alejandro Colomar <alx@xxxxxxxxxx>
Date: Fri Mar 24 12:39:33 2023 +0100
bin/: Use pcre2grep(1) instead of pcregrep(1)
PCRE is EOL, and replaced by PCRE2.
Reported-by: Oskari Pirhonen <xxc3ncoredxx@xxxxxxxxx>
Reported-by: Sam James <sam@xxxxxxxxxx>
Signed-off-by: Alejandro Colomar <alx@xxxxxxxxxx>
diff --git a/bin/stdc b/bin/stdc
index 8c725a2..e70e067 100755
--- a/bin/stdc
+++ b/bin/stdc
@@ -14,7 +14,7 @@ err()
grep_proto()
{
- pcregrep -M "(?s)\b$1 *\([[:alnum:]*,._\s\(\)-]*\);$";
+ pcre2grep -M "(?s)\b$1 *\([[:alnum:]*,._\s\(\)-]*\);$";
}
libc_summ_c89()
> - Make libc_summ() standard-agnostics by passing in the filter
> expression and the path to the doc as arguments.
It's really standards agnostic except for C89. ISO C drafts
are pretty much all the same except for the original one, since
I was really ANSI C. That's why C99 and C11 (and C2x if we add
it) can reuse libc_summ(), but C89 is an exception.
> - Make libc_summ() error out if the doc is not found.
The shell already errors out with a quite readable message:
$ ./bin/stdc c99 fgets
./bin/stdc: line 54: /usr/local/share/doc/c/c99/n1256.foo: No such file or directory
$ echo $?
1
Is it not good enough?
> - Give basic usage information on usage errors.
I prefer writing a man page. It keeps the source code simpler.
Please keep this suggestion around, and resend it if you feel it's
necessary after having a man page (1 or 2 weeks maybe).
> - Make the standard version match case insensitive.
Okay.
>
> Signed-off-by: Oskari Pirhonen <xxc3ncoredxx@xxxxxxxxx>
Please send individual patches for each of the logical changes.
> ---
> v1 -> v2:
> - Prefer pcre2grep from libpcre2. Suggested by Sam on IRC.
>
> bin/stdc | 101 +++++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 68 insertions(+), 33 deletions(-)
>
> diff --git a/bin/stdc b/bin/stdc
> index 8c725a2..0d322af 100755
> --- a/bin/stdc
> +++ b/bin/stdc
> @@ -1,65 +1,100 @@
> -#!/bin/bash
> +#!/usr/bin/env bash
>
> -set -Eefuo pipefail;
> +set -Efuo pipefail
Why not set -e? It's not documented in the commit message.
>
> -prefix="/usr/local";
> -datarootdir="$prefix/share";
> -docdir="$datarootdir/doc";
> +prefix="/usr/local"
> +datarootdir="$prefix/share"
> +docdir="$datarootdir/doc"
> +
> +c89_filter='/A.3 LIBRARY SUMMARY/,$p'
> +c89_doc="$docdir/c/c89/c89-draft.txt"
> +c99_filter='/Library summary$/,/Sequence points$/p'
> +c99_doc="$docdir/c/c99/n1256.txt"
> +c11_filter='/Library summary$/,/Sequence points$/p'
This filter is really non_c89_filter :)
> +c11_doc="$docdir/c/c11/n1570.txt"
> +
> +pcregrep="$(type -P pcre2grep)"
> +if [[ -z "$pcregrep" ]]; then
> + pcregrep="$(type -P pcregrep)"
> +fi
Not necessary anymore. I switched to pcre2grep after your
suggestion. I prefer not adding legacy fallbacks. If someone
wants to backport this to an old system, that's not our problem.
>
> err()
> {
> - >&2 echo "$(basename "$0"): error: $*";
> - exit 1;
> + >&2 echo "$*"
> }
>
> -grep_proto()
> +die()
> +{
> + err "$(basename "$0"): error: $*"
> + exit 1
> +}
> +
> +# Args:
> +# 1: usage error
> +usage()
> {
> - pcregrep -M "(?s)\b$1 *\([[:alnum:]*,._\s\(\)-]*\);$";
> + err "$*"
> + err
> + err "usage: $(basename "$0") <version> <function>"
> + err
> + err " version ISO C version. Supported versions:"
> + err " C89, C99, C11"
> + err " (case insensitive)"
> + err " function Function to look for"
> + exit 1
> }
>
> -libc_summ_c89()
> +# Args:
> +# 1: declaration to look for
> +grep_proto()
> {
> - sed -n '/A.3 LIBRARY SUMMARY/,$p' <"$docdir/c/c89/c89-draft.txt";
> + "$pcregrep" -M "(?s)\b$1 *\([[:alnum:]*,._\s\(\)-]*\);\$"
> }
>
> +# Args:
> +# 1: filter expression
> +# 2: path to plaintext standard
> libc_summ()
> {
> - sed -n '/Library summary$/,/Sequence points$/p';
> + if [[ ! -r "$2" ]]; then
> + die "cannot find or read '$2'"
> + fi
> + sed -n -e "$1" "$2"
> }
>
> +if [[ -z "$pcregrep" ]]; then
> + die "pcre2grep or pcregrep required but is not installed"
> +fi
> +
> case $# in
> -0)
> - err "missing ISO C version.";
> - ;;
> -1)
> - err "missing function name.";
> +0|1)
> + usage "missing ISO C version and/or function name."
> ;;
> 2)
> ;;
> *)
> - shift;
> - shift;
> - err "unsupported extra argument(s): $*";
> + shift
> + shift
> + usage "unsupported extra argument(s): ${*@Q}"
> ;;
> -esac;
> +esac
>
> -case "$1" in
> +case "${1@L}" in
I'm not a fan of shell features. I prefer some command like
"$(printf "%s" "$1" | tr [[:upper:]] [[:lower:]])"
which is much more readable (IMO). Does that look good to you?
> c89)
> - shift;
> - libc_summ_c89 \
> - | grep_proto $@;
> + libc_summ "$c89_filter" "$c89_doc" \
> + | grep_proto "$2"
I applied the following:
commit 0e54f2590d9689c9b5cef2369d4664f4096fdd37 (HEAD -> main)
Author: Alejandro Colomar <alx@xxxxxxxxxx>
Date: Fri Mar 24 12:56:37 2023 +0100
bin/: Remove some unnecessary shifts and `$@`s
Reported-by: Oskari Pirhonen <xxc3ncoredxx@xxxxxxxxx>
Signed-off-by: Alejandro Colomar <alx@xxxxxxxxxx>
diff --git a/bin/stdc b/bin/stdc
index e70e067..1067cf8 100755
--- a/bin/stdc
+++ b/bin/stdc
@@ -45,19 +45,16 @@ esac;
case "$1" in
c89)
- shift;
libc_summ_c89 \
- | grep_proto $@;
+ | grep_proto "$2";
;;
c99)
- shift;
libc_summ <"$docdir/c/c99/n1256.txt" \
- | grep_proto $@;
+ | grep_proto "$2";
;;
c11)
- shift;
libc_summ <"$docdir/c/c11/n1570.txt" \
- | grep_proto $@;
+ | grep_proto "$2";
;;
*)
err "$1: unsupported ISO C version.";
> ;;
> c99)
> - shift;
> - libc_summ <"$docdir/c/c99/n1256.txt" \
> - | grep_proto $@;
> + libc_summ "$c99_filter" "$c99_doc" \
> + | grep_proto "$2"
> ;;
> c11)
> - shift;
> - libc_summ <"$docdir/c/c11/n1570.txt" \
> - | grep_proto $@;
> + libc_summ "$c11_filter" "$c11_doc" \
> + | grep_proto "$2"
> ;;
> *)
> - err "$1: unsupported ISO C version.";
> + usage "$1: unsupported ISO C version."
> ;;
> -esac;
> +esac
> +
> +# vim: set noexpandtab:
I'm not sure we want vim comments like this one for every file. Aren't
they too noisy? But maybe it's just me. I use things like the following
for when I contribute regularly to projects that use a different rule for
indentation:
au bufnewfile,bufread ~/src/linux/man-pages/*/man*/* setlocal expandtab
au bufnewfile,bufread ~/src/linux/man-pages/*/man*/* setlocal shiftwidth=4
au bufnewfile,bufread ~/src/linux/man-pages/*/man*/* setlocal softtabstop=4
au bufnewfile,bufread ~/src/linux/man-pages/*/man*/* setlocal tabstop=5
au bufnewfile,bufread ~/src/nginx/* setlocal expandtab
au bufnewfile,bufread ~/src/nginx/* setlocal shiftwidth=4
au bufnewfile,bufread ~/src/nginx/* setlocal softtabstop=4
au bufnewfile,bufread ~/src/nginx/* setlocal tabstop=5
(tabstop=5 is for realizing when something is actually a tab, which for
example happens in Makefiles in NGINX.)
Thanks!
Cheers,
Alex
--
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
