bugGNU roff - Bugs: bug #63202, [troff] missing delimiter...

 
 

bug #63202: [troff] missing delimiter diagnostics could say what they were expecting

Submitter:  G. Branden Robinson <gbranden>
Submitted:  Wed 12 Oct 2022 06:33:11 PM UTC
   
 
Category:  Core Severity:  1 - Wish
Item Group:  Warning/Suspicious behaviour Status:  Fixed
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  1.24.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Tue 20 Aug 2024 12:37:08 AM UTC, comment #3: 


commit 956955c6634b506617818b6a29c892ff18375847
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Sat Aug 17 18:24:34 2024 -0500

    [troff]: Fix Savannah #63202 (1/3).

    [troff]: When throwing a warning diagnostic about a mismatched escape
    sequence delimiter, say what what we were expecting and what we got
    instead.

    * src/roff/troff/input.cpp (read_delimited_number, get_line_arg)
      (do_register):
    * src/roff/troff/reg.cpp (inline_define_register [0]): Do it.

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

    $ cat ATTIC/mismatched-delimiter-demo.groff
    foo\R'a 20g'
    foo\L'1ig#
    foo\h'5mbar
    foo\H'20@
    $ ./build/test-groff -ww -z ATTIC/mismatched-delimiter-demo.groff
    troff:ATTIC/mismatched-delimiter-demo.groff:1: warning: closing delimiter does not match; expected character "'", got character 'g'
    troff:ATTIC/mismatched-delimiter-demo.groff:2: warning: closing delimiter does not match; expected character "'", got character '#'
    troff:ATTIC/mismatched-delimiter-demo.groff:3: warning: closing delimiter does not match; expected character "'", got character 'b'
    troff:ATTIC/mismatched-delimiter-demo.groff:4: warning: closing delimiter does not match; expected character "'", got character '@'

commit 852d2209bd892a71be36fec55efb7dac28109a22
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Sat Aug 17 18:32:51 2024 -0500

    [troff]: Fix Savannah #63202 (2/3).

    * src/roff/troff/input.cpp (do_overstrike, do_bracket, do_name_test):
      (do_zero_width, do_width, do_device_control): When throwing a warning
      diagnostic about a mismatched escape sequence delimiter (in an escape
      sequence that accepts a newline as a delimiter(!)), say what what we
      were expecting and what we got instead.

    $ printf '\\o\nabc' | build/test-groff -a -ww
    troff: warning: missing closing delimiter in overstrike escape sequence; expected a newline, got end of input
    <beginning of page>

    $ printf '\\b\nabc' | build/test-groff -a -ww
    troff: warning: missing closing delimiter in bracket-building escape sequence; expected a newline, got end of input
    <beginning of page>

    $ printf '\\A\nabc' | build/test-groff -a -ww
    troff: warning: missing closing delimiter in identifier validation escape sequence; expected a newline, got end of input
    <beginning of page>
    1

    $ printf '\\Z\nabc' | build/test-groff -a -ww
    troff: warning: missing closing delimiter in zero-width escape sequence; expected a newline, got end of input
    <beginning of page>

    $ printf '\\w\nabc' | build/test-groff -a -ww
    troff: warning: missing closing delimiter in width computation escape sequence (got end of input)
    <beginning of page>
    13880
    $ printf '\\X\nabc' | build/test-groff -a -ww
    troff: warning: missing closing delimiter in device control escape sequence (got end of input)
    <beginning of page>

    We don't see output from most of the cases above because the
    "approximate output" option `-a` doesn't do anything to render
    overstrike, bracket, zero-width output, or device control nodes.  The
    "abc" characters are visible in device-independent output `-Z` and of
    course, e.g., PostScript.

commit 173e9849f34f57cd3f52df841dd6103ce65365a6
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Aug 19 15:14:05 2024 -0500

    [troff]: Fix Savannah #63202 (3/3).

    * src/roff/troff/input.cpp (do_expr_test): When throwing a warning
      diagnostic about a mismatched escape sequence delimiter (in an escape
      sequence that accepts a newline as a delimiter(!)), say what what we
      were expecting and what we got instead.

    $ printf '\\Babc\n' | build/test-groff -a -ww
    troff:<standard input>:1: warning: missing closing delimiter in numeric expression validation escape sequence; expected character 'a', got a newline
    <beginning of page>
    0


G. Branden Robinson <gbranden>
Group administrator
Sat 17 Aug 2024 09:29:54 PM UTC, comment #2: 


$ cat ATTIC/mismatched-delimiter-demo.groff
foo\R'a 20g'
foo\L'1ig#
foo\h'5mbar
foo\H'20@
$ ./build/test-groff -ww -z ATTIC/mismatched-delimiter-demo.grofftroff:ATTIC/mismatched-delimiter-demo.groff:1: warning: D closing delimiter does not match; expected character "'", got character 'g'
troff:ATTIC/mismatched-delimiter-demo.groff:2: warning: C closing delimiter does not match; expected character "'", got character '#'
troff:ATTIC/mismatched-delimiter-demo.groff:3: warning: B closing delimiter does not match; expected character "'", got character 'b'
troff:ATTIC/mismatched-delimiter-demo.groff:4: warning: A closing delimiter does not match; expected character "'", got character '@'


G. Branden Robinson <gbranden>
Group administrator
Sat 17 Aug 2024 09:29:02 PM UTC, comment #1: 

Almost 2 years later, this proved much less troublesome.


diff --git a/src/roff/troff/input.cpp b/src/roff/troff/input.cpp
index f49dba429..9e994be7a 100644
--- a/src/roff/troff/input.cpp
+++ b/src/roff/troff/input.cpp
@@ -5186,8 +5186,15 @@ static bool read_delimited_number(units *n,
   if (start_token.is_usable_as_delimiter(true /* report error */)) {
     tok.next();
     if (read_measurement(n, si, prev_value)) {
-      if (start_token != tok)
-       warning(WARN_DELIM, "closing delimiter does not match");
+      if (start_token != tok) {
+       // token::description() writes to static, class-wide storage, so
+       // we must allocate a copy of it before issuing the next
+       // diagnostic.
+       char *delimdesc = strdup(start_token.description());
+       warning(WARN_DELIM, "closing delimiter does not match;"
+               " expected %1, got %2", delimdesc, tok.description());
+       free(delimdesc);
+      }
       return true;
     }
   }
@@ -5201,8 +5208,15 @@ static bool read_delimited_number(units *n, unsigned char si)
   if (start_token.is_usable_as_delimiter(true /* report error */)) {
     tok.next();
     if (read_measurement(n, si)) {
-      if (start_token != tok)
-       warning(WARN_DELIM, "closing delimiter does not match");
+      if (start_token != tok) {
+       // token::description() writes to static, class-wide storage, so
+       // we must allocate a copy of it before issuing the next
+       // diagnostic.
+       char *delimdesc = strdup(start_token.description());
+       warning(WARN_DELIM, "closing delimiter does not match;"
+               " expected %1, got %2", delimdesc, tok.description());
+       free(delimdesc);
+      }
       return true;
     }
   }
@@ -5226,8 +5240,15 @@ static bool get_line_arg(units *n, unsigned char si, charinfo **cp)
       tok.next();
     }
     if (!(start_token == tok
-         && input_stack::get_level() == start_level))
-      warning(WARN_DELIM, "closing delimiter does not match");
+         && input_stack::get_level() == start_level)) {
+      // token::description() writes to static, class-wide storage, so
+      // we must allocate a copy of it before issuing the next
+      // diagnostic.
+      char *delimdesc = strdup(start_token.description());
+      warning(WARN_DELIM, "closing delimiter does not match; expected"
+             " %1, got %2", delimdesc, tok.description());
+      free(delimdesc);
+    }
     return true;
   }
   return false;
@@ -5439,8 +5460,13 @@ static void do_register()
   int val;
   if (!read_measurement(&val, 'u', prev_value))
     return;
+  // token::description() writes to static, class-wide storage, so we
+  // must allocate a copy of it before issuing the next diagnostic.
+  char *delimdesc = strdup(start_token.description());
   if (start_token != tok)
-    warning(WARN_DELIM, "closing delimiter does not match");
+    warning(WARN_DELIM, "closing delimiter does not match; expected %1,"
+           " got %2", delimdesc, tok.description());
+  free(delimdesc);
   if (r != 0 /* nullptr */)
     r->set_value(val);
   else
diff --git a/src/roff/troff/reg.cpp b/src/roff/troff/reg.cpp
index 57f10b5a7..654b2c371 100644
--- a/src/roff/troff/reg.cpp
+++ b/src/roff/troff/reg.cpp
@@ -360,8 +360,15 @@ void inline_define_register()
     if (start_token != tok) {
       if (read_measurement(&v, 'u')) {
        r->set_increment(v);
-       if (start_token != tok)
-         warning(WARN_DELIM, "closing delimiter does not match");
+       if (start_token != tok) {
+         // token::description() writes to static, class-wide storage,
+         // so we must allocate a copy of it before issuing the next
+         // diagnostic.
+         char *delimdesc = strdup(start_token.description());
+         warning(WARN_DELIM, "closing delimiter does not match;"
+                 " expected %1, got %2", delimdesc, tok.description());
+         free(delimdesc);
+       }
       }
     }
   }


(The code in "reg.cpp" is "#if 0"-ed out.  FYI.)

G. Branden Robinson <gbranden>
Group administrator
Wed 12 Oct 2022 06:33:11 PM UTC, original submission:  

In all of the escape sequences that accept user-determined delimiters, we have diagnostic messages that complain of a missing closing one.

We could therefore say in those messages which delimiter we were expecting, which might make life easier, especially for users who bother to distinguish their nested delimiters instead of just spraying apostrophes everywhere and praying.  The opening delimiter is always stored already as `start`, or in one case `delim`, so there's no overhead to giving the user this information.

However, an attempt to dash this off in five minutes revealed two problems:

1.  Those damnable newline-accepting escape sequences only throw the mismatch diagnostic upon EOF.  When I try to make them complain when a newline is read and it's NOT the delimiter, my regression test trips.  So this feature, apart from being of dubious worth, is subtle and quick to anger.

2.  Having applied, I was sometimes told that the opening delimiter was a "node".  This is not helpful feedback to the user.

Come back to this someday.  The fact that we now (post-groff 1.22.4) identify which escape sequence is being processed when trouble occurs may mitigate the motivation I offered in paragraph 2 above.


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 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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-08-20 gbranden Planned ReleaseNone 1.24.0
    2024-08-20 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2024-08-17 gbranden StatusNone In Progress
        Assigned toNone gbranden

    Back to the top

    Powered by Savane 3.14-430a.
    Corresponding source code