Hi,
On Fri, Mar 24, 2023 at 13:21:58 +0100, Alejandro Colomar wrote:
> 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.
>
Style is obviously your call here.
> > - 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:
>
... snip ...
>
> 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.
>
That's good to know.
> > - 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?
>
I suppose that is fine for me. I may be too used to having to performing
my own checks for files that I ignored Bash being able to do the same.
> > - 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).
>
Heh. Perhaps you're biased towards man pages ;)
I'll keep it around (rebasing as needed for current and possibly future
edits).
> > - 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.
>
Whoops. Will do in the future.
> > ---
> > 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.
>
I forgot to mention that in the message, but it's no longer relevant
since you switched it to just pcre2grep. With `set -e` it was aborting
the script when `type -P` failed.
... snip ...
> >
> > -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?
>
Fine by me.
> > c89)
> > - shift;
> > - libc_summ_c89 \
> > - | grep_proto $@;
> > + libc_summ "$c89_filter" "$c89_doc" \
> > + | grep_proto "$2"
... snip ...
> > +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.)
>
This is an interesting idea that I've never considered before.
Thanks for at least accepting some of the suggestions.
- Oskari
Attachment:
signature.asc
Description: PGP signature
