bugGNU roff - Bugs: bug #61348, [troff] assertion in...

 
 

bug #61348: [troff] assertion in env.cpp:distribute_space() has to come out

Submitter:  G. Branden Robinson <gbranden>
Submitted:  Mon 18 Oct 2021 10:53:08 AM 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
   

Wed 20 Oct 2021 05:08:03 AM UTC, comment #3: 


comment #1:

>   This happens when the selected adjustment is NOT just to the left
> margin.
>
> I have changed the default adjustment to the left one in my code.


>  void environment::set_family(symbol fam)
> @@ -646,7 +646,7 @@ environment::environment(symbol nm)
>    char_slant(0),
>    space_size(12),
>    sentence_space_size(12),
> -  adjust_mode(ADJUST_BOTH),
> +  adjust_mode(ADJUST_LEFT),
>    fill(1),
>    interrupted(0),
>    prev_line_interrupted(0),


> I have seen to many warnings about "cannot adjust line".


> @@ -2122,7 +2116,7 @@ void environment::possibly_break_line(in
>          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");
> +        output_warning(WARN_BREAK, "cannot adjust line at both margins");
>        break;
>      case ADJUST_CENTER:
>        saved_indent += (target_text_length - bp->width)/2;


Hi Bjarni,

I'm afraid I don't agree with many of your claims.

I have written at length before about the difference between "alignment" and "adjustment", in spite of the unfortunate history of AT&T troff's conflation of these ideas.

So it is true that the `distribute_space()` function should only be called when an output line is being adjusted.

You are at liberty to alter GNU roff to left-align output by default; please keep your local customizations in mind when you file defect reports.  Also I would note that a much less intrusive way to get the same result is available: you can simply put '.ad l' or '.na' in the `troffrc` file instead of changing the way an object is initialized in the C++ source.

I don't think either of your patches are suitable for inclusion in GNU roff (although they're fine for your personal use--apart from potentially getting out of sync with documentation I don't see any problems with them for a private build).  GNU roff's default adjustment mode is what it is for continuity with the 45+ years of history of AT&T troff and its descendants.  Regarding the second patch, I find it redundant if we define the word "adjustment" scrupulously; I have tried to move our documentation in that direction and if you spot any exceptions I would appreciate you pointing them out to me.

> Adjustment to both margins does not make sense with nroff,


It's made sense for longer than many groff users have been alive.
Some people might not care for it, which is why we try to give them ways to express alternative preferences reliably, like the AD string in our man(7).

>  and should therefore NOT be the default for nroff and man pages,
>  for example by using ".ds AD l" and ".ad l" in tmac files.


I simply don't agree, except perhaps for new macro packages that might get written; they're free to choose different defaults because they have no legacy of expectations to meet.

> Especially for a long single word, like web addresses (URL).


This is indeed an ergonomic problem sometimes, and I'm mulling over possible remedies for it--at least if you're thinking of the same diagnostic messages I am.

-verbatim-
troff: backtrace: '.../groff/build/../tmac/an-ext.tmac':107: macro 'link-text-div'
troff: backtrace: '.../groff/build/../tmac/an-ext.tmac':126: macro 'UE'
troff: backtrace: file './gpinyin.1':294
troff:./gpinyin.1:294: warning [p 1, 13.3i]: cannot adjust line
-verbatim-

G. Branden Robinson <gbranden>
Group administrator
Wed 20 Oct 2021 04:54:02 AM UTC, comment #2: 


commit 45293d589293cf4d938aebdd4812926956ec36ea
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Tue Oct 19 13:30:29 2021 +1100

    Fix Savannah #61348.

    * src/roff/troff/env.cpp (distribute_space): Revert an `assert()` I
      added in commit b93eacd8d7 (5 September); we can indeed reach this
      code with a negative amount of desired space, and in fact the "show
      hyphenation points" trick
      <https://froude.eu/groff/examples/hyphenation-points.html> relies upon
      it.  Add explanatory comment.

    Fixes <https://savannah.gnu.org/bugs/?61348>.


G. Branden Robinson <gbranden>
Group administrator
Mon 18 Oct 2021 05:56:24 PM UTC, comment #1: 

  This happens when the selected adjustment is NOT just to the left
margin.

I have changed the default adjustment to the left one in my code.

 void environment::set_family(symbol fam)
@@ -646,7 +646,7 @@ environment::environment(symbol nm)
   char_slant(0),
   space_size(12),
   sentence_space_size(12),
-  adjust_mode(ADJUST_BOTH),
+  adjust_mode(ADJUST_LEFT),
   fill(1),
   interrupted(0),
   prev_line_interrupted(0),

I have seen to many warnings about "cannot adjust line".

@@ -2122,7 +2116,7 @@ void environment::possibly_break_line(in
         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");
+        output_warning(WARN_BREAK, "cannot adjust line at both margins");
       break;
     case ADJUST_CENTER:
       saved_indent += (target_text_length - bp->width)/2;

Adjustment to both margins does not make sense with nroff,
 and should therefore NOT be the default for nroff and man pages,
 for example by using ".ds AD l" and ".ad l" in tmac files.

Especially for a long single word, like web addresses (URL).

Bjarni Ingi Gislason <bjarniig>
Mon 18 Oct 2021 10:53:08 AM UTC, original submission:  

I added an `assert()` to `distribute_space()` last month in commit b93eacd8d7, but it as with Gumby brain surgery, it has to come out.

It breaks the hyphenation point detector that we've had kicking around on the groff list for a few years (at least).

The reason is that the amount of desired space can be negative if the line is overrunning.


.hy 4
.ll 1u
troff: backtrace: file '<standard input>':2
troff: <standard input>:2: warning: invalid line length 0u rounded to device horizontal resolution
struct\~sockaddr_mctp
troff: backtrace: file '<standard input>':3
troff: <standard input>:3: warning [p 1, 0.0i]: can't break line
troff: backtrace: file '<standard input>':3
troff: <standard input>:3: debug: GBR: desired_space is -264
troff: backtrace: file '<standard input>':3
troff: <standard input>:3: debug: GBR: nspaces is 1
troff: ../src/roff/troff/env.cpp:2065: distribute_space(): assertion failed: 'desired_space > 0'
/home/branden/src/GIT/groff/build/groff: error: troff: Aborted (core dumped)


G. Branden Robinson <gbranden>
Group administrator

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Date Changed by Updated Field Previous Value => Replaced by
    2021-10-20 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
        Planned ReleaseNone 1.23.0

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code