Re: [groff] hyphenation issues

[G. Branden Robinson]

At 2018-05-06T12:10:13+0100, Ralph Corderoy wrote:
> > My debugging instincts tell me to always report the value of an
> > argument being rejected.  For one thing, the user could have passed
> > .hy a register value (possibly with arithmetic manipulation), and if
> > we screw up the validity checks it's easier to see which cases are
> > incorrect.
> 
> Definitely.  Another thing to look out for is the same error message
> being generated at multiple points as different criteria fail.  I'm left
> with an error and the source and don't know which one printed.

Oh, yes.  I once preached to a lecture hall of sophomore Java
programming students the following commandment:

"Never, EVER, use exactly the same string as a diagnostic message in
more than one place in your code."

Backtraces are great but we don't always have them.  Untold hours and
money are saved by knowing at exactly which line the source code decided
there was a problem.

> > +  // The _usable_ maximum is 52; see hyphenate_request() below.
> > +  HYPHEN_MAX = 63,
> 
> Not based on the `hy.trf.out' you provide.

Yes, I had a momentary lapse of literacy.  The useful maximum is 50.

> I'd just say `Not all values are legal; see hyphenate_request()' as
> otherwise it's repeating information: the function's logic, and its
> relative position.

Yes, that would have saved embarrassment.  ;-)

> > -      warning(WARN_SYNTAX, "contradicting hyphenation flags, ignored");
> > +      warning(WARN_SYNTAX, "contradictory flags in hyphenation request "
> > +   "value '%1'; ignored", n);
> 
> It seems to have grown wordy.  Unix convention, I don't know about
> groff's, would be more like
> 
>     clashing hyphenation flags ignored: %1

That's fine, though I would keep the word "contradictory" because
"clashing" is a little bit idiomatic.  Groff does not appear to have
localized diagnostics, so we should be friendly to non-native speakers
when we can.

> > +      warning(WARN_RANGE, "hyphenation request value '%1' out of range "
> > +   "(must be in %2..%3); ignored", n, HYPHEN_NONE, HYPHEN_MAX);
> 
> This is easier to phrase with two tests.
> 
>     negative hyphenation flags ignored: %1
>     unknown hyphenation flags, maximum %2, ignored: %1
> 
> But it's just a suggestion.  You're the one doing the work!

I'll tweak this a bit.

Thanks for the review!

-- 
Regards,
Branden

signature.asc
Description: PGP signature