(was: Documenting a set of functions with -man)

At 2024-06-23T23:03:50+0000, Bjarni Ingi Gislason wrote:
>   There are two issues here,
> 
> 1) wrong use of the adjustment requests

I agree.  But I also think the traditional *roff behavior here is badly
counterintuitive.

Reviewing the language of CSTR #54 ยง4.2 (1976 and 1992 revisions differ
little here), I think I see why.  I don't find it clear.  It says in the
`ad` request description that the "margins" get adjusted; what gets
adjusted is the text (specifically, the output line).  It also says that
the "initial value" is "adj,both", which I can comprehend, but that the
"if no argument" implication is "adjust"--without "both".[1]

But the formatter doesn't behave that way, as I understand the words
"alignment" and "adjustment".

Consider the following document.

$ cat ATTIC/madness.roff
.pl 1v
.ll 9n
abc def\p
.na
ghi jkl\p
.ad l
mno pqr\p
.na
stu vwx\p
.ad
yza bcd\p
.ad l
.ad b
efg hij\p
.na
klm nop\p
.ad c
qrs tuv\p
.na
wxy zab\p
.ad
cde fgh\p
.ad r
ijk lmn\p
.na
opq rst\p
.ad
uvw xyz\p
.ad b
.ad 100
ABC DEF\p

It renders identically on Unix V7 nroff, DWB 3.3 nroff, Heirloom
Doctools nroff, and groff.  I confirmed this with "diff -u".

$ cat ATTIC/madness-groff.txt
abc   def
ghi jkl
mno pqr
stu vwx
yza   bcd
efg   hij
klm nop
 qrs tuv
wxy zab
 cde fgh
  ijk lmn
opq rst
  uvw xyz
ABC   DEF

> 2) still not fixed bug #55579 (patch in bug #59795).

Both of those bugs are closed as resolved--correctly, as far as I can
tell.

>   The patch in #59795 (comment #0) is simple enough to apply it with
>   an editor.

The patch in comment #0 does not resolve a documentation issue, but
changes the behavior of the formatter.  As it happens I've been working
up an alternative resolution of the same issue.

For evidence of this, see the recent expansion of the
adjustment/alignment test cases.[2]

See attachment.

Applying it gives the following results.

$ ./build/test-groff -ww -T utf8 ./ATTIC/madness.roff >| 
./ATTIC/madness-new-groff.txt
troff:./ATTIC/madness.roff:30: warning: out-of-range adjustment mode ignored: 
100
$ diff -u ./ATTIC/madness-groff.txt ./ATTIC/madness-new-groff.txt 
--- ./ATTIC/madness-groff.txt   2024-06-23 18:35:26.863861074 -0500
+++ ./ATTIC/madness-new-groff.txt       2024-06-23 18:43:08.034119343 -0500
@@ -6,10 +6,10 @@
 efg   hij
 klm nop
  qrs tuv
-wxy zab
- cde fgh
+ wxy zab
+cde   fgh
   ijk lmn
-opq rst
-  uvw xyz
+  opq rst
+uvw   xyz
 ABC   DEF
 
Going back to the input document, we can see that the following
scenario changes its behavior:

.ad c
qrs tuv\p
.na
wxy zab\p\" DIFFERENT
.ad
cde fgh\p\" DIFFERENT

Having established centered alignment, turning off adjustment
reactivates "left gravity", almost as if ".ad l" had been invoked.  And
then we when we "restore adjustment" with ".ad" (no argument),
we..._don't restore adjustment_, but rather reset alignment back to its
previous value.

A parallel situation arises with right-alignment (".ad r").  Just
substitute the word "right" for "centered" in the foregoing paragraph.

It seems to me that CSTR #54 did not cleanly separate the concepts of
text alignment and adjustment, and neither did the formatter itself.  I
would argue that "alignment" is where the "gravity" of your output line
is.  Glyphs accumulate toward the left, right, or center (without
changing their ordering or eliminating spaces).

_Adjustment_ is the widening of the spaces between words until glyphs
abut both the left and right margins.

When adjustment is enabled, it is therefore impossible to tell what
_alignment_ is in use.  Given the same indentation and line length,
words spread out the same.

Admittedly, this presentation flies in the face of 50 years of *roff
precedent and is surely due for voluble criticism from people who don't
actually use GNU troff or, if they do, never exhibit the usage patterns
illustrated in the diffs above.[3]

But I am curious to hear from people who don't fit that description.

Decisions still to be made:

1.  Whether to expose `.align` and `.adjust(ment)` registers to more
    clearly communicate the information encoded in `.j`.  The former
    would be string-valued, interpolating "l", "c", or "r", and the
    latter a Boolean.

2.  Whether to introduce new `align` and `adjust` requests to manipulate
    these two properties separately.  They still would not be usable in
    portable man pages.

3.  Whether to alter the behavior of the `ad` request, as the attached
    patch contemplates.  Doing so accommodates man page authors' DWIM
    intentions, at some risk to altering the formatting of documents
    that follow `.ad c` or `.ad r` with `.ad`, expecting to restore left
    alignment instead of merely disabling adjustment.  It's not clear to
    me how many such documents actually exist.  I don't think I've ever
    seen one that I didn't write as a test case.

4.  Whether to retain AT&T-compatible `ad` behavior in AT&T
    compatibility mode.  Deciding #3 likely decides this one.

I undertook this as an experiment, since I have become familiar enough
with the formatter's source to make the change without frustration.[4]

What do people think?

Regards,
Branden

[1] This implies that to Ossanna (and I guess other veteran *roff
    users), "adjustment" without further qualification meant only "left
    alignment, with no expansion of spaces".  This implied definition is
    consistent with CSTR #54 as written, but I find it a baffling one
    given that the `na` request, "no adjustment", MEANS THE SAME THING.
    Maybe someone has a less jarring interpretation.

[2] 
https://git.savannah.gnu.org/cgit/groff.git/commit/?id=f0b806df12e6f30669eab39cb38f11be74d9bd5d

[3] Perhaps this message is sufficiently long to escape the attention of
    one such person.

[4] Building and running "make check" with the attached patch fails one
    and only one test, the one seen in footnote 2 above.  As expected.
diff --git a/src/roff/troff/env.cpp b/src/roff/troff/env.cpp
index 7e315c70d..a7e9a6646 100644
--- a/src/roff/troff/env.cpp
+++ b/src/roff/troff/env.cpp
@@ -40,11 +40,10 @@ 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,
-  ADJUST_MAX = 5
+enum { ALIGN_LEFT = 0,
+  ALIGN_CENTER = 1,
+  ALIGN_RIGHT = 2,
+  ALIGN_MAX = 2
 };
 
 enum {
@@ -696,7 +695,8 @@ environment::environment(symbol nm)
   char_slant(0),
   space_size(12),
   sentence_space_size(12),
-  adjust_mode(ADJUST_BOTH),
+  alignment(ALIGN_LEFT),
+  adjust(true),
   fill(true),
   line_interrupted(false),
   prev_line_interrupted(0),
@@ -790,7 +790,8 @@ environment::environment(const environment *e)
   family(e->family),
   space_size(e->space_size),
   sentence_space_size(e->sentence_space_size),
-  adjust_mode(e->adjust_mode),
+  alignment(e->alignment),
+  adjust(e->adjust),
   fill(e->fill),
   line_interrupted(false),
   prev_line_interrupted(0),
@@ -872,7 +873,8 @@ void environment::copy(const environment *e)
   char_slant = e->char_slant;
   space_size = e->space_size;
   sentence_space_size = e->sentence_space_size;
-  adjust_mode = e->adjust_mode;
+  alignment = e->alignment;
+  adjust = e->adjust;
   fill = e->fill;
   line_interrupted = false;
   prev_line_interrupted = 0;
@@ -1040,7 +1042,7 @@ hunits environment::get_digit_width()
 
 unsigned environment::get_adjust_mode()
 {
-  return adjust_mode;
+  return (alignment << 1) + unsigned(adjust);
 }
 
 int environment::get_fill()
@@ -1998,7 +2000,7 @@ breakpoint *environment::choose_breakpoint()
 		  // exceed the maximum number of consecutive hyphenated
 		  // lines.
 		  || hyphen_line_count + 1 <= hyphen_line_max)
-	      && !(adjust_mode == ADJUST_BOTH
+	      && !(adjust
 		   // Don't choose the hyphenated breakpoint if the line
 		   // can be justified by adding no more than
 		   // hyphenation_space to any word space.
@@ -2017,7 +2019,7 @@ breakpoint *environment::choose_breakpoint()
 	  return bp;
 	}
 	else {
-	  if ((adjust_mode == ADJUST_BOTH
+	  if ((adjust
 	       ? hyphenation_space == H0
 	       : hyphenation_margin == H0)
 	      && (hyphen_line_max < 0
@@ -2205,27 +2207,27 @@ void environment::possibly_break_line(bool must_break_here,
     bp->nd->split(bp->index, &pre, &post);
     *ndp = post;
     hunits extra_space_width = H0;
-    switch (adjust_mode) {
-    case ADJUST_BOTH:
+    if (adjust) {
       if (bp->nspaces != 0)
 	extra_space_width = target_text_length - bp->width;
       else if (bp->width > 0 && target_text_length > 0
 	       && target_text_length > bp->width)
 	output_warning(WARN_BREAK, "cannot adjust line");
-      break;
-    case ADJUST_CENTER:
+    }
+    else {
+    switch (alignment) {
+    case ALIGN_CENTER:
       saved_indent += (target_text_length - bp->width)/2;
       was_centered = true;
       break;
-    case ADJUST_RIGHT:
+    case ALIGN_RIGHT:
       saved_indent += target_text_length - bp->width;
       break;
-    case ADJUST_LEFT:
-    case ADJUST_CENTER - 1:
-    case ADJUST_RIGHT - 1:
+    case ALIGN_LEFT:
       break;
     default:
-      assert(0 == "unhandled case of `adjust_mode`");
+      assert(0 == "unhandled case of `alignment`");
+    }
     }
     distribute_space(pre, bp->nspaces, extra_space_width);
     hunits output_width = bp->width + extra_space_width;
@@ -2491,12 +2493,12 @@ void environment::do_break(bool want_adjustment)
   input_line_start = H0;
   if (line != 0 /* nullptr */) {
     if (fill) {
-      switch (adjust_mode) {
-      case ADJUST_CENTER:
+      switch (alignment) {
+      case ALIGN_CENTER:
 	saved_indent += (target_text_length - width_total)/2;
 	was_centered = true;
 	break;
-      case ADJUST_RIGHT:
+      case ALIGN_RIGHT:
 	saved_indent += target_text_length - width_total;
 	break;
       }
@@ -2603,43 +2605,59 @@ void title()
   tok.next();
 }
 
-void adjust()
+void set_adjustment(bool 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 > ADJUST_MAX)
-	  warning(WARN_RANGE, "out-of-range adjustment mode ignored: "
-		  "%1", n);
-	else
-	  curenv->adjust_mode = n;
+  curenv->adjust = adjust;
+}
+
+void set_alignment(int alignment)
+{
+  curenv->alignment = alignment;
+}
+
+void adjust_or_align()
+{
+  if (!has_arg()) {
+    set_adjustment(true);
+    skip_line();
+    return;
+  }
+  switch (tok.ch()) {
+  case 'l':
+    set_adjustment(false);
+    set_alignment(ALIGN_LEFT);
+    break;
+  case 'r':
+    set_adjustment(false);
+    set_alignment(ALIGN_RIGHT);
+    break;
+  case 'c':
+    set_adjustment(false);
+    set_alignment(ALIGN_CENTER);
+    break;
+  case 'b':
+  case 'n':
+    set_adjustment(true);
+    break;
+  default:
+    int n;
+    if (get_integer(&n)) {
+      // See environment::get_adjust_mode().
+      if ((n < 0) || (n > ((1 << ALIGN_MAX) + 1)))
+	warning(WARN_RANGE, "out-of-range adjustment mode ignored: "
+		"%1", n);
+      else {
+	set_adjustment(bool(n & 1));
+	set_alignment(n >> 1);
       }
     }
   }
   skip_line();
 }
 
-void no_adjust()
+void no_adjustment()
 {
-  curenv->adjust_mode &= ~1;
+  set_adjustment(false);
   skip_line();
 }
 
@@ -3420,14 +3438,13 @@ void environment::print_env()
   errprint("  previous line interrupted/continued: %1\n",
 	   prev_line_interrupted ? "yes" : "no");
   errprint("  filling: %1\n", fill ? "on" : "off");
-  errprint("  alignment/adjustment: %1\n",
-	   adjust_mode == ADJUST_LEFT
+  errprint("  alignment: %1\n",
+	   alignment == ALIGN_LEFT
 	     ? "left"
-	     : adjust_mode == ADJUST_BOTH
-		 ? "both"
-		 : adjust_mode == ADJUST_CENTER
-		     ? "center"
-		     : "right");
+	     : alignment == ALIGN_CENTER
+	         ? "center"
+		 : "right");
+  errprint("  adjustment: %1\n", adjust ? "on" : "off");
   if (centered_line_count > 0)
     errprint("  lines remaining to center: %1\n", centered_line_count);
   if (right_aligned_line_count > 0)
@@ -4112,7 +4129,7 @@ const char *hyphenation_default_mode_reg::get_string()
 // init_hyphenation_pattern_requests() below for globally managed state.
 void init_env_requests()
 {
-  init_request("ad", adjust);
+  init_request("ad", adjust_or_align);
   init_request("br", break_without_adjustment);
   init_request("brp", break_with_adjustment);
   init_request("ce", center);
@@ -4141,7 +4158,7 @@ void init_env_requests()
   init_request("ls", line_spacing);
   init_request("lt", title_length);
   init_request("mc", margin_character);
-  init_request("na", no_adjust);
+  init_request("na", no_adjustment);
   init_request("nf", no_fill);
   init_request("nh", no_hyphenate);
   init_request("nm", number_lines);
diff --git a/src/roff/troff/env.h b/src/roff/troff/env.h
index 68b6a5e7b..4a1693970 100644
--- a/src/roff/troff/env.h
+++ b/src/roff/troff/env.h
@@ -105,8 +105,8 @@ void title_length();
 void space_size();
 void fill();
 void no_fill();
-void adjust();
-void no_adjust();
+void set_adjustment();
+void set_alignment();
 void center();
 void right_justify();
 void vertical_spacing();
@@ -157,7 +157,8 @@ class environment {
   font_family *family;
   int space_size;		// in 36ths of an em
   int sentence_space_size;	// same but for spaces at the end of sentences
-  int adjust_mode;
+  int alignment;
+  bool adjust;
   bool fill;
   bool line_interrupted;
   int prev_line_interrupted;	// three-valued Boolean :-|
@@ -376,8 +377,8 @@ public:
   friend void space_size();
   friend void fill();
   friend void no_fill();
-  friend void adjust();
-  friend void no_adjust();
+  friend void set_adjustment(bool);
+  friend void set_alignment(int);
   friend void center();
   friend void right_justify();
   friend void vertical_spacing();

Attachment: signature.asc
Description: PGP signature

Reply via email to