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 <schwa...@openbsd.org> >> 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 <schwa...@openbsd.org> 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 gbranden@. 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)" \