bugGNU roff - Bugs: bug #66434, [troff] want `so` and related...

 
 

bug #66434: [troff] want `so` and related requests to accept file names with embedded spaces

Submitter:  G. Branden Robinson <gbranden>
Submitted:  Tue 12 Nov 2024 03:22:01 PM UTC
   
 
Category:  Core Severity:  1 - Wish
Item Group:  Feature change Status:  Fixed
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  1.24.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 16 Jan 2025 09:58:03 PM UTC, comment #5: 

Bug #66673 further refined the documentation associated with this change.

Dave <barx>
Group Member
Thu 14 Nov 2024 05:03:10 PM UTC, comment #4: 


commit f4c19e6e44d8831b0a674eb4865a89ee8f575aed
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Tue Nov 12 12:06:08 2024 -0600

    [troff]: Fix Savannah #66434 (spaces in `so` arg).

    * src/roff/troff/input.cpp (do_source, do_macro_source): Refactor to use
      `read_string()` instead of `get_long_name()` to obtain the argument;
      this means that the argument consumes the rest of the input line
      instead of being treated as a groff identifier.  This change permits
      spaces in sourced file names.

    * doc/groff.texi.in (I/O) <so, soquiet, mso, msoquiet>
      (Other Differences):
    * man/groff.7.man (Request short reference) <so>:
    * man/groff_diff.7.man (New requests) <mso, msoquiet, soquiet>:
      (Other differences): Document it.

    * src/roff/groff/tests/so-request-accepts-embedded-space-in-arg.sh: Test
      it.
    * src/roff/groff/groff.am (groff_TESTS): Run test.

    * NEWS: Add item.

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


G. Branden Robinson <gbranden>
Group administrator
Tue 12 Nov 2024 05:39:25 PM UTC, comment #3: 

comment #2:

> In progress as we speak...


Oh.  I missed bug #66027 altogether.  Today is my day for overlooking what's in plain sight.  Thanks for removing my blinders.

Dave <barx>
Group Member
Tue 12 Nov 2024 05:22:28 PM UTC, comment #2: 

comment #1:

> A way around this discussed in 65108 is removing the backslash requirement from soelim.  As you observed, "Since backslash-space is apparently a GNU extension in the first place, we might consider dropping it."


In progress as we speak...


commit 2d88c01b31420033b5d5d2f473b26cc7042c610e
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Tue Nov 12 10:26:07 2024 -0600

    [soelim]: Fix Savannah #66027.

    Read argument to `so` request in a more GNU troff-ish fashion (and a
    more AT&T troff-ish fasion, if one considers the `ds` and `as`
    requests).  Accept but discard a leading double quote to permit
    embedding of leading spaces in the file name, and accept unescaped
    literal spaces in argument.

    * src/preproc/soelim/soelim.cpp (do_so): Do it.
    * src/preproc/soelim/soelim.1.man (Description): Document it.

    * NEWS: Add item.

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

diff --git a/ChangeLog b/ChangeLog
index a7693c628..f08cc3875 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2024-11-12  G. Branden Robinson <g.branden.robinson@gmail.com>
+
+       [soelim]: Read argument to `so` request in a more GNU troff-ish
+       fashion (and a more AT&T troff-ish fasion, if one considers the
+       `ds` and `as` requests).  Accept but discard a leading double
+       quote to permit embedding of leading spaces in the file name,
+       and accept unescaped literal spaces in argument.
+
+       * src/preproc/soelim/soelim.cpp (do_so): Do it.
+       * src/preproc/soelim/soelim.1.man (Description): Document it.
+
+       * NEWS: Add item.
+
+       Fixes <https://savannah.gnu.org/bugs/?66027>.
+
 2024-11-12  G. Branden Robinson <g.branden.robinson@gmail.com>

        [soelim]: Fix code style nits.
diff --git a/NEWS b/NEWS
index a40ed64fe..ca2bda6ba 100644
--- a/NEWS
+++ b/NEWS
@@ -184,6 +184,17 @@ nroff
 *  nroff now supports clustered options ("-tzms", for example) as groff,
    troff, and other GNU getopt-using programs do.

+soelim
+------
+
+*  soelim no longer requires embedded space characters in `so` arguments
+   to be backslash-escaped.  (It continues to support that syntax.)  If
+   the argument to a `so` request must contain leading spaces, any such
+   sequence of spaces must now be prefixed with a double quote character
+   ("), which the program then discards.  These changes are to better
+   align this program's parsing rules with those of GNU troff and of
+   AT&T troff historically; consider the `ds` and `as` requests.
+
 Macro packages
 --------------

diff --git a/src/preproc/soelim/soelim.1.man b/src/preproc/soelim/soelim.1.man
index 7bec18434..5670a8348 100644
--- a/src/preproc/soelim/soelim.1.man
+++ b/src/preproc/soelim/soelim.1.man
@@ -135,19 +135,34 @@ .SH Description
 .
 .
 .PP
-If the name of a
+.I @g@soelim
+reads the
 .I included-file
-contains a backslash,
+argument as
+GNU
+.I troff \" GNU
+does.
+.
+It ignores spaces immediately after \[lq]so\[rq];
+to embed a sequence of one or more leading spaces in the argument,
+prefix the sequence with a neutral double quote
+.RB ( \[dq] ).
+.
+Non-leading spaces are interpreted literally.
+.
+If the included file name requires a backslash,
 use
 .B \[rs]\[rs]
 or
 .B \[rs]e
 to embed it.
 .
-To embed a space,
-write
-.RB \[lq] \[rs]\~ \[rq]
-(backslash followed by a space).
+A backslash followed by a space
+.RB (\[lq] \[rs]\~ \[rq])
+also encodes a space,
+for compatibility with earlier versions of
+GNU
+.IR soelim . \" GNU
 .
 Any other escape sequence in
 .IR included-file ,
@@ -157,6 +172,12 @@ .SH Description
 .I @g@soelim
 from replacing the source request.
 .
+AT&T and descendant versions of
+.I soelim
+have no means of embedding spaces in file names;
+they replace the first space encountered with a newline
+and stop interpreting the request.
+.
 .
 .PP
 The dot must be at the beginning of a line and must be followed by
diff --git a/src/preproc/soelim/soelim.cpp b/src/preproc/soelim/soelim.cpp
index e10447d06..28e80d736 100644
--- a/src/preproc/soelim/soelim.cpp
+++ b/src/preproc/soelim/soelim.cpp
@@ -140,9 +140,10 @@ void do_so(const char *line)
     p++;
   string filename;
   bool is_filename_valid = true;
-  for (const char *q = p;
-       is_filename_valid && *q != '\0' && *q != '\n' && *q != ' ';
-       q++)
+  const char *q = p;
+  if ('"' == *q)
+    q++;
+  for (; is_filename_valid && (*q != '\0') && (*q != '\n'); q++)
     if (*q == '\\') {
       switch (*++q) {
       case 'e':


G. Branden Robinson <gbranden>
Group administrator
Tue 12 Nov 2024 04:55:17 PM UTC, comment #1: 

As discussed in bug #65108 (of which the current bug is a special case), this change, as presented, will make the .so request require a different syntax when soelim is used than when it isn't.

A way around this discussed in 65108 is removing the backslash requirement from soelim.  As you observed, "Since backslash-space is apparently a GNU extension in the first place, we might consider dropping it."

Dave <barx>
Group Member
Tue 12 Nov 2024 03:22:01 PM UTC, original submission:  

It'd be nice if this worked.


$ cat ATTIC/foo.groff
foo
.so bar baz.groff
qux
$ cat ATTIC/bar\ baz.groff
bar baz
$ (cd ATTIC; groff -a -ww -T ascii foo.groff | cat -s)
troff:foo.groff:2: error: cannot open 'bar': No such file or directory
<beginning of page>
foo qux


But with this patch and a couple of prerequisite changes:


diff --git a/src/roff/troff/input.cpp b/src/roff/troff/input.cpp
index 6ad2770eb..cba7ecc75 100644
--- a/src/roff/troff/input.cpp
+++ b/src/roff/troff/input.cpp
@@ -6570,23 +6570,17 @@ static void while_continue_request()

 void do_source(bool quietly)
 {
-  symbol nm = get_long_name(true /* required */);
-  if (nm.is_null())
-    skip_line();
-  else {
-    while (!tok.is_newline() && !tok.is_eof())
-      tok.next();
-    errno = 0;
-    FILE *fp = include_search_path.open_file_cautious(nm.contents());
-    if (fp)
-      input_stack::push(new file_iterator(fp, nm.contents()));
-    else
-      // Suppress diagnostic only if we're operating quietly and it's an
-      // expected problem.
-      if (!(quietly && (ENOENT == errno)))
-       error("cannot open '%1': %2", nm.contents(), strerror(errno));
-    tok.next();
-  }
+  char *filename = read_string();
+  errno = 0;
+  FILE *fp = include_search_path.open_file_cautious(filename);
+  if (fp != 0 /* nullptr */)
+    input_stack::push(new file_iterator(fp, filename));
+  else
+    // Suppress diagnostic only if we're operating quietly and it's an
+    // expected problem.
+    if (!(quietly && (ENOENT == errno)))
+      error("cannot open '%1': %2", filename, strerror(errno));
+  tok.next();
 }

 void source_request() // .so
@@ -8740,26 +8734,20 @@ static void process_startup_file(const char *filename)

 void do_macro_source(bool quietly)
 {
-  symbol nm = get_long_name(true /* required */);
-  if (nm.is_null())
-    skip_line();
-  else {
-    while (!tok.is_newline() && !tok.is_eof())
-      tok.next();
-    char *path;
-    FILE *fp = mac_path->open_file(nm.contents(), &path);
-    if (fp != 0 /* nullptr */) {
-      input_stack::push(new file_iterator(fp, symbol(path).contents()));
-      free(path);
-    }
-    else
-      // Suppress diagnostic only if we're operating quietly and it's an
-      // expected problem.
-      if (!quietly && (ENOENT == errno))
-       warning(WARN_FILE, "cannot open macro file '%1': %2",
-               nm.contents(), strerror(errno));
-    tok.next();
+  char *macro_filename = read_string();
+  char *path;
+  FILE *fp = mac_path->open_file(macro_filename, &path);
+  if (fp != 0 /* nullptr */) {
+    input_stack::push(new file_iterator(fp, macro_filename));
+    free(path);
   }
+  else
+    // Suppress diagnostic only if we're operating quietly and it's an
+    // expected problem.
+    if (!quietly && (ENOENT == errno))
+      warning(WARN_FILE, "cannot open macro file '%1': %2",
+             macro_filename, strerror(errno));
+  tok.next();
 }

 void macro_source_request() // .mso


...it can.


$ (cd ATTIC; ../build/test-groff -a -ww -T ascii foo.groff | cat -s)
<beginning of page>
foo bar baz qux


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

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by barx (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 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-11-14 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
        Planned ReleaseNone 1.24.0
    2024-11-12 gbranden Dependencies- bugs #65099 is dependent

    Back to the top

    Powered by Savane 3.14-9aa3.
    Corresponding source code