Re: [groff] 04/40: [gropdf]: Provide more info in diagnostic message.

[G. Branden Robinson]

Hi Deri,

At 2022-11-16T22:02:49+0000, Deri wrote:
> In the case when a particular entry in a download file points to a
> file which is not readable, i.e. is added to the missing list, there
> are three items of information which are useful:-
> 
> The particular download file which contains the incorrect entry.
> The postscript name of the font.
> The groff font name which includes the foundry letter.
> 
> This allows the particular download file to be opened with an editor,
> and the errant line located using the foundry letter and postscript
> name, aided by the fact this is the sort order of the file.
> 
> This is the message before and after your change.
> 
> Original:-
> 
> /usr/bin/gropdf:warning: The download file in '/usr/share/groff/site-font/
> devpdf'  has erroneous entry for 'Times-Roman (TR)'
> 
> Your version:-
> 
> /home/derij/groff-git/groff/build/gropdf:warning: unable to embed font file 
> for 'Times-Roman' (TR); check "download" file(s) (tried: "/usr/share/groff/
> site-font/devpdf/missing")
> 
> This does not tell you which download file is affected but the
> incorrect entry instead, and even that is confusing since if I
> eventually look in site-font/ devpdf/download I would see:-
> 
>       Times-Roman     missing
> 
> Since this is a relative pathname, so a user may be confused by your message. 

That's true, however there is other information not being shared.
Here's what I'd like to see:

1. Explication of what operation--that the user cares about--has failed.
   What is the _consequence_ of a download file having an "erroneous
   entry"?  If we say we can't embed the font, we're addressing the
   user's desires.  Reading download files and opening PFAs is only a
   means to an end.

2. Identification of which "download" file is a culprit.  There's
   nothing to stop multiple bad entries for the same font from
   occurring either in one "download" or across multiple ones.

   "The download file in '/usr/share/groff/site-font/devpdf' has
   erroneous entry for 'Times-Roman (TR)'"

   A novice may not understand that we're talking literally about a file
   called "download".  They might also think we're not native English
   speakers (or are not themselves) and misinterpret it as "the
   downloaded file...".  I think it is clearer and friendlier to hand
   the user the entire problematic filespec at issue.

3. Which operation was attempted that actually failed.  Saying that an
   entry is erroneous communicates much less than saying what our
   program did that went wrong.

4. The underlying system error--was it EPERM?  ENOENT?  Perl gives us
   `$!` on a platter for inclusion in diagnostics.  That information
   would aid the user in the above case.  They are less likely to say,
   "what's wrong with my entry?  I've got it right there!" if our
   diagnostic includes "No such file or directory".  We should of course
   make it crystal clear _which_ file is suffering the problem, since
   we're juggling one or more "download" files and one or more Type 1
   font files listed in one or more of those "download" files.

I therefore propose something like:

gropdf: warning: cannot embed font for 'TR':
"/usr/share/groff/site-font/devpdf/download" has invalid entry for
Times-Roman; could not open ".../timesrom.pfa" (Permission denied)

(lines broken only for email)

Yes, it's long.  And yes, every download file with bad entries like this
is going to produce diagnostic output like it if there is not an
eventual success in locating the Type 1 font in question.  On the bright
side, this output should leave the user with few questions about what is
wrong and what they need to do to fix it.

> Even more confusing is that you concatenate paths from different
> download files, so this one message could be referring to multiple
> different download files.

That's largely a consequence of this somewhat unusual procedure (among
groff diagnostic messages) of gathering up some problems and then
running a filter on them later, which can suppress them in whole or
part.   Most everywhere else we throw a diagnostic, we throw it at the
site the problem occurs (possibly filtered by some sort of "user
interest mask" like GNU troff's warning categories).

> I'm afraid I have failed to see how this can be considered an
> improvement.

I concede that you have identified real flaws in my change.  See my
revised proposal above.

> Since you didn't wait for me to comment on your proposed changes
> before committing

Right; as we discussed in private mail, I thought a "handoff" had been
performed in Savannah #62950.[1]

> I thought it best not to push my latest fixes for the landscape
> hotspot issue notified by Blake until you have done your reverts.

I don't think that is necessary; the hunks of your diff affect lines ca.
319 and 1490 of "gropdf.pl".  The commit to which you're objecting
(4753f2b17b8d836cf66fcb17f5412239e8b45743) altered lines around 676 and
2555.  In my experience that is easily generous enough spacing; git
reverts are not limited to the immediately previous change to a file.

So I think it unlikely that any changes to relocate the hotspot will
interfere with font embedding-related work.

> But I have attached them as a diff to gropdf before your changes, so
> can be applied after.

It need not wait; feel free to go ahead and commit now.  If you'd prefer
I did it for whatever reason, let me know--I'll take care of it, with
you as --author of course.

As noted in private, I feel gated on the reversions because I haven't
yet heard from you on whether you approve of any of my proposed
mechanisms for causing the build to fail if font embedding fails when
generating "groff-man-pages.pdf" (and thereby "keeping the tree green"
in this respect).  To me, running Poppler's pdffonts(1) to scan the
generated file is extremely undesirable because doing so doubles groff's
build time on my system (and surely others').  I'm open to suggestions.

Regards,
Branden

[1] https://savannah.gnu.org/bugs/?62950

signature.asc
Description: PGP signature