Re: pic anomalies

[Ingo Schwarze]

Hi Ralph,

Ralph Corderoy wrote on Mon, Dec 30, 2019 at 06:31:34PM +0000:
> Ingo Schwarze wrote:

>>  * line 1896, while (*form):
>>    Testing char as if it were boolean seems dubious style to me;
>>    "while (*form != '\0')" would seem clearer because it makes the
>>    data type obvious on first sight.

> It's idiomatic in C to write just `foo' to test it against its `zero
> value' whether it's an int, pointer, etc.

You are right, coding styles differ on that.  I noticed the style
to be inconsistent in this very function and gave a short explanation
why i suggested to stick to one of the styles already in use.
I don't feel strongly about it even though OpenBSD does consistently
encourage pointer == NULL and character == '\0' and integer == 0
everywhere and reserves !integer to int variables that are supposed
to represent only boolean values.

Let's see what Colin will do.  :-)

[...]
> If I've a `bool b', I don't write `b != false' to hint that b is a bool.

Of course not.  I think all coding styles will agree that "if (b)"
is fine if b is boolean.

[...]
> Getting back on topic, are we sure we want to deviate from GNU pic's
> current behaviour without checking historical norms of other pics?
> 
>     $ printf '%s\n' \
>       .PS 'print sprintf("%.17g %.0f% % %%", 3.14, 42, 99)' .PE |
>     > pic >/dev/null
>     3.1400000000000001 42% % %%
> 
> Though that may seem odd to our modern C-standardised eyes, it's
> understandable in that if it isn't a valid %f, etc., format specifier
> then it's a literal percent sign.

   $ printf '%s\n' \                                             
    .PS 'print sprintf("%.17g %.0f% % %%", 3.14, 42, 99)' .PE | 
    /usr/local/heirloom-doctools/bin/pic | grep -F 3.14         
  3.1400000000000001 42% %

   $ printf '%s\n' \                                             
     .PS 'print sprintf("%.17g %.0f% % %%", 3.14, 42, 99)' .PE | 
     /usr/local/plan9/bin/pic | grep -F 3.14                     
  3.1400000000000001 42% %

which both agree with what we currently have in git HEAD:

   $ printf '%s\n' \                                             
     .PS 'print sprintf("%.17g %.0f% % %%", 3.14, 42, 99)' .PE | 
     /usr/local/bin/pic | grep -F 3.14                           
  3.1400000000000001 42% %

Usually, both Heirloom doctools and Plan9 respect historical behaviour
quite well, so i don't feel like digging for historical implementations
right now.

So i don't think the change i committed goes against historical practice
but that it rather fixed a genuine groff bug.

> That example also happens to show GNU pic gives no warning of unused
> arguments.

That's a separate matter...  As i said in another posting, i'm quite
sure doing a code review across the groff tree would reveal large
amounts of aspects that can be improved.

Yours,
  Ingo