bugGNU roff - Bugs: bug #57583, [ms] stop manipulating warnings

 
 

bug #57583: [ms] stop manipulating warnings

Submitter:  Bjarni Ingi Gislason <bjarniig>
Submitted:  Sun 12 Jan 2020 08:11:17 AM UTC
   
 
Category:  Macro ms Severity:  3 - Normal
Item Group:  Warning/Suspicious 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

Mon 27 Feb 2023 12:56:50 AM UTC, comment #7: 


commit 5aa934e701ac96c48cad71eae39ddb5817a7399c
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Thu Feb 6 17:17:38 2020 +1100

    ms: Cease warning level manipulation.

    Stop ms macro package from manipulating warnings.

    * tmac/s.tmac: Remove code block that attempted to enable all
      warnings "only if none are given on the command line".  It did this by
      simply comparing the value of the warn register (\n[.warn]) against
      the default value; but of course, a user could specify -w options that
      exactly matched the default and the test would not be able to tell,
      causing puzzling and undesired behavior.  Furthermore, the hard-coded
      default was out of date and did not correspond to recent releases of
      groff.  If you want all warnings on, use the ".warn" request with no
      arguments in your ms document or pass "-w w" to groff (see troff(1) or
      the Texinfo manual for more on warnings).

    Thanks to Bjarni Ingi Gislason for bringing this issue to our attention.

    Resolves <https://savannah.gnu.org/bugs/?57583>.


G. Branden Robinson <gbranden>
Group administrator
Thu 06 Feb 2020 06:27:56 AM UTC, comment #6: 

Fixed in 5aa934e701ac96c48cad71eae39ddb5817a7399c.

G. Branden Robinson <gbranden>
Group administrator
Wed 05 Feb 2020 02:35:00 PM UTC, comment #5: 

If you want to change anything, it would probably be best to simply delete the line ".if (\n[.warn] == 1245191) .warn" and the comment above it.

Why should a macro set fiddle with user settings at all?
Why not simply honour what the user requests?
Very few macro sets interfere with the warning settings chosen by the user.

On top of that, as you observed, the current code leaves no way for the user to select 1245191.

Ingo Schwarze <schwarze>
Group Member
Tue 04 Feb 2020 10:32:56 PM UTC, comment #4: 


>   Your own wording is wrong.


You're right.

> 1) .\" Enable warnings (only if none are given on the command line).


groff(7) does not document what .warn with no arguments does.  That's a problem.

.warn with no arguments turns all warnings on.

But even so, you're not testing what was given on the command-line.  This comment, which predates your patch, is misleading.  Warnings don't keep track of what set them.  You don't know how the \n[.warn] register got set to the value it has.  What I think you're doing is checking the value of this register against the current (groff 1.22.4) default, judging by the following experiment:


$ for TS in utf8 ps; do printf '.tm \\n[.warn]\n' | groff -T$TS; done
1114119
1245191


Someone could specify a sequence of -w and -W arguments that reproduce the default, and falsify your comment, because you'd still turn all the warnings on.

So, you're right, my wording was incorrect.  It should have been

"tmac/s.tmac: Enable all warnings in nroff mode if warning register has its default value."

Your patch makes more sense to me now.  I don't know if it is good policy for the macro package to be doing this, but your patch makes its behavior more consistent with what its intention appears to be; the original implementer perhaps did not think about nroff mode, or didn't care, or this code dates back to a time when the \n[.warn] values did not differ between the modes.

Interestingly, the `.nroff` and `.troff` requests do not affect `\n[.warn]`:


$ groff
.warn 1
.tm \n[.warn]
1
.nroff
.tm \n[.warn]
1
.troff
.tm \n[.warn]
1


As none of these are classical roff requests I suppose we can revisit the issue of whether the above is good behavior.

Regarding starting sentences on new lines, that's fine material for a different bug/patch.

G. Branden Robinson <gbranden>
Group administrator
Tue 04 Feb 2020 10:04:21 PM UTC, comment #3: 


[comment #1:]

> If I understand this patch correctly, I'm not sure I agree with it.
>
> Rewriting the proposed commit message in my own words, I have:
>
> "tmac/s.tmac: Disable font warnings in nroff mode."
>
> If that's accurate then I don't think I agree.  I think ms document authors should get warnings if they try to "mount" other font families when they're rendering to a terminal, which can't (generally) perform this action.
>


  Your own wording is wrong.

1) .\" Enable warnings (only if none are given on the command line).

  is unchanged.

2) What the software processed (saw, sees):

.if (\n[.warn] == 1245191) .warn

3) What it processes with my patch (will see):

+.ie n \{\
+.  if (\n[.warn] == 1114119) .warn
+.\}
+.el \{\
+.  if  (\n[.warn] == 1245191) .warn
+.\}

4) What is changed?

  What unchanged?

N.B. How about beginning more sentences on a new line?

Bjarni Ingi Gislason <bjarniig>
Tue 04 Feb 2020 02:02:42 PM UTC, comment #2: 

I agree with all you are saying.  On top of that, the new code would be messy and ugly.  There is value in keeping things as straightforward and simple as possible.

So i would welcome closing this ticket as "Won't fix".

Ingo Schwarze <schwarze>
Group Member
Tue 04 Feb 2020 07:47:40 AM UTC, comment #1: 

If I understand this patch correctly, I'm not sure I agree with it.

Rewriting the proposed commit message in my own words, I have:

"tmac/s.tmac: Disable font warnings in nroff mode."

If that's accurate then I don't think I agree.  I think ms document authors should get warnings if they try to "mount" other font families when they're rendering to a terminal, which can't (generally) perform this action.

People love to use font family changes to communicate semantic content.  The warning reminds them of what they're losing in nroff-mode output.  If they want to avoid the warnings they should `.if n` or `.ie n` in their source document.  They can supply alternative information to replace what would have been communicated by the font family change.

Or they can shut the warning off themselves.

Or they can do this at the top of their ms document:


.if n .ab TERMINALS GO HOME


What do other people think?

G. Branden Robinson <gbranden>
Group administrator
Sun 12 Jan 2020 08:11:17 AM UTC, original submission:  


From 0c68c9ac087102d8622e756cf75c788f4b80d327 Mon Sep 17 00:00:00 2001
From: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
Date: Sun, 12 Jan 2020 07:39:47 +0000
Subject: [PATCH] s.tmac: Test the default number for warnings for nroff

  Add the default number for warnings in the file "groff/tmac/s.tmac"
(ms macros) in a test for nroff.

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

diff --git a/tmac/s.tmac b/tmac/s.tmac
index 5737dfd3..08f18cf2 100644
--- a/tmac/s.tmac
+++ b/tmac/s.tmac
@@ -23,9 +23,19 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 .if !\n(.g .ab These ms macros require groff.
 .if \n(.C \
 .        ab The groff ms macros do not work in compatibility mode.
+.
 .\" Enable warnings (only if none are given on the command line).
 .\" You can delete this if you want.
-.if (\n[.warn] == 1245191) .warn
+.\" All warnings sum to 2,097,151.
+.\" The numbers are for the default warnings.
+.ie n \{\
+.  \" No warning about missing fonts
+.  if (\n[.warn] == 1114119) .warn
+.\}
+.el \{\
+.  if  (\n[.warn] == 1245191) .warn
+.\}
+.
 .\" See if already loaded.
 .if r GS .nx
 .mso devtag.tmac
--
2.24.1


Bjarni Ingi Gislason <bjarniig>

 

(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 schwarze (Posted a comment)
  • -email is unavailable- added by gbranden (Posted a comment)
  • -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 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-03-05 gbranden Item GroupFeature change Warning/Suspicious behaviour
        Summarys.tmac: stop manipulating warnings [ms] stop manipulating warnings
    2022-01-22 gbranden Item GroupNone Feature change
        Planned ReleaseNone 1.23.0
    2020-02-06 gbranden Severity1 - Wish 3 - Normal
        StatusNeed Info Fixed
        Open/ClosedOpen Closed
        Summary[PATCH] s.tmac: Test the default number for warnings for nroff s.tmac: stop manipulating warnings
    2020-02-04 schwarze Severity3 - Normal 1 - Wish
        Item GroupIncorrect behaviour None
    2020-02-04 gbranden StatusNone Need Info
        Assigned toNone gbranden

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code