Re: [groff] Recent contrib/hdtbl changes may have broken it

[Ingo Schwarze]

Hi Branden,

Bjarni Ingi Gislason wrote on Sun, Nov 04, 2018 at 02:16:24PM +0000:
> On Sun, Nov 04, 2018 at 01:01:40AM -0400, G. Branden Robinson wrote:
>> On Fri, Oct 26, 2018 at 04:28:02PM -0400, G. Branden Robinson wrote:

>>>   GROFF    contrib/hdtbl/examples/color_nested_tables.ps
>>> ../contrib/hdtbl/examples/color_nested_tables.roff:40:
>>> The 1st width value () is too small. It should be greater than 12000.
>>> ../contrib/hdtbl/examples/color_nested_tables.roff:52:
>>> The 1st width value () is too small. It should be greater than 11999.

>> These warnings appear to be caused by:
>> 
>> commit 305701e856baa0b23066279160eaeb38bd27b9e4
>> Author: Ingo Schwarze <address@hidden>
>> Date:   Thu Aug 9 22:50:47 2018 +0200
>> 
>>     contrib/hdtbl: do forgotten renamings .pv -> .t*pv
>> 
>>     This was forgotten in commit 6fb4a0ab on Feb 8, 2010.
>>     Fixing it improves the formatting of all hdtbl examples.
>>     Reported by Bjarni Ingi Gislason in http://savannah.gnu.org/bugs/?54470.
>> 
>>     While here, also fix a typo in short_reference.roff.

That commit did not cause the problem, but merely exposed it by fixing
a bug that was hiding it.

>> If I revert it, these warnings go away.

It's better to actually fix the problem than to restore the bug
that used to hide it.

>   Removing the line
> 
> .t*pv 1.2 1.2 "" X
> 
>   in "common.roff" avoids the warnings.
> 
>   It also fixes the output of "color_nested_tables.ps" and "font_n.ps"
> 
>   If this line is needed in some file, it should be added there, for
> example after including the "common.roff" file.

Bjarni is completely right here.

The purpose of the macro .t*pv is setting font sizes, line spacing,
and the like, relative to some defaults, see the comment in
hdmisc.tmac.  The file examples/common.roff sets some defaults, and
changing defaults by default makes little sense because then the
defaults are no longer defaults - if you follow me ;-).

Also, those examples that do want the settings changed
by '.t*pv 1.2 1.2 "" X' already contain that line, exactly as
Bjani rightfully says they should.

Before the August cleanups and bugfixes, fonts_n worked because
the line '.pv 1.2 1.2 "" X' in common.roff was broken and hence
ineffective; it was also ineffective for all the other examples
for about a decade.

So Bjarni is right that the best fix is to just remove the line
which was misplaced in common.roff in the first place, which had
no effect because it was broken, and which apparently wasn't
missed by anyone.

>   col_rowspan_colors.roff: The "random-seed ..." line produces only a
> one colored area in the middle instead of lines and columns of
> different colors.
> 
>   That line must be outside the macro "color#"; otherwise the macro
> always produces the same number, instead of random ones.

That's right, too.


Regarding the .pso calls, a partial revert, restoring the -U,
seems required.  Sorry for missing that .pso was still in use
when i committed.

I would no doubt like to get rid of the -U, but that requires either
fixing the examples preserving the functionality, or reaching a
consensus that we no longer want these font listings, but that
wasn't even discussed yet.  So restoring -U until we have either
of the above seems the way to go.

So here is a complete patch to fix all issues mentioned by Branden
and Bjarni in this thread, unless i missed any.

I don't think a ChangeLog entry is needed because these a merely
simple fixes of recent regressions in a very unimportant area.

OK to push?

Yours,
  Ingo


commit 5d703b35b7fe10d78e3a5e608912e0ea50798038
Author: Ingo Schwarze <address@hidden>
Date:   Mon Nov 5 11:14:38 2018 +0100

    fix a few recent regressions in the hdtbl examples
    
    1. hdtbl.am: restore the -U flag to the groff(1) command because
    the two fonts examples still use .pso; sorry for breaking this
    in commit c675115cd on Aug 4 20:34:15 2018.  More work is needed
    before this can be dropped.  Issue observed by address@hidden
    
    2. examples/common.roff: remove the line '.t*pv 1.2 1.2 "" X' which
    sets a point size and line spacing that some of the examples don't
    work with, most notably fonts_n and also color_nested_tables.  Those
    examples that do want these changed settings already contain the
    line themselves.  This problem used to be hidden by a bug and exposed
    by the bugfix commit 305701e8 on Aug 9 22:50:47 2018.  Issue observed
    by gbranden@, fix suggested by Bjarni Ingi Gislason.
    
    3. examples/col_rowspan_colors.roff: When you seed a random number
    generator, you have to do that once at the beginning, not inside a
    tight loop.  In this case, seeding over and over again resulted in
    the famous Elbonian random number sequence: 9, 9, 9, 9, 9, 9, 9... -
    each random number was the same as the previous one, resulting in
    a uniformly coloured picture that was supposed to be a patchwork
    of different colours.  Bug exposed by the cleanup commit a519a612
    on Aug 9 23:34:10 2018.  Issue noticed by Bjarni Ingi Gislason.

diff --git a/contrib/hdtbl/examples/col_rowspan_colors.roff 
b/contrib/hdtbl/examples/col_rowspan_colors.roff
index 5255678f..9a427660 100644
--- a/contrib/hdtbl/examples/col_rowspan_colors.roff
+++ b/contrib/hdtbl/examples/col_rowspan_colors.roff
@@ -25,10 +25,10 @@ along with this program.  If not, see 
<http://www.gnu.org/licenses/>.
 .
 .so \*[sopath]examples/common.roff
 .
-.de color#
-.nr # +1
 .\" Seed the random number generator for reproducible builds.
 .random-seed 131545532 19201711
+.de color#
+.nr # +1
 .random#
 .defcolor c\\n# rgb \\*[#random]
 ..
diff --git a/contrib/hdtbl/examples/common.roff 
b/contrib/hdtbl/examples/common.roff
index f7d1f6e9..fa45674e 100644
--- a/contrib/hdtbl/examples/common.roff
+++ b/contrib/hdtbl/examples/common.roff
@@ -236,7 +236,6 @@ along with this program.  If not, see 
<http://www.gnu.org/licenses/>.
 .nr v \n[.v]
 .nr p \n[.p]
 .nr o \n[.o]
-.t*pv 1.2 1.2 "" X
 .nr l 6.6i                             \"      set text width
 .ll \n[t*l]u
 .nr o 2c                               \"      set offset
diff --git a/contrib/hdtbl/hdtbl.am b/contrib/hdtbl/hdtbl.am
index 1b80a9a9..bdc657b1 100644
--- a/contrib/hdtbl/hdtbl.am
+++ b/contrib/hdtbl/hdtbl.am
@@ -26,7 +26,7 @@ man7_MANS += contrib/hdtbl/groff_hdtbl.7
 
 # Groff command used to generate .ps files
 HDTBL_TFLAG = -M$(hdtbl_srcdir) -M$(hdtbl_builddir)
-HDTLB_PFLAG=-t -p -e
+HDTLB_PFLAG=-t -p -e -U
 HDTBLGROFF = \
   GROFF_COMMAND_PREFIX= \
   GROFF_BIN_PATH="$(GROFF_BIN_PATH)" \