Hi Bjarni,
wow! That's a rant for change. I like it. Most of what you say
makes quite some sense. As with most good rants, there are some
exceptions where you overstate your case.
Oh, btw., nice to meet you, this is the author of that chunk of code
speaking... :-)
These are the points where your evaluation could be contended:
1. While explicit braces for blocks indeed reduce the risk to
introduce future errors by adding requests, adding redundant
braces makes the code longer, more deeply indented, and sometimes
harder to read, which can increase the risk to introduce future
errors. I strongly agree that good coding style is crucial for
maintainability, but braces around one-statement loop bodies
are a typical example of a stylistic detail where both variants
have upsides and downsides.
2. While the stylistic defect i introduced was maybe due to a
lack of professionalism (indeed i'm more used to C and Perl
than to roff, and there "if () {} elsif () {} else {}" is
just fine), i have to defend Werner. By the time he merged
this, he had already asked for retirement but couldn't retire
for want of developers. Still, he tested and merged, which
is more than one could expect in that situation.
3. As Carsten noticed, the code is functionally correct, there
is merely a stylistic defect that groff warns about.
Agreed, i should have tested with -ww but only tested
with the default options.
To be constructive, here is a patch to fix the warning in question.
Sorry for not sending it earlier, it was rotting here and i
nearly forgot:
http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/textproc/groff/patches/patch-tmac_doc-common?rev=1.8&content-type=text/x-cvsweb-markup
Regarding issue 1, i suggest to remain consistent with what the
rest of the file does and not use redundant braces for one-request
loop bodies: Consistency of style also helps maintainability.
Regarding issue 2, i consider
if (cond1)
action(1);
else if (cond2)
action(2);
else
action(3);
about as easy to read as
if (cond1)
action(1);
else {
if (cond2)
action(2);
else
action(3);
}
if not easier, so lets remain consistent with the style used in the
definition of Os right above, which would get exceedingly deep and
arguably unreadable using your proposed style.
Yours,
Ingo
2015-03-07 Ingo Schwarze <schwa...@openbsd.org>
* tmac/doc-common-u: Avoid groff warning "unbalanced .el request".
diff --git a/tmac/doc-common-u b/tmac/doc-common-u
index e63fdb4..2bc54ee 100644
--- a/tmac/doc-common-u
+++ b/tmac/doc-common-u
@@ -802,12 +802,12 @@
. ie \n[.$] \{\
. ie "\$1"$Mdocdate:" \
. ds doc-date-string \$2\~\$3, \$4
-. el .ie (\n[.$] == 3) \
+. el \{ .ie (\n[.$] == 3) \
. ds doc-date-string \$1\~\$2 \$3
. el \{\
. ds doc-date-string "\*[doc-date-\n[mo]]
. as doc-date-string \~\n[dy], \n[year]
-. \}
+. \}\}
. \}
. el \
. ds doc-date-string Epoch
