(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();
signature.asc
Description: PGP signature