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

[Deri]

On Saturday, 12 November 2022 20:11:09 GMT Deri wrote:
> On Saturday, 12 November 2022 19:43:35 GMT G. Branden Robinson wrote:
> > gbranden pushed a commit to branch master
> > in repository groff.
> > 
> > commit 4753f2b17b8d836cf66fcb17f5412239e8b45743
> > Author: G. Branden Robinson <g.branden.robinson@gmail.com>
> > AuthorDate: Tue Nov 8 19:26:24 2022 -0600
> > 
> >     [gropdf]: Provide more info in diagnostic message.
> >     
> >     * src/devices/gropdf/gropdf.pl: Provide more information in diagnostic
> >     
> >       issued when an embeddable font is required by the document and
> >       listed
> >       in the "download" file, but can't be opened.
> >       
> >       (LoadDownload): Construct a "path" of each file specification
> >       attempted.  Store this as the value for the hash `missing` keyed on
> >       the tuple of scalars `foundry` and `name`, instead of the name of
> >       the
> >       device description directory, which isn't very helpful for
> >       troubleshooting.  Also: don't start message clause with a capital
> >       letter (per GNU Coding Standards); double quote file name
> >       references;
> >       and move parenthesized groff font name outside of single quotes.
> >       
> >       (LoadFont): Recast diagnostic message to communicate the foregoing
> >       information.  Also make embedding failure messages more consistent
> >       in
> >       style.
> > 
[...]
> Hi Branden,
> 
> Please revert this until we have finished our discussion. You are too quick
> for me, I wish I could type as fast as you.
> 
> Thank you.
> 
> Cheers
> 
> Deri

Hi Branden,

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. 
Even more confusing is that you concatenate paths from different download 
files, so this one message could be referring to multiple different download 
files.

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

Since you didn't wait for me to comment on your proposed changes before 
committing I thought it best not to push my latest fixes for the landscape 
hotspot issue notified by Blake until you have done your reverts. But I have 
attached them as a diff to gropdf before your changes, so can be applied 
after.

Cheers 

Deri

Landscape.diff
Description: Text Data