[CC -= Ingo, as requested] Hi Bjarni,
On 4/27/23 03:40, Bjarni Ingi Gislason wrote: > > "groff" is not the right tool for such things, but "grep" is. It could work for an initial implementation. It would only have some false positives for things like defining your own macros at the top of a page, but that's not something I do, so I could live with it. > > The attachment contains a shell script that tests various cases of > defects in man pages. > > It can test for just one or few cases or all of them. > > For example create a file with > > foo. bar > foo. bar > foo. Bar > foo. Bar > > or more examples > > and run > > <name of script> all <file> $ ./semantic_newlines all ./foo.man checking test case nr 7 ./semantic_newlines: line 803: 3*/4: syntax error: operand expected (error token is "/4") ./semantic_newlines: line 1450: mjd: command not found ./semantic_newlines: line 1470: /semantic_newlines.all.diff.new..112419: Permission denied sed: couldn't open file /home/alx/bin/groff.comment.sed: No such file or directory sed: couldn't open file /home/alx/bin/groff.comment.sed: No such file or directory Input file is ./foo.man, case 1 I guess this script has dependencies that I don't know about? $ apt-file find bin/mjd $ > > Later you can use the reported test numbers to just run those tests. > > The script can (still) produce a lot of wrong positive results. > Regarding the script itself (and its documentation), here goes some review: > #!/bin/bash > # Input > # 1) one number, one or more files > # 2) "all", one or more files What's the meaning of the number? I'd appreciate a man-page-like documentation, hopefully available via -h. See what I mean: <https://github.com/nginx/unit/blob/1a485fed6a8353ecc09e6c0f050e44c0a2d30419/tools/setup-unit#L827> > > # In $SEDLIB: "groff.comment.sed", "groff.TH.sed", "groff.hyphen-minus.sed", > # "check_manuals", "strings_gt" > # > # "chk_manuals" uses: "in_out_put.sh", "mandoc", "groff.lint", and > # "roff.singleword.sed" > > # Environmental variable: MANWIDTH (see man(1)) with 'm' unit > # > # Instead of "test-groff" (in the git repository) I don't understand the above (expect for MANWIDTH). > > *) if test "$type" -gt $total; then > echo "$Cmd_name: test number \"$type\" is greater than defined > \($total\)" >&2 > exit 1 > else > diff_file=${TMPDIR}/type.${type}.diff.new.$time > prof_listi="$type" > fi else is unnecessary after exit. Conditional code is harder to read, since the brain needs to remember one more thing, so I suggest making that code unconditional. > > if eval eval \""\${command[${prof}]}"\" "$input" > "$tempfile" ; then > # eval printf "'%s\n\n'" \"Test nr. ${prof}: \${do_what[${prof}]}\" > # if test "$old_filename" != "$file" ; then > if test "$filename_printed" = 'no'; then > printf '%s\n' "Input file is $input, case 1" > filename_printed=yes > fi > if test "$print_test_nr" = 'yes'; then > printf '%s\n' "Test nr. ${prof}:" > fi > eval printf "'\n%s\n\n' \"\${patch_explain[${prof}]}\"" > eval printf "'%s\n\n' \"\${do_what[${prof}]}\"" >> > "$TMPDIR/${file##*/}".summary > case "$1" in > file) : > ;; > test) > if [ -s "$tempfile" ] ; then > : > # printf '####\n\n%s\n\n' "Input file is $file, case 2" > # printf '%s\n' '#### evaluate: case "test"' > fi > ;; > *) echo "$Cmd_name"': function evaluate: case "'"$1"'" is missing' >&2 > exit 1 > ;; > esac > # eval printf "'%s\n\n'" \"\${patch_explain[${prof}]}\"\ > # | tee -a "$diff_file" > cat "$tempfile" > printf '\n#####\n\n' > return 0 > else At this point, I already forgot what the condition was. Cache misses hurt in meatware too. When writing 2-branch conditionals, prefer writing the short branch first. It will help human readers avoid cache misses. Hopefully, it will also help the CPU avoid similar problems. Also, for when both branches are similarly long, putting the exceptional condition in the first branch usually results in faster and more readable code, since the else branch usually loads less instructions[1]. > return 1 > fi On top of that, this branch has a return, so when reversed, the other branch wouldn't even need to be within an else, which reduces the complexity even more. [1]: if (A == exceptional_value) f(); else g(); translates into load A; if not exceptional_value, jump to else; f(); jump to done; else: g(); done: This kind of micro-optimization is probably too much to be considered sane, but I find that it also optimizes for human brains reading the code (at least for mine). Cheers! Alex -- <http://www.alejandro-colomar.es/> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
OpenPGP_signature
Description: OpenPGP digital signature
