bugGNU troff - Bugs: bug #53314, [PATCH] strip.sed: Keep the...

 
 

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

Submitted by:  Bjarni Ingi Gislason <bjarniig>
Submitted on:  Sat 10 Mar 2018 12:34:39 AM UTC  
 
Category:  None Severity:  3 - Normal
Item Group:  Build/Installation Status:  Need Info
Privacy:  Public Assigned to:  G. Branden Robinson <gbranden>
Open/Closed:  Open Planned Release:  None

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

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

http://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>
Project 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>
Project MemberIn charge of this item.
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)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-04-23 gbranden StatusNone => Need Info
        Assigned toNone => gbranden

    Back to the top


    Powered by Savane 3.4