bugGNU roff - Bugs: bug #59795, interaction of .na and '.ad l'...

 
 

bug #59795: interaction of .na and '.ad l' documented incorrectly

Submitter:  Bjarni Ingi Gislason <bjarniig>
Submitted:  Fri 01 Jan 2021 01:10:33 PM UTC
   
 
Category:  Core Severity:  3 - Normal
Item Group:  Documentation Status:  Fixed
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  1.23.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 28 Jan 2021 08:32:04 AM UTC, comment #15: 

comment #14:

> comment #13:
> > My brain screams for orthogonality, and James Clark just didn't provide it in \n[.j].
>
> Is there any reason his .j values must be preserved, given that
> * the documentation (now) states, "The value of '.j' ... is an implementation detail and should not be relied upon as a programmer's interface," and
> * no mapping of adjustment modes to .j values has ever been documented, so it's not something document authors should have been relying on even before this warning was spelled out
> ?


No, no reason.  If we were doing "next-generation adjustment control" to go along with "next-generation hyphenation control" (bug #55070), then I'd propose splitting out enablement status into a separate register (perhaps \n[.ad] or \n[.adjust]), preserving the mode-selection semantics of .j if possible.

This isn't merely a matter of preference or design biases on my part; if we ever get support for right-to-left languages, then the difference between `.na` and `.ad l` could become much more relevant and evident.

G. Branden Robinson <gbranden>
Group administrator
Wed 27 Jan 2021 01:19:11 PM UTC, comment #14: 

comment #13:

> My brain screams for orthogonality, and James Clark just didn't provide it in \n[.j].


Is there any reason his .j values must be preserved, given that

  • the documentation (now) states, "The value of '.j' ... is an implementation detail and should not be relied upon as a programmer's interface," and
  • no mapping of adjustment modes to .j values has ever been documented, so it's not something document authors should have been relying on even before this warning was spelled out

?

Dave <barx>
Group Member
Wed 27 Jan 2021 07:04:58 AM UTC, comment #13: 

And still more.

My brain screams for orthogonality, and James Clark just didn't provide it in \n[.j].


commit e8141665c122fea3d6abe68487c55650db351b13 (HEAD -> master, origin/master, origin/HEAD)
Author:     G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Wed Jan 27 14:13:59 2021 +1100
Commit:     G. Branden Robinson <g.branden.robinson@gmail.com>
CommitDate: Wed Jan 27 14:13:59 2021 +1100

    doc/groff.texi: Continue fixing recent error.

    Continues ba4c88e5741d05e7edd265c09b0073e7220cc71c.

diff --git a/doc/groff.texi b/doc/groff.texi
index c496272f..d81e592c 100644
--- a/doc/groff.texi
+++ b/doc/groff.texi
@@ -6935,9 +6935,9 @@ detail and should not be relied upon as a programmer's interface.
 @endDefreq

 @Defreq {na, }
-Disable adjustment.  This looks the same as adjustment to the left
-margin, but the value of the adjustment mode register @code{.j} is not
-changed.
+Disable adjustment.  This produces the same output as adjustment to the
+left margin, but the value of the adjustment mode register @code{.j} is
+altered differently.

 The adjustment mode and enablement status are associated with the
 current environment (@pxref{Environments}).


G. Branden Robinson <gbranden>
Group administrator
Tue 26 Jan 2021 04:58:56 AM UTC, comment #12: 

Re-fixed.


commit ba4c88e5741d05e7edd265c09b0073e7220cc71c (HEAD -> master, origin/master, origin/HEAD)
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Tue Jan 26 03:00:59 2021 +1100

    doc/groff.texi, groff(7): Fix recent error.

    I was wrong to document .na and ".ad l" as synonymous in commits
    0f7e6a8b and e79f5314 on 24 January; they have the same visible effect
    on text, but they alter the .j register (which encodes both adjustment
    mode and adjustment enablement status) differently.

    Thanks to Bjarni Ingi Gislason for the close review.


G. Branden Robinson <gbranden>
Group administrator
Mon 25 Jan 2021 04:50:25 AM UTC, comment #11: 

comment #10:

> It also smells a little funny to start right-adjusting if the adjustment mode is out of range (that's what adjust_mode = 5 means).  I'll have to see if AT&T troff supported numeric adjustment modes; it is already known that V7 troff did not support the .j register.


Sigh.

V7 /usr/src/cmd/troff/n5.c:


casead(){
        register i;

        ad = 1;
        /*leave admod alone*/
        if(skip())return;
        switch(i = getch() & CMASK){
                case 'r':       /*right adj, left ragged*/
                        admod = 2;
                        break;
                case 'l':       /*left adj, right ragged*/
                        admod = ad = 0; /*same as casena*/
                        break;
                case 'c':       /*centered adj*/
                        admod = 1;
                        break;
                case 'b': case 'n':
                        admod = 0;
                        break;
                case '0': case '2': case '4':
                        ad = 0;
                case '1': case '3': case '5':
                        admod = (i - '0')/2;
        }
}
casena(){
        ad = 0;
}


Warden Leo Glynn is my spirit animal.

G. Branden Robinson <gbranden>
Group administrator
Mon 25 Jan 2021 04:44:57 AM UTC, comment #10: 

Bjarni is correct.  I was attempting to infer groff (and Heirloom) behavior in black-box fashion, and did not contrive enough experimental cases.

But the implementation is sufficiently strange that it's easier to get an understanding by quoting the code.

From src/roff/troff/env.cpp:


void adjust()
{
  curenv->adjust_mode |= 1;
  if (has_arg()) {
    switch (tok.ch()) {
    case 'l':
      curenv->adjust_mode = ADJUST_LEFT;
      break;
    case 'r':
      curenv->adjust_mode = ADJUST_RIGHT;
      break;
    case 'c':
      curenv->adjust_mode = ADJUST_CENTER;
      break;
    case 'b':
    case 'n':
      curenv->adjust_mode = ADJUST_BOTH;
      break;
    default:
      int n;
      if (get_integer(&n)) {
        if (n < 0)
          warning(WARN_RANGE, "negative adjustment mode");
        else if (n > 5) {
          curenv->adjust_mode = 5;
          warning(WARN_RANGE, "adjustment mode '%1' out of range", n);
        }
        else
          curenv->adjust_mode = n;
      }
    }
  }
  skip_line();
}

void no_adjust()
{
  curenv->adjust_mode &= ~1;
  skip_line();
}


From this we can see that adjustment enablement is simply the least significant bit of the adjustment mode.  Where things get weird is the value of the ADJUST_LEFT constant, but we shouldn't scar users' minds with that.

I'm dubious about this bit:


        else if (n > 5) {
          curenv->adjust_mode = 5;
          warning(WARN_RANGE, "adjustment mode '%1' out of range", n);
        }


This should be comparing to an ADJUST_MAX constant instead of a literal.  It also smells a little funny to start right-adjusting if the adjustment mode is out of range (that's what adjust_mode = 5 means).  I'll have to see if AT&T troff supported numeric adjustment modes; it is already known that V7 troff did not support the .j register.


G. Branden Robinson <gbranden>
Group administrator
Mon 25 Jan 2021 04:33:16 AM UTC, comment #9: 

Reopening.  In an email to groff@ that I unaccountably did not receive at my GMail account, Bjarni wrote:


The page has been changed a little,
instead of a single "Submit Changes"
there are now two buttons.

  This is the content of my "Comment":

+Disable adjustment.  This is synonymous with the request
+@w{@samp{.ad l}}; the value of the adjustment mode register @code{.j}
+is not changed.

 Remove the second sentence.

 a) Don't mention '.ad l'; it can cause confusion; '.na' has not the
same internal effect (on register '.j').

 b) '.na' does change the value of the register '.j' (except for the
value '0').

  Why should "disable adjustment" not be enough?

  Or add (something like this):

Adjustment can be restored with

a) '.ad' (without an argument) to
the previous adjustment mode (before the '.na' request),
if there is no intervening '.ad <argument>' request in the same
environment.

b) a saved '.j' register before the '.na' request,
using ".ad \n[saved_.j_register]".

c) a different type of adjustment using '.ad <argument>'.


G. Branden Robinson <gbranden>
Group administrator
Sun 24 Jan 2021 07:04:28 AM UTC, comment #8: 

Forgot to fix the man page.  Done now.


commit 0f7e6a8be8ed2cd0bdcc91e18b27786431deb18b (HEAD -> master, origin/master, origin/HEAD)
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Sun Jan 24 17:56:41 2021 +1100

    groff(7): Clarify synonymy of ".na" and ".ad l".

    Continues fixing <https://savannah.gnu.org/bugs/?59795>.


G. Branden Robinson <gbranden>
Group administrator
Sun 24 Jan 2021 06:20:32 AM UTC, comment #7: 


commit e79f5314db2c474d0117e093c9b0abc1de5ced78
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Sun Jan 24 16:00:08 2021 +1100

    doc/groff.texi: Update filling/adjustment matter.

    * doc/groff.texi (Manipulating Filling and Adjustment): Clarify synonymy
      of ".na" and ".ad l".

      Thanks to Bjarni Ingi Gislason for pointing out the potential for
      confusion.

      Fixes <https://savannah.gnu.org/bugs/index.php?59795>.

    Also:
      - Define the concept of a "pending output line", including index
        entry.  (Some call this a "partially collected line".)
      - Use this concept consistently in description of requests (in this
        node) that cause breaks.
      - Update .br example to illustrate usage with both control characters.
      - Use the more idiomatic noun "adjustment" instead of "adjusting".
      - Say "GNU troff" instead of "gtroff".
      - Clarify "status" of filling and adjustment as "enablement status",
        reflecting their boolean state (and in the latter case to
        distinguish it from the available "modes" of adjustment).
      - Clarify when a change to adjustment mode takes effect (it does _not_
        cause a break).
      - Supply possible mnemonic for ".ad n"--adjust "normally" (for *roff).
      - Tweak '.ad' example to use more suggestive example text.
      - Warn people off assuming that values of the .j register are stable
        across implementations.
      - Add .ll request to "\p" example to ease reproducibility.
      - Describe .ce and .rj better, fronting essential information.
      - Adjust ".ad c" and ".ce" example to use macro for repeated text,
        place typographical quotes (if available) in example output, and
        match break locations with actual groff output.
      - Describe .rj in its own right, not in terms of .ce.
      - Tighten wording.
      - Move @codequote* commands to mark this node as reviewed for correct
        grave accent and apostrophe glyph usage.


G. Branden Robinson <gbranden>
Group administrator
Sat 23 Jan 2021 01:23:55 PM UTC, comment #6: 

Attaching a patch to groff.texi.  Please review.

I revised several examples to be more illustrative and/or to match actual groff output.

Still to be done:
1. Break this up into a few of discrete commits; I found and fixed some other (minor) problems.
2. See if any of the man pages need an update.

(file #50769)

G. Branden Robinson <gbranden>
Group administrator
Sat 23 Jan 2021 10:39:35 AM UTC, comment #5: 

Thanks, Dave.  You're right; Bjarni's example in bug #55579 exposes a legitimately confusing point.  I'm attaching a slight revision of his reproducer from that bug that I think reveals the situation more clearly.  Thanks for providing it, Bjarni.

Without worrying about the numeric values of the .j register, I think we can say that .na causes the previous adjustment mode to be remembered if the adjustment mode isn't already "l" (off), and that .na is a no-op if the current adjustment mode is "l", and vice versa.

I strongly feel that we should not burden the documentation with details of the implementation of the .j register or the numerical representation of the adjustment modes.  All a user needs to know is that they can save .j into another register and interpolate that saved value into the argument of an .ad request.  It seems that Heirloom Doctools troff and GNU roff happen to use the same implementation, but that should be treated as a coincidence.

I'm working on a rewrite of the "Manipulating Filling and Adjustment" node of our Texinfo manual right now to clean this up.

Here's the output of my reproducer, based on Bjarni's.  It's the same in Heirloom and groff.


Adjustment type is 1.   Line  to
test adjustment.
Adjustment type is 0.  Line to
test adjustment.
Adjustment type is 0.  Line to
test adjustment.
Adjustment type is 1.   Line  to
test adjustment.


G. Branden Robinson <gbranden>
Group administrator
Thu 21 Jan 2021 09:23:05 AM UTC, comment #4: 

Thanks, Branden.  I asked because this report didn't make it entirely clear to me what the problem was.

The example in bug #55579--showing the results of the adjustment, not just the value of the .j register--better highlights the difference between what groff does and what the Texinfo says.  According to the .na section, .na "won't change the current adjustment mode: A subsequent call to 'ad' uses the previous adjustment setting."  The 55579 example shows a subsequent call to 'ad' using something other than the previous (pre-.na) adjustment setting.

What the 55579 example doesn't show (but is easy to verify with a single-character edit to it) is that any other adjustment mode besides "l" is remembered after an .na call.  ".ad l" is the exception.

Even if that is historical troff practice, it's a weird incongruity hard to justify on any other grounds.  But it should at least be documented if it stays that way.

Dave <barx>
Group Member
Thu 21 Jan 2021 05:02:34 AM UTC, comment #3: 

Is this bug a dup of bug #55579?

Dave <barx>
Group Member
Mon 18 Jan 2021 08:43:28 AM UTC, comment #2: 

I'm dubious about this.

My observations:

1. groff as currently implemented is compatible with Heirloom Doctools troff in this respect.  Both Heirloom and groff extend AT&T troff to expose a .j register which reports the numerical value of the adjustment mode.  I'll bet this was done so that you could save it; see point 3 below.

2. CSTR #54 (1992) is a bit slippery in its terminology here.  Sometimes it uses the word "adjust" to mean adjustment to both margins, and sometimes to the collective possible values of the .ad request.  Nevertheless it is clear that the "initial default" for the formatter is "adj,both", and that if no argument is given to the request, the implicit command is to "adjust".  How?  To both margins.

3. AT&T troff was inconsistent about requests stashing state; sometimes you got a 1-level stack to which an ".xx YY" request pushed and from which ".xx" popped, and sometimes AT&T troff was just clobberific.  I myself had assumed that .na caused the previous adjustment mode to be remembered, but upon investigation that appears not to be true.

Changing this would be really disruptive, perhaps to the extent of requiring a new request to support the desired semantics, if anything is done at all.

I'm inclined to close this as invalid, but would appreciate comments from others.

G. Branden Robinson <gbranden>
Group administrator
Mon 18 Jan 2021 08:25:06 AM UTC, comment #1: 

You mean line adjustment, not "link" adjustment.

G. Branden Robinson <gbranden>
Group administrator
Fri 01 Jan 2021 01:10:33 PM UTC, original submission:  

src/roff/troff/env.cpp: fix wrong restoration of link adjustment after
'.na' and '.ad'

a) Example:

.ad l
\n(.j
.na
\n(.j
.ad
\n(.j

  Result:

0 0 1

  instead of

0 0 0

  The '.na' request does

1) .j = 0 no change

2) .j > 0 changes the value to the next lower even number

  The 'ad' request does (without an argument)

1) .j = 0 changes the value to the next higher uneven number

2) .j > 0 same change as for .j = 0

  This works correctly when the current value for .j > 0,
but not for .j = 0, as it must be unchanged (links) and
not "both" (.j = 1).

b) Use a name for the maximal defined value of '.j'.

Signed-off-by: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
---
 src/roff/troff/env.cpp | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/roff/troff/env.cpp b/src/roff/troff/env.cpp
index bade3927..bd90244f 100644
--- a/src/roff/troff/env.cpp
+++ b/src/roff/troff/env.cpp
@@ -35,7 +35,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>. */

 symbol default_family("T");

-enum { ADJUST_LEFT = 0, ADJUST_BOTH = 1, ADJUST_CENTER = 3, ADJUST_RIGHT = 5 };
+enum { ADJUST_LEFT = 0, ADJUST_BOTH = 3, ADJUST_CENTER = 5,
+  ADJUST_RIGHT = 7, ADJUST_MAX = 7 };

 enum {
   // Not all combinations are valid; see hyphenate_request() below.
@@ -2503,7 +2504,8 @@ void title()

 void adjust()
 {
-  curenv->adjust_mode |= 1;
+  if ( curenv->adjust_mode != 0 )
+    curenv->adjust_mode |= 1;
   if (has_arg()) {
     switch (tok.ch()) {
     case 'l':
@@ -2524,8 +2526,8 @@ void adjust()
       if (get_integer(&n)) {
         if (n < 0)
           warning(WARN_RANGE, "negative adjustment mode");
-        else if (n > 5) {
-          curenv->adjust_mode = 5;
+        else if (n > ADJUST_MAX) {
+          curenv->adjust_mode = ADJUST_MAX;
           warning(WARN_RANGE, "adjustment mode '%1' out of range", n);
         }
         else
--
2.29.2


Bjarni Ingi Gislason <bjarniig>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #50769:  59795.diff added by gbranden (19KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by barx (Posted a comment)
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by bjarniig (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 15 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-01-26 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2021-01-25 gbranden StatusFixed In Progress
        Open/ClosedClosed Open
    2021-01-24 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
        Planned ReleaseNone 1.23.0
    2021-01-23 gbranden Attached File- Added 59795.diff, #50769
    2021-01-23 gbranden Item GroupIncorrect behaviour Documentation
        StatusNeed Info In Progress
        Summary[PATCH] troff/env.cpp: fix wrong restoration of line adjustment after '.na' and '.ad' interaction of .na and '.ad l' documented incorrectly
    2021-01-18 gbranden StatusIn Progress Need Info
    2021-01-18 gbranden StatusNone In Progress
        Assigned toNone gbranden
        Summary[PATCH] troff/env.cpp: fix wrong restoration of link adjustment after '.na' and '.ad' [PATCH] troff/env.cpp: fix wrong restoration of line adjustment after '.na' and '.ad'

    Back to the top

    Powered by Savane 3.13-bb6a.
    Corresponding source code