bugGNU roff - Bugs: bug #53314, strip.sed: Keep the protection...

 
 

bug #53314: strip.sed: Keep the protection (and visibility) of spaces

Submitter:  Bjarni Ingi Gislason <bjarniig>
Submitted:  Sat 10 Mar 2018 12:34:39 AM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  Build/Installation Status:  Invalid
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 04 Jul 2021 03:49:35 PM UTC, comment #7: 

This report has been invalidated by the deletion of strip.sed from the groff distribution.

G. Branden Robinson <gbranden>
Group administrator
Mon 04 Jan 2021 05:40:45 AM UTC, comment #6: 

This patch needs to be updated with respect to the following commit to be considered.  Any future revision should be attached as a diff file to avoid damage to the literal spaces and tabs in the file.

However, it may not be worth the trouble except as a potential future example in documentation--mailing list consensus is in favor of dropping the stripping procedure and I plan to do so at some point after 1.23.0 is released.


commit c4d82acf4de9f1d6e19ef824607f3fd45f0dcf33
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Nov 5 07:00:46 2018 -0500

    tmac/strip.sed: Operate only if given permission.

            * tmac/strip.sed: Do nothing whatsoever to any input file that
            lacks a line matching "%beginstrip%".  This implements a
            contract between the macro file writer and the stripper script
            (roff input is not parseable with crude weaponry like regular
            expressions).  In practice, most of the changes performed by the
            script were already in a block enforcing this.  My changes:
              + Stop unconditionally inserting a roff comment noting the
                reader of stripping at line 2.  Instead, replace the line
                matching "%beginstrip%" with this comment if and only if it
                is present.
              + Stop deleting lines containing only the empty request if
                they occur before "%beginstrip%".
              + Tweak the wording of the roff comment.
              + Remove comment within the sed script; see
            <https://lists.gnu.org/archive/html/groff/2017-11/msg00057.html>
                and the sed section of "Limitations of Usual Tools" in the
                GNU Autoconf Manual.

            * tmac/e.tmac-u: Add a comment line to visually separate the end
            of the license statement from the comment emplaced by strip.sed.
            This is also consistent with the mdoc macro files.

            Invalidates <https://savannah.gnu.org/bugs/index.php?53784>.


G. Branden Robinson <gbranden>
Group administrator
Thu 26 Apr 2018 07:43:50 PM UTC, comment #5: 

  1)

  This is my latest version of strip.sed

/%beginstrip%/,$ {
  /^\.ig$/,/^\.\./d
  s/^\([.']\)[         ]*/\1/
  s/^\(\\![.']\)[         ]*/\1/
  /^\.\\"/d
  /^\\#/d
  s/\\#.*/\\/
  s/\\"..*/\\"/
  /\.[ad]s/ !s/[         ]*\\"//
  /\.[ad]s/ s/\([^         ]\)\\"/\1/
}
/^\.$/d

  Checked with the following script:

#!/bin/sh
#Compare stripped with unstripped versions of tmac files.

output=/tmp/tmac.u.s.diff
# get list of all tmac files
test_dir=/tmp/tmac.test
if test -d $test_dir ;then
  rm -f $test_dir/*
else
  mkdir $test_dir
fi

#cp $(find . -name '*.tmac' -o -name '*-u') $test_dir
cp -t $test_dir $(find $HOME/git/groff -name '*-u')

rm -f $output

cd $test_dir

for item in * ; do
  sed -f $HOME/bin/strip.sed $item | \
  diff -w ${item} -  >> $output || :
done

sed -i -e '/^< \. *\\"/d' -e '/^< \.$/d' -e '/^[0-9]\+[,d][0-9]\+d*$/d' $output

  Noticed one case of insufficient protection in file "e.tmac-u",
reported in groff bug nr. 53760.

  2) My change was wrong, as noticed in comment #2


  I have not yet compiled groff with my version of "strip.sed".

  My aim is to make a stripped version of all macro files available for
use, at least for myself.

  Why may (should) users not have a choice?

###

  Something about "education":

  Seen on Usenet forum "alt.education"
  Date: Tue, 24 Apr 2018 22:28:57 GMT

www.americanthinker.com/articles/2018/04/k12_how_our_schools_make_monsters.html

Bjarni Ingi Gislason <bjarniig>
Mon 23 Apr 2018 02:42:37 PM UTC, comment #4: 

I think it is a bad idea to try to tweak this further.

Through at least two (fortunately very small) bugs, we have seen that stripping tends to introduce incorrect behaviour.  Making this more complicated is going to increase the risk of bugs.

The root cause is that roff is a complicated (some might even say "quirky") language, and parsing such a language with an ad-hoc three-line script will inevitably break sooner or later.

The real fix is to stop stripping at all, with other beneficial side effects in addition to correctness, discussed at length on the mailing list.  What happened to that plan?  The mom maintainer - as the most important user currently - is clearly in favour of not stripping, and so am i for mdoc.  It cannot really be important for the relatively small macro sets me and hdtbl.

Then again, if you want to waste your time on such hopeless tweaking, i don't strongly object to that, just make sure you are not introducing serious regressions.  Small bugs can't be avoided either way, i think.

Ingo Schwarze <schwarze>
Group Member
Mon 23 Apr 2018 12:12:22 PM UTC, comment #3: 

Bjarni,

I have 2 questions.

1. Do you have an updated and tested version of your patch?

2. You propose a partial reversion of a commit I made, ae59ee569eb5cddab8bdf12a8009bd1b907a8205 ("Remove nilpotent capture.")  I do not understand what was wrong with it; the grouping operators or "captures" \( \) I removed were in the line-matching, addressing part of the sed command, not the regex parameter to the s(ubstitute) command.

Did my commit ae59ee569eb5cddab8bdf12a8009bd1b907a8205 introduce a problem that was not already present?

G. Branden Robinson <gbranden>
Group administrator
Sun 11 Mar 2018 06:12:39 PM UTC, comment #2: 



  My change is still flawed.

  The line '/.[ad]s/s/\([^         ]*\)\\"/\1/'

should not be removed but changed.
(besides . -> \.[         ]*)

'/\.[         ]*[ad]s/ s/\([^         ]\)\\"/\1/'

  This will only remove the comment escape, if there is no space in
front of it.

N.B.  The space is only relevant if a beginning '"' (quotation mark) is
in front of it.


Bjarni Ingi Gislason <bjarniig>
Sat 10 Mar 2018 12:05:52 PM UTC, comment #1: 

  A new approach.

From d83ccadc31ba5201da40695ca1ac1ff90338be86 Mon Sep 17 00:00:00 2001
From: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
Date: Sat, 10 Mar 2018 11:58:05 +0000
Subject: [PATCH] tmac/strip.sed: Keep the protection (and visibility) of
 spaces

  Do not remove the protection (\") of spaces at the end of a
definition of strings (line '/.[ad]s/s/\([^         ]*\)\\"/\1/'
removed)

  Make the change only apply to definitions of strings
(change '.' to '\.[ ]*'

  The comment escape (\") after a space in the definition of strings is
used to protect spaces, make them visible and significant.

  Commit ae59ee569eb5cddab8bdf12a8009bd1b907a8205 is thus partially
reversed.

Signed-off-by: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
---
 tmac/strip.sed | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tmac/strip.sed b/tmac/strip.sed
index c2c9e892..c5b72657 100644
--- a/tmac/strip.sed
+++ b/tmac/strip.sed
@@ -8,7 +8,6 @@
   s/^\\#.*/./
   s/\\".*/\\"/
   s/\\#.*/\\/
-  /.[ad]s/!s/[ ]*\\"//
-  /.[ad]s/s/\([^ ]*\)\\"/\1/
+  /\.[ ]*[ad]s/!s/[ ]*\\"//
 }
 /^\.$/d
--
2.16.1


Bjarni Ingi Gislason <bjarniig>
Sat 10 Mar 2018 12:34:39 AM UTC, original submission:  

From 72bac6cdef91bca9ea2095832592c5ebf85e900f Mon Sep 17 00:00:00 2001
From: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
Date: Sat, 10 Mar 2018 00:15:54 +0000
Subject: [PATCH] strip.sed: Keep the protection (and visibility) of
 spaces

  The comment escape (\") after a space in the definition of strings
must not be removed.
  It is used to protect spaces, make them visible and significant.

  Commit ae59ee569 is thus partially fixed.

Signed-off-by: Bjarni Ingi Gislason <bjarniig@rhi.hi.is>
---
 tmac/strip.sed | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tmac/strip.sed b/tmac/strip.sed
index c2c9e892..7f1b94e2 100644
--- a/tmac/strip.sed
+++ b/tmac/strip.sed
@@ -9,6 +9,5 @@
   s/\\".*/\\"/
   s/\\#.*/\\/
   /.[ad]s/!s/[         ]*\\"//
-  /.[ad]s/s/\([^         ]*\)\\"/\1/
 }
 /^\.$/d

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

    Date Changed by Updated Field Previous Value => Replaced by
    2022-06-10 gbranden Summary[PATCH] strip.sed: Keep the protection (and visibility) of spaces strip.sed: Keep the protection (and visibility) of spaces
    2021-07-04 gbranden StatusNeed Info Invalid
        Open/ClosedOpen Closed
    2018-04-23 gbranden StatusNone Need Info
        Assigned toNone gbranden

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code