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

[G. Branden Robinson]

At 2018-11-05T11:46:30+0100, Ingo Schwarze wrote:
> Hi Branden,

Hi Ingo!

Sorry for the delayed reply.  There were two problems:

1.  GMail threw your mail into the spam folder because you "don't meet
the sender guidelines".  Not sure why that affected you and not anyone
else who posts to the groff list... :-/

2. I cut-and-pasted your patch to a text file put "git am" refused to
recognize the patch format.  Eventually I got tired of fighting with it
and applied your patch manually.

The good news is that, with it, the build is quiescent and all the
hdtbl examples PS documents look great.  Thanks!

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

Okay.  My understanding of hdtbl internals is poor.

> >> 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.

I have to say that's a ghastly failure mode.  If "t*pv" has some
mnemonic value I'm sure I don't know what it is, and I guess we have
discovered the opposite of idempotency, especially for something that's
just "setting some defaults"--make the call once and you're okay, make
it twice and bizarre things happen.

This puts the same look on my face as:

$ PATH=/usr/local/bin:/usr/bin:/bin # everything's fine
$ PATH=/usr/local/bin:/usr/bin:/bin # let's do it again!
sh: Exec format error

> >   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.

This part makes perfect sense to me, by contrast.

> 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.

Getting rid of -U is a good goal.  We just have to be cleverer than
hdtbl's author.  :P

> 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?

No objections; looks good to me.  Go for it!

-- 
Regards,
Branden

signature.asc
Description: PGP signature