groff
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Reverting various commits


From: G. Branden Robinson
Subject: Reverting various commits
Date: Fri, 21 May 2021 19:28:17 +1000
User-agent: NeoMutt/20180716

It must be spring in the Groff Hemisphere, because the flowers of
aggressive contention are once again in bloom...

Hi Ingo,

At 2021-05-20T16:09:59+0200, Ingo Schwarze wrote:
> Hi Branden,
> 
> G. Branden Robinson wrote on Thu, May 20, 2021 at 01:23:18AM -0400:
> 
> > commit bf4b3dde3ba442a0cf52e986d2549f1dc47f43c5
> > Author: G. Branden Robinson <g.branden.robinson@gmail.com>
> > AuthorDate: Thu May 20 11:45:10 2021 +1000
> > 
> >     [mdoc]: Align header/footer spacing with man(7).
> >     
> >     * tmac/mdoc/doc-common-u (doc-end-macro): When continuously
> >       rendering, increase page length by same amount we vertically
> >       space after flushing a pending output line, for symmetry with
> >       other spacing requests (and to prevent nasty surprises
> >       analogous to those in Savannah #60611).
> >     
> >       (doc-header): Put 3 vees of space after the header in
> >       continuous rendering mode, not 1 (and increase page length
> >       accordingly).
> 
> I very strongly object.  Please revert all recent commits manipulating
> mdoc(7) vertical spacing before and after header and footer lines
> (i think there was at least one other during the last one or two
> days).

I can't take this literally.  As part of the above quoted material there
is the following.

> >     * tmac/mdoc/doc-common-u (doc-end-macro): When continuously
> >       rendering, increase page length by same amount we vertically
> >       space after flushing a pending output line, for symmetry with
> >       other spacing requests (and to prevent nasty surprises
> >       analogous to those in Savannah #60611).

If you'll take the time to read Savannah #60611[1] I believe you will
agree that I found and fixed a rendering problem.  Maybe you'll need to
do some experimentation on your own to be convinced.  I sliced my
changes very finely while tracking it down and I'm as confident as I can
be without contriving an mdoc document exhibiting it--to which I did
give some consideration, before deciding that groff_mdoc(7) is
sufficiently lengthy and exhibits sufficient exercise of the mdoc(7)
feature set that I'd catch regressions in my usual process[2].

I encourage you to predict the behavior of the following "plain groff"
document, and experiment with it.  Does it ever output 100 vees of
space?  If so, where?  If not, why not?

$ cat EXPERIMENTS/invisible-trap.groff
.nr a 35 -1
.nr d (u;2v)
.de b
foo \\na \\n[.t]u (\n[d]u)
.  if (\\n[.t] <= \nd) .sp 100v
.  sp
..
.while \n+a \
.  b

I also have a V7-compatible troff version of the above.

(The above document plants no traps.  It just gave me an idea for one we
can use even in continuous rendering mode, however.)

> For nroff output, in the (default) continuous rendering mode, there
> has never been additional vertical whitespace after the header line
> or before the footer line in mdoc(7).  I checked Cynthia's Version 2
> mdoc from 4.3BSD-Reno (1990-06), Cynthia's Version 3 mdoc from
> 4.3BSD-Net/2 (1991-08-20), the oldest version in our groff git
> (1992-09-01), the oldest Version imported into NetBSD (1993-03-21)
> and Cynthias final version from 4.4BSD-Lite/2 (1995-06-23) - they
> are all consistent about this.

Okay.  I trust you as a historian of mdoc(7).

> Also, this additional whitespace makes nothing better, it merely
> wastes screen real estate and looks ugly.

I don't find it objectionable, personally, or I'd have leaned in the
direction you suggest below.

> Not wasting that space apparently was a conscious and reasonable
> choice, as still witnessed by the "Test for crt" comment in the
> doc-old.tmac-u file in our own git, and has never been called into
> question as far as i'm aware.

I don't doubt it.

> I would not oppose making this consistent with man(7) in the opposite
> way, by deleting the spurious blank lines from -man terminal output,
> even though that would break with tradition.  Note that AT&T Version 3
> UNIX (1973-02) had no additional blank lines in these places, Version
> 4 to Version 6 (1975-05) had two blank lines, and Version 7 (1979-01)
> had half an inch of whitespace here, so that tradition is less
> consistent.  Well, it probably has been consistent since 1979.  Either
> way, i don't feel strongly about what may be decided about the legacy
> V7-UNIX man(7) format in this respect.

Traditional man(7) never had continuous rendering mode.  As far as I
know, in the context of man(7) documents, it's groff's innovation, and
we can therefore change it.

I have no objection to doing so; the main reason I didn't was that I
didn't want to disturb existing practice.  I get the feeling you'll tell
me I respect all the wrong traditions.  ;-)

Before we (temporarily) depart historical matters, is it your claim that
mdoc/BSD nroff innovated "continuous rendering mode"?  I ask simply
because I'm curious.

At 2021-05-20T18:09:38+0200, Ingo Schwarze wrote:
> Hi Branden,
> 
> this is the other commit that i was talking about that needs
> reverting.  I'm not sure whether there is a third one; maybe not.  The
> commit 456dad17ba6a86738b980c21bd5a88eab25e2840 (Nov 18 18:44:24 2020
> +1100) is still OK i guess.

That commit (456dad17) is completely unrelated to spacing issues, and a
consensus was reached on this list regarding it, as cross referenced in
the commit message.  What relevance does it have to your present
complaints?

> > commit e51af2a52fff97807143d20ee58d745105d237b6
> > Author: G. Branden Robinson <g.branden.robinson@gmail.com>
> > AuthorDate: Thu May 20 10:47:56 2021 +1000
> > 
> >     [mdoc]: Space before footer as our man(7) does.
> >     
> >     * tmac/mdoc/doc-common-u (doc-end-macro): When continuously
> >       rendering and after flushing the last line of the body text of
> >       a page, vertically space by 3 vees instead of 1, for
> >       consistency with our man(7) implementation.
> >     
> >     This was possibly an oversight in 058f72af8 (23 March 2001--the
> >     Great Groff Mdoc Rewrite); earlier in the macro the page length
> >     was already increased by 3v as if in anticipation of spacing by
> >     this amount.

I need you to account for the existing ".pl +3v" when the actual spacing
nearby was only of one vee.  If you can't, then an outright git revert
is not in order, but instead a further modification to make the .sp and
the .pl both use the same amount of space (1v).

At 2021-05-20T19:04:20+0200, Ingo Schwarze wrote:
> > commit 69863462003ed26b8da81ab174b0a6c0a7d26eea
> > Author: G. Branden Robinson <g.branden.robinson@gmail.com>
> > AuthorDate: Thu May 20 10:28:26 2021 +1000
> > 
> >     tmac/an-old.tmac: Refactor DT and PD.
> >     
> >     * tmac/an-old.tmac: Refactor to move bodies of DT and PD into
> >       private macros.
> >     
> >       (an-reset-tab-stops, an-reset-paragraphp-spacing): New names
> >       for the former DT and PT.
> >     
> >       (TH): Call these new macro names.
> >     
> >       (DT, PD): Wrap the corresponding private macros.

> I think this is terrible roff(7) coding style and should be reverted.

I modeled it on an example from our Texinfo manual, which you have
earlier fulsomely praised on this list[4], and which in these
particulars long predates my involvement with groff.

In the groff 1.22.4 info manual, we find the following.

---SNIP---
 5.19 Strings
 ============

 'gtroff' has string variables, which are entirely for user convenience
 (i.e. there are no built-in strings except '.T', but even this is a
 read-write string variable).
[...]
      Strings, macros, and diversions (and boxes) share the same name
      space.  Internally, even the same mechanism is used to store them.
      This has some interesting consequences.  For example, it is
      possible to call a macro with string syntax and vice versa.
[...]
      In particular, interpolating a string does not hide existing macro
      arguments.  Thus in a macro, a more efficient way of doing

           .xx \\$@

      is

           \\*[xx]\\

      Note that the latter calling syntax doesn't change the value of
      '\$0', which is then inherited from the calling macro.
---SNIP---

This construction is idiomatic enough--for groff--that we find many
occurences of it in:
  contrib/mm/m.tmac
  contrib/mom/om.tmac
  tmac/psatk.tmac
  tmac/pspic.tmac
  tmac/s.tmac

It also of course occurs in andoc.tmac, an-old.tmac, an-ext.tmac, but
some of those can be attributed to me.  For a brief example of prior
art, I refer you to 7351ffde2ef55946eb22d49359885c4c1bac21d1[5], from 4
October 2008.

Granted, the trailing \\newline is not always present, but it doesn't
always have to be; its necessity is context-dependent, and the manual,
as is pedagogically proper, gives a recipe for a _reliable_ method that
one can "optimize" on a case-by-case basis with knowledge appropriate to
the situation.

> I needed to scratch my head for a considerable time to even understand
> why and how it works, even though i'm maintaining a roff(7) parser
> myself.  How are people supposed to understand such obfuscated code
> who do not maintain roff parsers?

By consulting, at some point in the past 13 years, portions of our
documentation that Werner wrote?

> It appears your code works because your new implementation of the
> .PD macro delays evaluation of the \*[an-reset-paragraph-spacing]
> string until macro expansion time.  At expansion time, the
> .ie-.el-roff(7) code gets inserted into the input stream, expanding
> the \\$1 argument, and since the inserted code happens to be at the
> beginning of an input line, the inserted code gets then parsed as
> roff(7) input, setting the .PD register as desired.  It would no
> longer work when indented, so it's fragile in addition to being
> visually misleading.  By the way, your implementation violates the
> indentation convention used in the rest of the file.

It's a text line, not a control line.

> That could be fixed by saying
> 
>   .de1 PD
>   .  nop \\*[an-reset-paragraph-spacing]\\
>   ..

Thank you at least for a use case for .nop, which Dave Kemper and I have
been casting about for recently.

> but i wouldn't consider that sufficiently readable either.

Okay.

> So you are obfuscating what is going on by
> 
>  1. Defining a macro the supports an optional argument without
>     any argument expansion ever appearing in the definition of the
>     macro (PD);

It's documented in the man page, but I'm happy to add a comment line
above the implementation summarizing its syntax.  This was on my mental
to-do list for all of the macros in the file.

As for the next two points...

>  2. using string expansion that actually amounts to a macro call
>     (\\*[an-reset-paragraph-spacing]);
> 
>  3. and also hiding the fact from the reader of the code that an
>     optional argument is being passed by the macro call
>     \\*[an-reset-paragraph-spacing].
> 
> I searched for this kind of obfuscation anywhere else in an-old.tmac,
> but all the other instances of lines starting with \\* or .nop \\
> actually expand strings rather than calling macros.
> 
> If you want to call a macro, please do write it as a macro call,
> not as string expansion.  For passing variable numbers of arguments,
> the escape sequences \\$* and \\$@ are available, and sometimes,
> even just using \\$1 for a single optional argument may be good
> enough, because in many situations, appending a space character
> and an empty string to a macro line changes nothing, not even \\n[.$].

...I'm not interested in litigating them; they consist of objections to
a groff language feature of significant age rather than to my commit per
se.  If Werner wants to defend his design decisions of many years ago to
you right now, he can.

For my own part I can say that I don't find this instance of syntax
particularly recondite.  But maybe I've already been corrupted...

The bottom line is that I'm using groff idiomatically and as documented.

> Finally, i'm not sure why new macros are needed at all.
> Even if you want to add deprecation diagnostics, why not just insert
> the required code into the existing macros?

Because I plan to use it multiple places and I don't need the wording to
vary on a per-deprecated-macro basis.  Everything except the name of the
deprecated macro is invariant.  I plan to extend application of
an-deprecation-warning beyond just these two, to encompass the others
described thus in groff_man(7).

But first I want to make sure our own man pages are clear of problems
this macro would complain about.  groff(7) has already set me a
significant task in this area, and the SY macro in an-ext is a much
worse problem because it's implemented in terms of .HP (the internals of
which could be cribbed, but I'm wondering if there's a better way to
skin that cat that would also improve .SY's HTML output).

Regards,
Branden

[1] https://savannah.gnu.org/bugs/?60611
[2] I render all 62 of our man pages three different ways: with
    continuous rendering in "batch mode", i.e., all 62 files given as
    operands to one groff command; separately with continuous rendering
    on; and separately with continuous rendering off.  The last is
    useful to catch almost anything that might go wrong in PS or PDF
    output, without the tedium of visually comparing such output.
[3]
    .nr a 35 -1
    .nr d 2v
    .de b
    foo \\na \\n(.tu (\ndu)
    .  if \\n[.t]<=\nd .sp 100v
    .  sp
    ..
    .de c
    .  b
    .  if \\n+a .c
    ..
    .c
[4] https://lists.gnu.org/archive/html/groff/2020-06/msg00046.html
[5] https://lists.gnu.org/archive/html/groff-commit/2008-10/msg00008.html

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]