bugGNU roff - Bugs: bug #57416, [PATCH] repair .AT and .UC in the...

 
 

bug #57416: [PATCH] repair .AT and .UC in the groff_man(7) macros

Submitter:  Ingo Schwarze <schwarze>
Submitted:  Sun 15 Dec 2019 05:04:41 PM UTC
   
 
Category:  Macro man Severity:  2 - Minor
Item Group:  Incorrect behaviour Status:  Fixed
Privacy:  Public Assigned to:  schwarze
Open/Closed:  Closed Planned Release:  1.23.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 05 Feb 2023 07:11:38 AM UTC, comment #7: 


commit a628605e2b26106a6e1e5afbf8cc390283099fa8
Author:     Ingo Schwarze <schwarze@openbsd.org>
AuthorDate: Thu Jan 16 13:59:48 2020 +0100
Commit:     Ingo Schwarze <schwarze@openbsd.org>
CommitDate: Thu Jan 16 14:19:17 2020 +0100

    Repair .AT and .UC in the groff_man(7) macros.

    * tmac/an-old.tmac:
    Setting user-defined strings in a macro that will later be called
    indirectly from page location traps is excessively complicated.
    Besides, the implementation doesn't work: when the trap is finally
    sprung, the defaults from the an-init macro clobber what the author
    specified with .AT or .UC.
    Instead, all that is needed is setting the strings for the header
    before triggering the page break, such that they appear right
    away, while setting the strings for the footer after the page
    break, such that they don't appear on the previous page.

    This bug was found by Jonathan Gray <jsg@openbsd.org>
    while he looked at 4.xBSD manual pages.

    Thanks to gbranden@ for finding a bug in first version of my patch
    and for agreeing with the idea, see: https://savannah.gnu.org/bugs/?57416


I'm not sure the "patch" annotation should be retained, but I'm leaving it for now.  I don't think any official semantics are defined anywhere, but the denotation I've drifted toward over the past five years are that a ticket get the "[PATCH]" annotation initially when a patch is attached to the ticket, either at creation or later, from someone who doesn't have (or elects not to use their) commit rights to the groff Git repository.  The annotation is retained if the patch is accepted by the developers, and removed if it is not.

If people agree, I guess the above should be written down somewhere more discoverable.

G. Branden Robinson <gbranden>
Group administrator
Thu 16 Jan 2020 02:52:19 PM UTC, comment #6: 

Er, rather, fixed in a628605e2b26106a6e1e5afbf8cc390283099fa8.

Regression test added in 5ee5bb7f1222a9c53c434984d7a755d61569dcd0.

G. Branden Robinson <gbranden>
Group administrator
Thu 16 Jan 2020 02:51:18 PM UTC, comment #5: 

Fixed in 5ee5bb7f1222a9c53c434984d7a755d61569dcd0.

G. Branden Robinson <gbranden>
Group administrator
Thu 16 Jan 2020 01:48:03 PM UTC, comment #4: 

Your explanations make perfect sense.

I find myself wanting a "continuous-numbering, continuous-rendering" mode that doesn't suppress the page footers now.

I suppose that is a wishlist item.

And yes, the output generation part of my regression test is working fine with your updated patch.  When I crank back to "HEAD^", the .AT page wrongly gets the user-specified .TH field, but the .UC page is not incorrect.  However, I will check to see if that is a matter of ordering.

I'll finish my reg test, push it when I'm happy, and then close the ticket.

Thanks!

G. Branden Robinson <gbranden>
Group administrator
Thu 16 Jan 2020 01:40:23 PM UTC, comment #3: 

Hi Branden,

thanks for checking!

The warning message was due to the four backslashes in "\\\\n[CT]".  Before the patch, that code was inside two nested macro definitions, so they all got expanded.  After the patch, it is only inside a single macro definition.  So i had to reduce the number of backslashes from four to two, which i did before pushing the patch.

Given that you agreed to the idea, i pushed it rather than starting a game of patch ping pong, hoping that will make life and testing easier for everyone.

Regarding the different behaviour in -T ps/pdf and -T utf8/ascii mode, note that the former macro .an-init was called at different  times depending on the cR register, which is on by default for nroff but off by default for troff.  So it doesn't surprise me that a bug related to it only affected some output devices.

Feel free to set this ticket to "closed" once you are satisfied that what we have in git is now good.  Thanks!

Ingo Schwarze <schwarze>
Group Member
Thu 16 Jan 2020 10:35:06 AM UTC, comment #2: 

I can confirm the failure as Ingo describes on the `ps` device.

The really interesting bit is that the failure does not manifest on the `utf8` device.

G. Branden Robinson <gbranden>
Group administrator
Thu 16 Jan 2020 10:30:47 AM UTC, comment #1: 

I'm writing a regression test for this, and, well, there's something wrong...


$ ./test-groff -Tutf8 -man src/devices/grops/grops.1
/home/branden/src/GIT/groff/build/../tmac/an-old.tmac:195: backtrace: string 'TH'
/home/branden/src/GIT/groff/build/../tmac/andoc.tmac:79: backtrace: macro 'TH'
troff: src/devices/grops/grops.1:1: warning: missing closing delimiter


Still scratching my chin over it (just started looking today, though).

I'm certainly a +1 on the idea here.  I'm just a greedy bastard and I want my regression test.

Please communicate my thanks to Jonathan Gray for finding this problem.

G. Branden Robinson <gbranden>
Group administrator
Sun 15 Dec 2019 05:04:41 PM UTC, original submission:  

The following file does not work as expected:

.TH FOO 1
.UC 4
.SH NAME
foo

In -Tps and -Tpdf output mode, it does not print the expected string "4th Berkeley Distribution" in the page footer because the (empty) default string from the an-init macro overrides what the author defines with .UC.

The comment

  \" This approach gets multiple man pages right.

is a lie: it doesn't even work for a single page.
Besides, even when a file contains multiple pages, all that is needed is setting the various strings on the right sides of the page breaks: those strings used in the page header before, those used in the page footer after the page break.

The attached patch works for single and multiple pages with and without .AT or .UC; OK to push it?

Ingo Schwarze <schwarze>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #48066:  man-UC.patch added by schwarze (4KiB - 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 gbranden (Posted a comment)
  • -email is unavailable- added by schwarze (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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-01-16 gbranden StatusConfirmed Fixed
        Open/ClosedOpen Closed
        Planned ReleaseNone 1.23.0
    2020-01-16 gbranden StatusFixed Confirmed
        Planned Release1.23.0 None
    2020-01-16 schwarze StatusConfirmed Fixed
        Planned ReleaseNone 1.23.0
    2020-01-16 gbranden StatusNone Confirmed
    2019-12-15 schwarze Attached File- Added man-UC.patch, #48066

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code