bugGNU roff - Bugs: bug #55109, [ms] add the name of the calling...

 
 

bug #55109: [ms] add the name of the calling macro to a diagnostic

Submitter:  Bjarni Ingi Gislason <bjarniig>
Submitted:  Tue 27 Nov 2018 01:27:33 AM UTC
   
 
Category:  Macro ms Severity:  1 - Wish
Item Group:  Warning/Suspicious behaviour Status:  Rejected
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 30 Mar 2023 05:11:21 PM UTC, comment #2: 

Dropping patch annotation from rejected ticket.

G. Branden Robinson <gbranden>
Group administrator
Sat 17 Oct 2020 10:48:42 PM UTC, comment #1: 

I think this patch has its heart in the right place but doesn't do quite the right thing.

See bug #59290 for a more philosophical discussion of what I think we want instead.

I'm attaching my proposed alternative, which is lower-impact in terms of line count and the spread of impact through the file.

Here's an example of the output it produces.


$ printf '.TL\n' | ./build/test-groff -Tutf8 -z -ms
s.tmac: error: diversion open while ejecting page (last file seen: <standard input>) (recovering)
$ printf '.TL\n' > /tmp/empty-title.ms && ./build/test-groff -Tutf8 -z -ms /tmp/empty-title.ms
s.tmac: error: diversion open while ejecting page (last file seen: /tmp/empty-title.ms) (recovering)
$ printf '.DS\n.DS\n.DE\n' > /tmp/unbalanced-diversion.ms && ./build/test-groff -Tutf8 -z -ms /tmp/unbalanced-diversion.ms
s.tmac: error: diversion open while ejecting page (last file seen: /tmp/unbalanced-diversion.ms) (recovering)
$ cat /tmp/two-TL.ms
.TL
This Is My Title
.TL
There Are Many Like It
.PP
But this one is mine.
$ ./build/test-groff -Tutf8 -z -ms /tmp/two-TL.ms
s.tmac:/tmp/two-TL.ms:3: error: TL is not allowed more than once


We can see particularly from the final example above that the name of the macro emitting the diagnostic is not what is of crucial importance; a diagnostic message that accurately identifies the problem, accompanied by the file name and location within, is what will help the user to remedy the problem.

As it happens, the GNU Coding Standards has excellent advice in this domain.  https://www.gnu.org/prep/standards/html_node/Errors.html

Linus Torvalds once said that the foregoing document should be set on fire and the ashes urinated upon, a comment oft repeated by his many fans.  I am sure the quality of the diagnostic messages those fans write speaks for itself.  Or would, if such messages existed at all.  But in their ethos, Ken Thompson's infamous '?' (from AT&T "ed") is one character of user communication too many.

Per code review on the submitted patch, I regard the many scattered identical changes of "@error:" to "@error: \\$0" and similar to be a code smell.  If that's really what you wanted, the better thing to do would, I suspect, be to interpolate @error as a string, which would preserve $0 and then @error could interpolate that itself.  See the "Strings" node in the groff Texinfo manual.  "Interpolating a string does not hide existing macro arguments."

Closing as "won't fix", but not because the patch wasn't well-intentioned.

(file #50005)

G. Branden Robinson <gbranden>
Group administrator
Tue 27 Nov 2018 01:27:33 AM UTC, original submission:  


From c4627d1f7602d2413f5bed3cd8bf99a34692e06d Mon Sep 17 00:00:00 2001
From: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
Date: Tue, 27 Nov 2018 01:17:30 +0000
Subject: [PATCH] s.tmac: Add the name of the calling macro to a diagnostic

  See groff bug #52463.

Signed-off-by: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
---
 tmac/s.tmac | 64 ++++++++++++++++++++++++++---------------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/tmac/s.tmac b/tmac/s.tmac
index 04265537..6dda23b7 100644
--- a/tmac/s.tmac
+++ b/tmac/s.tmac
@@ -89,15 +89,15 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 ..
 .\" print an error message and then try to recover
 .de @error-recover
-.@error \\$@ (recovering)
+.@error \\$0: \\$@ (recovering)
 .nr *pop-count 0
 .while !'\\n(.z'' \{\
-.        \"@warning automatically terminating diversion \\n(.z
+.        \"@warning \\$0: automatically terminating diversion \\n(.z
 .        ie d @div-end!\\n(.z .@div-end!\\n(.z
 .        el .*div-end-default
 .        nr *pop-count +1
 .        \" ensure that we don't loop forever
-.        if \\n[*pop-count]>20 .@fatal recovery failed
+.        if \\n[*pop-count]>20 .@fatal \\$0: recovery failed
 .\}
 .while !'\\n[.ev]'0' .ev
 .par@reset-env
@@ -137,7 +137,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .ch cov*first-page-init
 .rm cov*first-page-init
 .if !'\\n[.ev]'0' \{\
-.        @error must be in top-level environment, not '\\n[.ev]',
+.        @error \\$0: must be in top-level environment, not '\\n[.ev]',
 .        @error when first page is started
 .\}
 .par@init
@@ -207,7 +207,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .        br
 .        di
 .\}
-.ie !\\n[cov*n-au] .@error AI before AU
+.ie !\\n[cov*n-au] .@error \\$0: AI before AU
 .el \{\
 .        di cov*ai-div!\\n[cov*n-au]
 .        nf
@@ -314,7 +314,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .        par@reset
 .        cov*print
 .  \}
-.  el .@error AE without AB
+.  el .@error \\$0: AE without AB
 .\}
 ..
 .de @div-end!cov*ab-div
@@ -328,7 +328,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .\}
 .el \{\
 .        if \\n[cov*rp-format] \{\
-.                @warning RP format but no TL
+.                @warning \\$0: RP format but no TL
 .                bp 1
 .                als FS @FS
 .                als FE @FE
@@ -368,7 +368,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .CHECK-FOOTER-AND-KEEP
 .\" If anything was printed below where the footer line is normally
 .\" printed, then that's an overflow.
-.if -\\n[FM]/2+1v+\\n[cov*page-length]<\\n[nl] .@error cover sheet overflow
+.if -\\n[FM]/2+1v+\\n[cov*page-length]<\\n[nl] .@error \\$0: cover sheet overflow
 .pl \\n[cov*page-length]u
 .bp 1
 .if !\\n[cov*rp-no] .cov*tl-au-print
@@ -415,7 +415,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .\" start of footnote on cover
 .de cov*FS
 .if \\n[cov*in-fn] \{\
-.        @error nested FS
+.        @error \\$0: nested FS
 .        FE
 .\}
 .nr cov*in-fn 1
@@ -438,7 +438,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .        nr cov*in-fn 0
 .        nr cov*fn-height +\\n[dn]
 .\}
-.el .@error FE without matching FS
+.el .@error \\$0: FE without matching FS
 ..
 .\" ***************************
 .\" ******** module pg ********
@@ -619,7 +619,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .\" flush out any keeps or footnotes
 .de pg@super-eject
 .br
-.if !'\\n(.z'' .@error-recover diversion open while ejecting page
+.if !'\\n(.z'' .@error-recover \\$0: diversion open while ejecting page
 .\" Make sure we stay in the end macro while there is still footnote
 .\" overflow left, or floating keeps.
 .while \\n[kp@tail]>\\n[kp@head]:\\n[pg@fn-flag] \{\
@@ -659,7 +659,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 ..
 .\" Second argument of 'no' means don't embellish the first argument.
 .de fn*do-FS
-.if \\n[fn*open] .@error-recover nested FS
+.if \\n[fn*open] .@error-recover \\$0: nested FS
 .nr fn*open 1
 .if \\n[.u] \{\
 .        \" Ensure that the first line of the footnote is on the same page
@@ -679,7 +679,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .el .@LP
 ..
 .de @FE
-.ie !\\n[fn*open] .@error FE without FS
+.ie !\\n[fn*open] .@error \\$0: FE without FS
 .el \{\
 .        nr fn*open 0
 .        br
@@ -771,7 +771,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .        \}
 .        el \{\
 .                ev fn
-.                @error-recover unclosed diversion within footnote
+.                @error-recover \\$0: unclosed diversion within footnote
 .        \}
 .\}
 ..
@@ -816,7 +816,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .di kp*div
 ..
 .de KF
-.if !'\\n(.z'' .@error-recover KF while open diversion
+.if !'\\n(.z'' .@error-recover \\$0: KF while open diversion
 .di kp*fdiv
 .ev k
 .par@reset-env
@@ -826,7 +826,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .ie '\\n(.z'kp*div' .kp*end
 .el \{\
 .        ie '\\n(.z'kp*fdiv' .kp*fend
-.        el .@error KE without KS or KF
+.        el .@error \\$0: KE without KS or KF
 .\}
 ..
 .de @div-end!kp*div
@@ -911,7 +911,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 ..
 .de ds@auto-end
 .if \\n[\\n[.ev]:ds-type] \{\
-.        @error automatically terminating display
+.        @error \\$0: automatically terminating display
 .        DE
 .\}
 ..
@@ -920,7 +920,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .el .ds*end!2
 ..
 .de ds*end!0
-.@error DE without DS, ID, CD, LD or BD
+.@error \\$0: DE without DS, ID, CD, LD or BD
 ..
 .de LD
 .br
@@ -962,7 +962,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .        rm ds*div
 .        ds*common-end
 .\}
-.el .@error-recover mismatched DE
+.el .@error-recover \\$0: mismatched DE
 ..
 .de DS
 .br
@@ -1005,7 +1005,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .        rm ds*div
 .        ds*common-end
 .\}
-.el .@error-recover mismatched DE
+.el .@error-recover \\$0: mismatched DE
 ..
 .de ds*end!4
 .ie '\\n(.z'ds*div' \{\
@@ -1018,7 +1018,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .        rm ds*div
 .        ds*common-end
 .\}
-.el .@error-recover mismatched DE
+.el .@error-recover \\$0: mismatched DE
 ..
 .\" ****************************
 .\" ******** module par ********
@@ -1355,7 +1355,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .        lt +\\n[par*box-n]u
 .        par*box-draw \\n[.i]u \\n[.l]u-(\\n[.H]u==1n*1n)
 .\}
-.el .@error B2 without B1
+.el .@error \\$0: B2 without B1
 .HTML-IMAGE-END
 ..
 .de par*box-mark-top
@@ -1508,7 +1508,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .de FP
 .br
 .if !d par*fp!\\n[FF] \{\
-.        @error unknown footnote format '\\n[FF]'
+.        @error \\$0: unknown footnote format '\\n[FF]'
 .        nr FF 0
 .\}
 .ie '\\$2'no' .par*fp!\\n[FF]-no "\\$1"
@@ -1573,7 +1573,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .        if !\\n[nh*hl] \{\
 .                nr H1 1
 .                nr nh*hl 1
-.                @error missing arguments to .NH S
+.                @error \\$0: missing arguments to .NH S
 .        \}
 .\}
 .el \{\
@@ -1586,7 +1586,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .                \}
 .                el \{\
 .                        if \\n[nh*hl]-\\n[nh*ohl]>1 \
-.                                @warning .NH \\n[nh*ohl] followed by .NH \\n[nh*hl]
+.                                @warning \\$0: .NH \\n[nh*ohl] followed by .NH \\n[nh*hl]
 .                \}
 .        \}
 .        el .nr nh*hl 1
@@ -1636,7 +1636,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .        ll -8n
 .        in (n;0\\$2)
 .\}
-.el .@error XA without XS
+.el .@error \\$0: XA without XS
 ..
 .de XE
 .ie '\\n(.z'toc*div' \{\
@@ -1644,7 +1644,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .        ev
 .        di
 .\}
-.el .@error XE without XS
+.el .@error \\$0: XE without XS
 ..
 .de toc*end-entry
 \\a\\t\\*[toc*num]
@@ -1716,7 +1716,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 ..
 .\" Note that geqn mark and lineup work correctly in centered equations.
 .de @EN
-.ie !'\\n(.z'eqn*div' .@error-recover mismatched EN
+.ie !'\\n(.z'eqn*div' .@error-recover \\$0: mismatched EN
 .el \{\
 .        br
 .        di
@@ -1812,7 +1812,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .        br
 .        di
 .        ie \\n[dn]+\\n[FM]+\\n[HM]+2v>=\\n[.p] \{\
-.                @error ridiculously long table header
+.                @error \\$0: ridiculously long table header
 .                ds@need \\n[dn]
 .                tbl*print-header
 .        \}
@@ -1823,14 +1823,14 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .                nr tbl*have-header 1
 .        \}
 .\}
-.el .@error-recover .TH without .TS H
+.el .@error-recover \\$0: .TH without .TS H
 ..
 .de @div-end!tbl*header-div
 .TH
 .TE
 ..
 .de TE
-.ie '\\n(.z'tbl*header-div' .@error-recover .TS H but no .TH before .TE
+.ie '\\n(.z'tbl*header-div' .@error-recover \\$0: .TS H but no .TH before .TE
 .el \{\
 .        nr tbl*have-header 0
 .       if !'\*(.T'html' .sp \\n[DD]u
@@ -1857,7 +1857,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .de PS
 .br
 .sp \\n[DD]u
-.ie \\n[.$]<2 .@error bad arguments to PS (not preprocessed with pic?)
+.ie \\n[.$]<2 .@error \\$0: bad arguments to PS (not preprocessed with pic?)
 .el \{\
 .        ds@need (u;\\$1)+1v
 .        in +(u;\\n[.l]-\\n[.i]-\\$2/2>?0)
--
2.19.2


Bjarni Ingi Gislason <bjarniig>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #50005:  s.tmac.55109.diff added by gbranden (2KiB - 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 (Updated the item)
  • -email is unavailable- added by bjarniig (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-03-30 gbranden Summary[PATCH] [ms] add the name of the calling macro to a diagnostic [ms] add the name of the calling macro to a diagnostic
    2022-03-05 gbranden Item GroupFeature change Warning/Suspicious behaviour
        Summary[PATCH] s.tmac: Add the name of the calling macro to a diagnostic [PATCH] [ms] add the name of the calling macro to a diagnostic
    2020-10-17 gbranden Attached File- Added s.tmac.55109.diff, #50005
        Severity3 - Normal 1 - Wish
        StatusNone Rejected
        Assigned toNone gbranden
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code