bugGNU roff - Bugs: bug #58962, Latin-1 NO-BREAK SPACE does not...

 
 

bug #58962: Latin-1 NO-BREAK SPACE does not behave as documented

Submitter:  Dave <barx>
Submitted:  Sat 15 Aug 2020 05:25:30 PM UTC
   
 
Category:  Core Severity:  3 - Normal
Item Group:  Incorrect behaviour 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

Sat 16 Apr 2022 09:30:57 PM UTC, comment #12: 

comment #7:

> The tmac files for the various input encodings remap the soft
> hyphen with a `tr` request.  I plan to take that stuff out.


For completeness, I note this has also been done:

commit 03eee8bfb62d63aa1a989ee01d5c49ca7fafb57b
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Thu Apr 14 18:52:21 2022 +1000

    [tmac]: Stop remapping input soft hyphen chars.

    * tmac/cp1047.tmac:
    * tmac/latin1.tmac:
    * tmac/latin2.tmac:
    * tmac/latin5.tmac:
    * tmac/latin9.tmac: Stop remapping input soft hyphen characters with
      `tr` requests in character encoding macro files.  The formatter does
      this for us now.


Dave <barx>
Group Member
Sat 16 Apr 2022 06:22:40 PM UTC, comment #11: 

Some history, if anyone is interested:

Commit b04d345e (which -- dating from September 1992 and reconstructed without granular change control -- encompasses hundreds of edits, so is not very useful as a specific reference point) added this text to groff_char(7): "The ISO Latin-1 no-break space (code 0240 octal) is equivalent to \(space)."

This claim survived (though the wording evolved) in groff_char(7) until 2003, when commit 48615a44 changed "the ISO Latin-1 no-break space is mapped to `\ ', the escaped space character" to "the ISO Latin-1 no-break space is mapped to `\~', the stretchable space character."  This commit made sweeping changes to groff_char(7), and also affected a couple other files, so nothing about this one particular change stands out.  But the commit included no concomitant code change that would alter this mapping, so Werner seemingly thought groff was already handling Latin-1 character 160 in this manner.

The claim remains in groff_char(7) to the present day, when Branden's just-referenced commit f9fb11f6 made it true.

The code changes are harder to trace, in part because, as comment #7 mentions, no code was specifically handling this case.  But that massive September 1992 commit does include a couple of ChangeLog entries of note (though again, with limited commit history from the time period, no way to map either of these to specific code changes).

Tue May  5 10:58:39 1992  James Clark  (jjc at jclark)

        * troff/input.c (init_charset_table): Translate 0240 to
        an unbreakable space.


Mon Jun  8 11:43:20 1992  James Clark  (jjc@jclark)

        * troff/input.c (init_charset_table): Don't translate 0240.
        * ps/tmac.ps: Translate char160 to space.
        * dvi/tmac.dvi: Likewise.
        * tty/tmac.tty: Likewise.

So it seems the Latin-1 no-break space was briefly handled in C++ code before being hustled off to tmac files.  Today it comes full circle and makes it triumphant return to input.cpp.

Dave <barx>
Group Member
Sat 16 Apr 2022 09:06:17 AM UTC, comment #10: 


commit f9fb11f6625fe859d3f22c5fc6cbd51e19b64ec6
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Thu Apr 14 12:49:27 2022 +1000

    [troff]: Translate 8-bit NBSP and SHY on input.

    * src/roff/troff/input.h: Define constant integers for "input" no-break
      spaces and soft hyphens for EBCDIC and non-EBCDIC (presumably
      ASCII/ISO 8859/Unicode) systems.

    * src/roff/troff/input.cpp (token::next): Translate the input character
      codes for input NBSP to \~ and nput SHY to \%.

    Fixes <https://savannah.gnu.org/bugs/?58962>.  Thanks to Dave Kemper for
    the report, code review, and his suggestion to push more work to compile
    time.


G. Branden Robinson <gbranden>
Group administrator
Thu 14 Apr 2022 07:36:03 AM UTC, comment #9: 


comment #8:

> So the patch looks good as-is.  One refinement that could maybe be made: instead of an explicit test for input encoding, maybe define new constants in src/roff/troff/input.h (which already has separate EBCDIC vs non-EBCDIC blocks) to compare against.  The rest of input.cpp seems to be encoding-agnostic by using these constants.  This also eliminates a run-time conditional, putting the decision on the compiler.


That's a good idea.  By defining constant ints in input.h, I can reduce the change to input.cpp to just adding 2 more cases to the big switch().

Thanks, Dave!

G. Branden Robinson <gbranden>
Group administrator
Thu 14 Apr 2022 04:46:44 AM UTC, comment #8: 

comment #7:

> I didn't because I fear encoding mangling.


Good point.  I just downloaded it, and it came through as a Latin-1-encoded file identical to the original, but that's with one browser on one OS (both the ones I used to upload it in the first place, so I'm kinda stacking the deck).

> Try unpatched troff with the -R flag to shut off the loading
> of troffrc.


Ah, yes, with that I see the same results you pasted at the end of your comment.

> I think what's happening here is that it's being handled
> not exactly as a space, but as an undefined glyph for which
> no information is available.  I haven't chased it down, but
> I'm guessing that when that happens you get an unbreakable
> space of 'spacewidth' size in the current font.


Aha, this explains why when I went looking for the code that did that mapping a while back, to see if I could create a simple patch that would resolve this bug, I was never able to find it.

So the patch looks good as-is.  One refinement that could maybe be made: instead of an explicit test for input encoding, maybe define new constants in src/roff/troff/input.h (which already has separate EBCDIC vs non-EBCDIC blocks) to compare against.  The rest of input.cpp seems to be encoding-agnostic by using these constants.  This also eliminates a run-time conditional, putting the decision on the compiler.

Dave <barx>
Group Member
Thu 14 Apr 2022 03:36:23 AM UTC, comment #7: 

Hi, Dave!

comment #6:

> comment #5:
> > I believe I've cracked this.
>
> Great news!
>
> > $ xxd EXPERIMENTS/dave-58962.roff
>
> I've also attached this file so anyone else who wants to run it doesn't have to reconstruct it from hex.


I didn't because I fear encoding mangling.
 

> > It seems like these cases just weren't ever dealt with in
> > the formatter's input parser.
>
> When I run unpatched groff (even as far back as groff 1.19.2), I get the second line of stderr output, but not the first.  So 0xAD was being handled somewhere, though seemingly not in token::next().


Try unpatched troff with the -R flag to shut off the loading of troffrc.

This is why I said "the formatter's input parser".

The tmac files for the various input encodings remap the soft hyphen with a `tr` request.  I plan to take that stuff out.
 

> And while an input 0xA0 didn't match anything according to the output-comparison operator, it did get interpreted as a fixed-width nonbreaking space ("\ " to groff) in the output, so it too was being handled, just not in the way one might have hoped (or, more the the point, not in the way that was documented).


I think what's happening here is that it's being handled not exactly as a space, but as an undefined glyph for which no information is available.  I haven't chased it down, but I'm guessing that when that happens you get an unbreakable space of 'spacewidth' size in the current font.

> That is, while the patch as written solves the reported problems, since it does so only by adding new logic, it would also seem to leave in place some now-dead code where these two characters were previously handled.  Whether this is of any practical concern, I leave to you to determine.


Right now, I think the extra handling of SHY by the input encoding macro files is superfluous and should go.

For unencoded characters, there's probably not much to do beyond what is already done.  Warn, advance the drawing the position, carry on, and don't match (in the output comparison operator) something that is defined.


$ troff -v
GNU troff (groff) version 1.22.4
$ troff -R ./EXPERIMENTS/dave-58962.roff
troff: ./EXPERIMENTS/dave-58962.roff:1: warning: can't find character with input code 160
troff: ./EXPERIMENTS/dave-58962.roff:2: warning: can't find character with input code 173


G. Branden Robinson <gbranden>
Group administrator
Thu 14 Apr 2022 03:09:22 AM UTC, comment #6: 

comment #5:

> I believe I've cracked this.


Great news!

> $ xxd EXPERIMENTS/dave-58962.roff


I've also attached this file so anyone else who wants to run it doesn't have to reconstruct it from hex.

> It seems like these cases just weren't ever dealt with in
> the formatter's input parser.


When I run unpatched groff (even as far back as groff 1.19.2), I get the second line of stderr output, but not the first.  So 0xAD was being handled somewhere, though seemingly not in token::next().

And while an input 0xA0 didn't match anything according to the output-comparison operator, it did get interpreted as a fixed-width nonbreaking space ("\ " to groff) in the output, so it too was being handled, just not in the way one might have hoped (or, more the the point, not in the way that was documented).

That is, while the patch as written solves the reported problems, since it does so only by adding new logic, it would also seem to leave in place some now-dead code where these two characters were previously handled.  Whether this is of any practical concern, I leave to you to determine.

(file #53088)

Dave <barx>
Group Member
Thu 14 Apr 2022 01:42:31 AM UTC, comment #5: 

Hi Dave,

I believe I've cracked this.


$ xxd EXPERIMENTS/dave-58962.roff
00000000: 2e69 6620 27a0 275c 7e27 202e 746d 2069  .if '.'\~' .tm i
00000010: 6e70 7574 2030 7841 3020 6d61 7463 6865  nput 0xA0 matche
00000020: 7320 5c5c 7e0a 2e69 6620 27ad 275c 2527  s \\~..if '.'\%'
00000030: 202e 746d 2069 6e70 7574 2030 7841 4420   .tm input 0xAD
00000040: 6d61 7463 6865 7320 5c5c 250a            matches \\%.
$ ./build/troff -F ./build/font -F ./font -M ./build/tmac -M ./tmac ./EXPERIMENTS/dave-58962.roff
input 0xA0 matches \~
input 0xAD matches \%
$ ./build/troff -F ./build/font -F ./font -T utf8 -M ./build/tmac -M ./tmac ./EXPERIMENTS/dave-58962.roff
input 0xA0 matches \~
input 0xAD matches \%


(Not like the output device should really matter.)

It seems like these cases just weren't ever dealt with in the formatter's input parser.  Maybe there was some dithering because the input encoding could be either ISO or EBCDIC.

Here's the patch.


$ git diff
diff --git a/src/roff/troff/input.cpp b/src/roff/troff/input.cpp
index 36822033a..015c17a87 100644
--- a/src/roff/troff/input.cpp
+++ b/src/roff/troff/input.cpp
@@ -1,4 +1,4 @@
-/* Copyright (C) 1989-2020 Free Software Foundation, Inc.
+/* Copyright (C) 1989-2022 Free Software Foundation, Inc.
      Written by James Clark (jjc@jclark.com)

 This file is part of groff.
@@ -1743,6 +1743,29 @@ void token::next()
     int cc = input_stack::get(&n);
     if (cc != escape_char || escape_char == 0) {
     handle_normal_char:
+      // Handle no-break space and soft hyphen.
+      if (0x41 == 'A') { // ASCII/ISO 8859/Unicode
+       if (0xA0 == cc) {
+         type = TOKEN_STRETCHABLE_SPACE;
+         return;
+       }
+       else if (0xAD == cc) {
+         type = TOKEN_HYPHEN_INDICATOR;
+         return;
+       }
+      }
+      else if (0xC1 == 'A') { // code page 1047 (EBCDIC)
+       if (0x41 == cc) {
+         type = TOKEN_STRETCHABLE_SPACE;
+         return;
+       }
+       else if (0xCA == cc) {
+         type = TOKEN_HYPHEN_INDICATOR;
+         return;
+       }
+      }
+      else
+       fatal("unrecognized input character encoding");
       switch(cc) {
       case PUSH_GROFF_MODE:
        input_stack::save_compatible_flag(compatible_flag);


G. Branden Robinson <gbranden>
Group administrator
Wed 13 Apr 2022 11:02:05 PM UTC, comment #4: 

troff's mapping of U+00A0 to "\ " has become even more aberrant with the fix for bug #62300, which makes preconv turn U+00A0 into \~.

Dave <barx>
Group Member
Tue 05 Oct 2021 03:33:44 AM UTC, comment #3: 

original submission:

> Either way, if Latin-1 A0 behaves the same as one of "\ " or
> "\~", the output-equivalency conditional operator (rendered
> as 'XXX'YYY' in the info manual, though a host of characters
> besides single quotes can be used) ought to recognize this.


What this operator actually tests is not straightforward, as bug #60836 discusses.  That is, input that produces identical output does not always return true under this operator.

I don't fully grok how this operator works, so I can't speak with authority about it.  But I think for the case outlined here, the operator should consider the results identical.  This is an instance of merely representing the same character (the no-break space) in different ways in the input; the edge cases 60836 talks about have to do with equivalent cursor motions being specified in different ways.

Dave <barx>
Group Member
Wed 25 Aug 2021 07:59:12 AM UTC, comment #2: 

original submission:

> I believe the documented behavior is more sensible than the
> actual behavior.  But that's a judgment call and open to debate.


The Texinfo manual is increasingly subscribing to this view as well.

Commit d19a3b98 (from a week before this bug was filed) says "\ " is "not optimal in most situations."

Last week's commit b20ff768 reinforced this, saying "Usually you want one of the following escapes instead" of "\ ", and further clarifying, "often '\~'."

Dave <barx>
Group Member
Sat 05 Jun 2021 10:59:12 AM UTC, comment #1: 

original submission:

> groff(7) is less clear, saying that the test 's1's2' is "True if string s1
> is identical to string s2," which implies it's comparing input strings.


I see that commit 356bc65d has remedied this.

The prior unclear wording seems to originate in setion 16 of CSTR #54.

Dave <barx>
Group Member
Sat 15 Aug 2020 05:25:30 PM UTC, original submission:  

(Another bug report spawned from the discovery process of bug #58930.)

Quoth groff_char(7): "the ISO latin1 no-break space is mapped to `\~', the stretchable space character."

An eminently sensible mapping.  Oh, if only it were so.

In fact, the Latin-1 no-break space (character 160 decimal, A0 hex):

  • behaves the same as "\ ", the nonstretchable nonbreaking space character
  • matches neither "\ " nor "\~" in an output-equivalency conditional


Examining these in detail:

Behavior


Consider an input file with one instance of the string "<>", representing a nonbreaking space.  sed can convert this string to the various types of nonbreaking space under consideration (the two escapes and the raw Latin-1 character), and the typeset results compared by seeing which ones produce identical PostScript output.


$ cat t0
Lorem ipsum dolor sit amet, consectetur<>adipiscing elit, sed
do eiusmod tempor incididunt ut labore et dolore magna aliqua.
$ # Baseline test, for escapes expected to be different:
$ diff <(sed 's/<>/\\ /' t0 | groff) <(sed 's/<>/\\~/' t0 | groff) | wc
      8      68     403
$ # Output expected to be the same based on what the docs say:
$ diff <(sed 's/<>/\\~/' t0 | groff) <(sed 's/<>/\xA0/' t0 | groff) | wc
      8      68     403
$ # Output that turns out to be the same:
$ diff <(sed 's/<>/\\ /' t0 | groff) <(sed 's/<>/\xA0/' t0 | groff) | wc
      0       0       0
$


I'm filing this as "Incorrect behavio[u]r" rather than "Documentation" because I believe the documented behavior is more sensible than the actual behavior.  But that's a judgment call and open to debate.

Equivalency conditional


Either way, if Latin-1 A0 behaves the same as one of "\ " or "\~", the output-equivalency conditional operator (rendered as 'XXX'YYY' in the info manual, though a host of characters besides single quotes can be used) ought to recognize this.  But this operator claims the output of character A0 is equivalent to neither one (first observed in comment #2 of the aforementioned bug).


$ printf ".if '\xA0'\~' .tm equal\n" | groff
$ printf ".if '\xA0'\ ' .tm equal\n" | groff
$


(Granted, the documentation muddies what this operator is actually testing.  The info manual is clear about 'XXX'YYY', saying this is "True if the output produced by XXX is equal to the output produced by YYY."  But groff(7) is less clear, saying that the test 's1's2' is "True if string s1 is identical to string s2," which implies it's comparing input strings.  Were that the case, you'd expect both the above tests to be false... but you'd also expect '\[em]'\[u2014]' to be false, which it isn't.)

Dave <barx>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #53088:  dave-58962.roff added by barx (76B - text/troff - test file from comment #5)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by barx (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-04-16 gbranden Planned ReleaseNone 1.23.0
    2022-04-16 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2022-04-14 barx Attached File- Added dave-58962.roff, #53088
    2022-04-14 gbranden StatusNone In Progress
        Assigned toNone gbranden
    2021-05-14 barx CategoryNone Core

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code