GNU roff - Bugs: bug #64061, pdfpic.tmac requires non-standard...

Group

Main
- Main
- View members
- Search
Homepage
Download
Source code
Bugs
- Submit new
- Browse
- Reset to open
- Digest
- Dependencies
- ~~Export~~
- Get statistics
- Search

bug #64061: pdfpic.tmac requires non-standard sed feature

Submitter:	G. Branden Robinson <gbranden>
Submitted:	Mon 17 Apr 2023 08:33:25 AM UTC


Category:	Macro package - others/general	Severity:	3 - Normal
Item Group:	Incorrect behaviour	Status:	Fixed
Privacy:	Public	Assigned to:	gbranden
Open/Closed:	Closed	Planned Release:	1.24.0

* Mandatory Fields

Add a New Comment Rich Markup

Jump to the original submission

Mon 10 Jul 2023 08:55:45 AM UTC, comment #19: commit f60452a44c1109a9315042514d310a057b4666f3 Author: G. Branden Robinson <g.branden.robinson@gmail.com> Date: Mon Apr 17 16:41:33 2023 -0500 [pdfpic]: Fix Savannah #64061. * tmac/pdfpic.tmac: Refactor to make comprehensible some woefully undocumented cleverness and improve efficiency. (PDFPIC): Break out flaming-hoop-leaping "clever" bit of `sy` usage into its own macro, calling from here and relocating its requests from here... (pdfpic@get-image-dimensions): ...to here. When using `sy` request to collect and munge output of pdfinfo(1), (a) disable the escape character while defining the macro; (b) construct the command in a roff string, appending to it in discrete, hopefully comprehensible chunks; (c) disable the escape character during macro interpretation wherever possible (most of it); (d) retain doubled backslashes so that they survive subsequent string interpolation; (e) stop using grep(1) in the pipeline when sed(1) is perfectly capable of performing its own input filtering; (f) invoke sed with '-n' option and emit output only upon a successful substitution; (g) replace unportable(!) POSIX BRE character class '[:digit:]' in substitution match text with '[0-9]'; and most importantly (h) replace multi-line sed 's' replacement text (see below for the reason we can't use it) with single roff control line employing the groff extension escape sequence `\R` to assign multiple registers. Annotate portability and escaping challenges. Tested on GNU/Linux, macOS 12, and (with simulated pdfinfo(1) output) Solaris 11. There is a problem with trying to embed true newlines into the arguments of a `sy` request. The C++ function that GNU troff uses to assemble the command string (character by character) _does not recognize C/C++ string literal escape sequences_. This means that you _cannot_ embed "\n" in `sy`'s arguments and have it survive, as a newline character, into the command string passed to the standard C library's system(3) function. ("A\nB" gets encoded as 'A', '\\', 'n', 'B', not 'A', '\n', 'B'.) Unfortunately, this appears to be AT&T troff-compatible behavior. But it means that you _cannot_ portably construct multi-line replacement text for sed's 's' command. (Other sed commands like 'a', 'c', and 'i' will be similarly affected.) See Savannah #64071. * PROBLEMS: Drop item. Fixes <https://savannah.gnu.org/bugs/?64061>. Thanks to Bruno Haible for the report, and to him and Ralph Corderoy for the discussion of portable and efficient sed constructs.	G. Branden Robinson <gbranden>
Mon 08 May 2023 12:40:13 AM UTC, comment #18: Updated fix on branch (does not expand test cases as mused about in comment #17). commit 15ca5031a84d98a195403de89bf33a0b96c032a7 Author: G. Branden Robinson <g.branden.robinson@gmail.com> Date: Mon Apr 17 16:41:33 2023 -0500 [pdfpic]: Fix Savannah #64061. * tmac/pdfpic.tmac: Refactor to make comprehensible some woefully undocumented cleverness and improve efficiency. (PDFPIC): Break out flaming-hoop-leaping "clever" bit of `sy` usage into its own macro, calling from here and relocating its requests from here... (pdfpic@get-image-dimensions): ...to here. When using `sy` request to collect and munge output of pdfinfo(1), (a) disable the escape character while defining the macro; (b) construct the command in a roff string, appending to it in discrete, hopefully comprehensible chunks; (c) disable the escape character during macro interpretation wherever possible (most of it); (d) retain doubled backslashes so that they survive subsequent string interpolation; (e) stop using grep(1) in the pipeline when sed(1) is perfectly capable of performing its own input filtering; (f) invoke sed with '-n' option and emit output only upon a successful substitution; (g) replace unportable(!) POSIX BRE character class '[:digit:]' in substitution match text with '[0-9]'; and most importantly (h) replace multi-line sed 's' replacement text (see below for the reason we can't use it) with single roff control line employing the groff extension escape sequence `\R` to assign multiple registers. Annotate portability and escaping challenges. Tested on GNU/Linux, macOS 12, and (with simulated pdfinfo(1) output) Solaris 11. There is a problem with trying to embed true newlines into the arguments of a `sy` request. The C++ function that GNU troff uses to assemble the command string (character by character) _does not recognize C/C++ string literal escape sequences_. This means that you _cannot_ embed "\n" in `sy`'s arguments and have it survive, as a newline character, into the command string passed to the standard C library's system(3) function. ("A\nB" gets encoded as 'A', '\\', 'n', 'B', not 'A', '\n', 'B'.) Unfortunately, this appears to be AT&T troff-compatible behavior. But it means that you _cannot_ portably construct multi-line replacement text for sed's 's' command. (Other sed commands like 'a', 'c', and 'i' will be similarly affected.) See Savannah #64071. * PROBLEMS: Drop item. Fixes <https://savannah.gnu.org/bugs/?64061>. Thanks to Bruno Haible for the report, and to him and Ralph Corderoy for the discussion of portable and efficient sed constructs. And for grins... commit 8de67a3bf1163b18ce8bfce54075fca8a6fd379b Author: G. Branden Robinson <g.branden.robinson@gmail.com> Date: Wed Apr 26 04:36:05 2023 -0500 [pdfpic]: Refactor (`sy` -> `pso`). * tmac/pdfpic.tmac: Migrate gathering of image dimensions from `sy` and a temporary file to `pso`. (pdfpic@cleanup): Drop `pdfpictemporary-file` string. (pdfpic@get-image-dimensions): Remove redirection. Invoke `pso`, not `sy`. (PDFPIC): Stop constructing `pdfpictemporary-file` string. Stop testing `systat` register. Stop sourcing and deleting temporary file. We keep the temporary directory handling because we will need it for the `PSPIC` fallback logic, but it also promises to be really painful to fix that before we have more formatter support for string traversal. See <https://savannah.gnu.org/bugs/index.php?64114>.	G. Branden Robinson <gbranden>
Sat 06 May 2023 09:45:28 PM UTC, comment #17: Hi Deri, > The switch from using grep to sed, which seems to have caused issues, Not exactly; it's more like it caused us to pay a lot more attention to something that was a bit jinky in the first place. Here's what the code looked like when Bernd added it in 2015 (after groff 1.22.3). +.\" get image dimensions +. ec @ +. sy pdfinfo @$1 \| \ +grep "Page size" \| \ +sed -e 's/Page size: \$[[:digit:].]\$ x \$[[:digit:].]\$.$/\ +.nr pdf-wid (p;\\1)\\n\ +.nr pdf-ht (p;\\2)/' \ +> /tmp/pdfpic\n[$$] +. so /tmp/pdfpic\n[$$] +. sy rm /tmp/pdfpic\n[$$] +. ec The piece that isn't portable sed(1) is this bit: .nr pdf-wid (p;\\1)\\n\ That '\\n' does not go into the shell command as an escape newline, but as the character sequence '\', 'n' (or, in C, '\\', 'n'), which is a GNU sed extension that the sed in macOS 12 also supports. But other seds don't. > given that the example pdf provided in bug #58206 was in fact an invalid pdf, which is why pdfinfo did not handle it correctly. That may be true, but I was able to reproduce the problem using conventional Linux tools, and did so in my regression test. https://git.savannah.gnu.org/cgit/groff.git/tree/tmac/tests/pdfpic_does-not-choke-on-bad-pdfinfo-output.sh?h=1.23.0.rc4#n68 ...unless you're saying that my contrived /Title annotation is similarly defective in lacking a byte order mark, which you might be. Maybe we should test both, since evidently a lot of PDFs have been produced by faulty tools that omitted the BOM. > So, if it makes things any easier we could go back to a simple grep, I'm afraid we can't. All this byte order mark business is a side issue from the mission of the PDFPIC macro's call to `sy`: to scrape the image dimensions out of pdfinfo(1)'s output into two separate *roff registers. Even if nobody ever produced PDFs with BOMs missing from their encoded text strings, or pdfinfo(1) never behaved badly (from our perspective) if they did, we'd still need to turn one line of pdfinfo output like this: Page size: 612 x 792 pts into valid (g)roff syntax--and that's going to take more than grep(1). Does this clear things up?	G. Branden Robinson <gbranden>
Sat 06 May 2023 08:30:35 PM UTC, comment #16: The switch from using grep to sed, which seems to have caused issues, may well have been unnecessary, given that the example pdf provided in bug #58206 was in fact an invalid pdf, which is why pdfinfo did not handle it correctly. The pdf reference says:- "For text strings encoded in Unicode, the first two bytes must be 254 followed by 255, representing the Unicode byte order marker, U+FEFF ." From the byte dump in that bug you can see no such BOM is present. Please also note that later versions of pdfinfo (checked with v. 0.26.4) now handles the mangled title correctly (must be recognising the alternating zero bytes and "guessing" it is UTF-16 with a missing BOM." So, if it makes things any easier we could go back to a simple grep, since if it fails we know it is a non-standard pdf and they are using an older version of pdfinfo.	Deri James <deri>
Fri 28 Apr 2023 07:26:46 PM UTC, comment #15: Fixed on branch. Still subject to change. commit 66b3077b7c265525ab72aa807e43047e5b86de8c Author: G. Branden Robinson <g.branden.robinson@gmail.com> Date: Mon Apr 17 16:41:33 2023 -0500 [pdfpic]: Fix Savannah #64061. * tmac/pdfpic.tmac: Refactor to make comprehensible some woefully undocumented cleverness and improve efficiency. (PDFPIC): Break out flaming-hoop-leaping "clever" bit of `sy` usage into its own macro, calling from here and relocating its requests from here... (pdfpic@system): ...to here. When using `sy` request to collect and munge output of pdfinfo(1), (a) disable the escape character while defining the macro; (b) construct the command in a roff string, appending to it in discrete, hopefully comprehensible chunks; (c) disable the escape character during macro interpretation wherever possible (most of it); (d) retain doubled backslashes so that they survive subsequent string interpolation; (e) stop using grep(1) in the pipeline when sed(1) is perfectly capable of performing its own input filtering; (f) invoke sed with '-n' option and emit output only upon a successful substitution; (g) replace unportable(!) POSIX BRE character class '[:digit:]' in substitution match text with '[0-9]'; and most importantly (h) replace multi-line sed 's' replacement text (see below for the reason we can't use it) with single roff control line employing the groff extension escape sequence `\R` to assign multiple registers. Annotate portability and escaping challenges. Tested on GNU/Linux, macOS 12, and (with simulated pdfinfo(1) output), on Solaris 11. There is a problem with trying to embed true newlines into the arguments of a `sy` request. The C++ function that GNU troff uses to assemble the command string (character by character) _does not recognize C/C++ string literal escape sequences_. This means that you _cannot_ embed "\n" in `sy`'s arguments and have it survive, as a newline character, into the command string passed to the standard C library's system(3) function. ("A\nB" gets encoded as 'A', '\\', 'n', 'B', not 'A', '\n', 'B'.) Unfortunately, this appears to be AT&T troff-compatible behavior. But it means that you _cannot_ portably construct multi-line replacement text for sed's 's' command. (Other sed commands like 'a', 'c', and 'i' will be similarly affected.) See Savannah #64071. * PROBLEMS: Drop item. Fixes <https://savannah.gnu.org/bugs/?64061>. Thanks to Bruno Haible for the report, and to him and Ralph Corderoy for the discussion of portable and efficient sed constructs.	G. Branden Robinson <gbranden>
Mon 24 Apr 2023 12:53:23 AM UTC, comment #14: comment #11: > comment #9: > True, but I thought you might have a setup that generated an expected temp file for the fake-pdfinfo input, which you could diff against mine, bypassing the eyeballs. Ah, no...that didn't occur to me. I don't really want to bypass eyeballs. Many brains are better than few. > Adding an output line to the temp file and groffing it also gives the same results on both Mac and Gentoo Linux: Good deal! comment #12: > comment #5: > > 1. Run pdfinfo twice and scrape out a different datum on each > > pass. I fear this would give up more than was gained by > > eliminating grep from the pipeline. > > Not that we need more options at this point, since two others now have working implementations, but I don't think this particular objection has much weight: the grep wasn't eliminated because it was incurring a noticeable performance penalty, but because it was deemed unnecessary on code inspection. I doubt the overhead of two invocations of pdfinfo would make a significant runtime difference even in a document that included dozens of PDFs. I think that's fair. The recovered efficiency is more significant in human brain cycles than CPU clock cycles.	G. Branden Robinson <gbranden>
Mon 24 Apr 2023 12:50:46 AM UTC, comment #13: Yet another realization has come to me. Since we're in unsafe mode anyway (duh, `sy`), why are we messing with a temporary file only to source it? This is what the groff `pso` request was made for. So I expect I'll grind on this a bit harder before its eventual merge to master (which won't happen until 1.23.0 final is tagged or Bertrand green-lights it as okay for the RC series).	G. Branden Robinson <gbranden>
Sun 23 Apr 2023 09:06:08 PM UTC, comment #12: comment #5: > 1. Run pdfinfo twice and scrape out a different datum on each > pass. I fear this would give up more than was gained by > eliminating grep from the pipeline. Not that we need more options at this point, since two others now have working implementations, but I don't think this particular objection has much weight: the grep wasn't eliminated because it was incurring a noticeable performance penalty, but because it was deemed unnecessary on code inspection. I doubt the overhead of two invocations of pdfinfo would make a significant runtime difference even in a document that included dozens of PDFs. Ultimately, though, this seems the least elegant of the three proposals, so even though it was mine initially, I'm happy to see it discarded.	Dave <barx>
Sun 23 Apr 2023 08:29:07 PM UTC, comment #11: comment #9: > I forgot to point you to my "private" branch on Savannah. Right. I did know about that branch, so I should have checked there. > It looks good; of course one of the the challenges with this sort of > cleverness is that it's easy for problems to escape the eyeballs. True, but I thought you might have a setup that generated an expected temp file for the fake-pdfinfo input, which you could diff against mine, bypassing the eyeballs. Adding an output line to the temp file and groffing it also gives the same results on both Mac and Gentoo Linux: $ echo '.tm pdfpicwidth \n[pdfpicwidth] pdfpicheight \n[pdfpicheight]' >> /tmp/pdfpic-temporary-file $ groff /tmp/pdfpic-temporary-file pdfpicwidth 612000 pdfpicheight 792000 $ nroff /tmp/pdfpic-temporary-file pdfpicwidth 2040 pdfpicheight 2640	Dave <barx>
Sat 22 Apr 2023 07:08:32 PM UTC, comment #10: comment #8: > comment #6: > > And because I couldn't resist testing my mad sed stunta > > skillz, here's a proof of concept. > > I got identical results to yours piping fake-pdfinfo into stunt.sed on OS X. So your stunt is even portable (to at least one of the outlier seds). As well it should be! This is old-school sed that should be portable everywhere. (He said, tempting fate.)	G. Branden Robinson <gbranden>
Sat 22 Apr 2023 07:07:25 PM UTC, comment #9: comment #7: > comment #5: > > Dave, can you give this a crack on your Mac OS X 10.11.6 host? > > I think I got it to work? > > You'll have to check my work, because I appear to be missing a step. I took the pdfpicsystem macro from the file "64061.diff" attached to http://lists.gnu.org/r/groff/2023-04/msg00190.html, but the patch in comment #5* is not against the same version of that macro; there appears to be an intermediate revision I'm missing. So I interpolated, hopefully correctly. Ah, yes, whoops. I forgot to point you to my "private" branch on Savannah. https://git.savannah.gnu.org/cgit/groff.git/commit/?h=branden-2023-04-20&id=4dc2c23f0c2b72517be62e60090f30b214299650 > First, I set up a small input file that is basically the "64061.diff" version of pdfpicsystem with a couple edits to (a) call your fake-pdfinfo script from comment #6, rather than pdfinfo itself, so that we know exactly what data we're working with, and (b) hard-code a path in place of \[pdfpictemporary-file]. All the test file does is define the macro and then invoke it: > $ cat pdfpic.test > .eo > .de pdfpicsystem > . ds pdfpiccommand ./fake-pdfinfo > . eo > . as pdfpiccommand " \| tr -d '\\000' > . as pdfpiccommand " \| sed -n -e '/Page size:/ > . as pdfpiccommand s/Page size: \$[0-9.]\$ x $[0-9.]\$.$/ > . as pdfpiccommand .nr pdfpicwidth (p;\\1)\\n > . as pdfpiccommand .nr pdfpicheight (p;\\2)/' > . as pdfpiccommand " -e tprint -e b -e :print -e p > . ec > . as pdfpiccommand " > /tmp/pdfpic-temporary-file > . sy \[pdfpiccommand] > . rm pdfpiccommand > .. > .ec > . > . pdfpicsystem > $ groff -U pdfpic.test > $ cat /tmp/pdfpic-temporary-file > .nr pdfpicwidth (p;612)n.nr pdfpicheight (p;792) > That didn't work and we knew it wouldn't, so no surprises there. Your testing approach seems sound to me. > Now's where I had to do a little guesswork to apply the patch. Here's how my version of pdfpic.patched.test differs from the original test: > $ diff -u pdfpic.test pdfpic.patched.test > --- pdfpic.test 2023-04-22 07:41:13.000000000 -0500 > +++ pdfpic.patched.test 2023-04-22 07:43:54.000000000 -0500 > @@ -5,9 +5,9 @@ > . as pdfpiccommand " \| tr -d '\\000' > . as pdfpiccommand " \| sed -n -e '/Page size:/ > . as pdfpiccommand s/Page size: \$[0-9.]\$ x $[0-9.]\$.$/ > -. as pdfpiccommand .nr pdfpicwidth (p;\\1)\\n > -. as pdfpiccommand .nr pdfpicheight (p;\\2)/' > -. as pdfpiccommand " -e tprint -e b -e :print -e p > +. as pdfpiccommand . \\\\R@pdfpicwidth (p;\\1)@ > +. as pdfpiccommand " \\\\R@pdfpicheight (p;\\2)@ > +. as pdfpiccommand /p' > . ec > . as pdfpiccommand " > /tmp/pdfpic-temporary-file > . sy \[pdfpiccommand] > $ groff -U pdfpic.patched.test > $ cat /tmp/pdfpic-temporary-file > . \R@pdfpicwidth (p;612)@ \R@pdfpicheight (p;792)@ > So, presuming that > 0 my patch is functionally equivalent to yours, and > 0 the contents of the temp file are what you expected > then it looks like we have a winner! It looks good; of course one of the the challenges with this sort of cleverness is that it's easy for problems to escape the eyeballs. > pdfpic.patched.test generates an identical temp file on my Linux box, so that's an encouraging sign. Very much so!	G. Branden Robinson <gbranden>
Sat 22 Apr 2023 01:47:34 PM UTC, comment #8: comment #6: > And because I couldn't resist testing my mad sed stunta > skillz, here's a proof of concept. I got identical results to yours piping fake-pdfinfo into stunt.sed on OS X. So your stunt is even portable (to at least one of the outlier seds). > I'm sure the FSF wouldn't appreciate me putting a "licensable" > photograph of a skateboarder's skull about to meet high-speed concrete, How about an ASCII art rendition?	Dave <barx>
Sat 22 Apr 2023 01:24:20 PM UTC, comment #7: comment #5: > Dave, can you give this a crack on your Mac OS X 10.11.6 host? I think I got it to work? You'll have to check my work, because I appear to be missing a step. I took the pdfpicsystem macro from the file "64061.diff" attached to http://lists.gnu.org/r/groff/2023-04/msg00190.html, but the patch in comment #5* is not against the same version of that macro; there appears to be an intermediate revision I'm missing. So I interpolated, hopefully correctly. First, I set up a small input file that is basically the "64061.diff" version of pdfpicsystem with a couple edits to (a) call your fake-pdfinfo script from comment #6, rather than pdfinfo itself, so that we know exactly what data we're working with, and (b) hard-code a path in place of \[pdfpictemporary-file]. All the test file does is define the macro and then invoke it: $ cat pdfpic.test .eo .de pdfpicsystem . ds pdfpiccommand ./fake-pdfinfo . eo . as pdfpiccommand " \| tr -d '\\000' . as pdfpiccommand " \| sed -n -e '/Page size:/ . as pdfpiccommand s/Page size: \$[0-9.]\$ x $[0-9.]\$.$/ . as pdfpiccommand .nr pdfpicwidth (p;\\1)\\n . as pdfpiccommand .nr pdfpicheight (p;\\2)/' . as pdfpiccommand " -e tprint -e b -e :print -e p . ec . as pdfpiccommand " > /tmp/pdfpic-temporary-file . sy \[pdfpiccommand] . rm pdfpiccommand .. .ec . . pdfpicsystem $ groff -U pdfpic.test $ cat /tmp/pdfpic-temporary-file .nr pdfpicwidth (p;612)n.nr pdfpicheight (p;792) That didn't work and we knew it wouldn't, so no surprises there. Now's where I had to do a little guesswork to apply the patch. Here's how my version of pdfpic.patched.test differs from the original test: $ diff -u pdfpic.test pdfpic.patched.test --- pdfpic.test 2023-04-22 07:41:13.000000000 -0500 +++ pdfpic.patched.test 2023-04-22 07:43:54.000000000 -0500 @@ -5,9 +5,9 @@ . as pdfpiccommand " \| tr -d '\\000' . as pdfpiccommand " \| sed -n -e '/Page size:/ . as pdfpiccommand s/Page size: \$[0-9.]\$ x $[0-9.]\$.$/ -. as pdfpiccommand .nr pdfpicwidth (p;\\1)\\n -. as pdfpiccommand .nr pdfpicheight (p;\\2)/' -. as pdfpiccommand " -e tprint -e b -e :print -e p +. as pdfpiccommand . \\\\R@pdfpicwidth (p;\\1)@ +. as pdfpiccommand " \\\\R@pdfpicheight (p;\\2)@ +. as pdfpiccommand /p' . ec . as pdfpiccommand " > /tmp/pdfpic-temporary-file . sy \[pdfpiccommand] $ groff -U pdfpic.patched.test $ cat /tmp/pdfpic-temporary-file . \R@pdfpicwidth (p;612)@ \R@pdfpicheight (p;792)@ So, presuming that my patch is functionally equivalent to yours, and the contents of the temp file are what you expected then it looks like we have a winner! pdfpic.patched.test generates an identical temp file on my Linux box, so that's an encouraging sign.	Dave <barx>
Sat 22 Apr 2023 02:39:16 AM UTC, comment #6: And because I couldn't resist testing my mad sed stunta skillz, here's a proof of concept. $ cat ATTIC/fake-pdfinfo #!/bin/sh cat <<EOF Creator: groff version 1.23.0.rc3.81-9e3e4 Producer: gropdf version 1.23.0.rc3.81-9e3e4 CreationDate: Sat Mar 25 18:43:18 2023 CDT ModDate: Sat Mar 25 18:43:18 2023 CDT Tagged: no UserProperties: no Suspects: no Form: none JavaScript: no Pages: 1 Encrypted: no Page size: 612 x 792 pts (letter) Page rot: 0 File size: 106745 bytes Optimized: no PDF version: 1.4 EOF $ cat ATTIC/stunt.sed /Page size:/s/Page size: //;h;s/$[0-9.]$ x $[0-9.]$.$/.nr pdfpicwidth (p;\1)/p;x;s/$[0-9.]$ x $[0-9.]$.$/.nr pdfpicheight (p;\2)/p $ ./ATTIC/fake-pdfinfo \| sed -n -f ATTIC/stunt.sed .nr pdfpicwidth (p;612) .nr pdfpic*height (p;792) It worked on the first try. Maybe I'm a less crap sed programmer than I thought. But at any rate I feel this is a bit lengthy, so I think I prefer solution 3, `\R`. I'm sure the FSF wouldn't appreciate me putting a "licensable" photograph of a skateboarder's skull about to meet high-speed concrete, so the reader will just have to imagine one.	G. Branden Robinson <gbranden>
Sat 22 Apr 2023 02:25:54 AM UTC, comment #5: As discussed on the list, there are a few tricks available to us. 1. Run pdfinfo twice and scrape out a different datum on each pass. I fear this would give up more than was gained by eliminating grep from the pipeline. 2. Perform a sed stunt involving the hold space. 3. Use groff's `\R` escape sequence to sneak two register assignments onto the same input line. Here's an illustration of the last. commit 0bad05526a72e63fd189babb166c0ad981370c42 (HEAD) Author: G. Branden Robinson <g.branden.robinson@gmail.com> Date: Fri Apr 21 20:16:21 2023 -0500 squash Fix Savannah #64061. (\R trick) diff --git a/tmac/pdfpic.tmac b/tmac/pdfpic.tmac index 024c10cd4..90a183774 100644 --- a/tmac/pdfpic.tmac +++ b/tmac/pdfpic.tmac @@ -77,8 +77,9 @@ . as pdfpiccommand " \| tr -d '\\000' . as pdfpiccommand " \| sed -n -e '/Page size:/ . as pdfpiccommand s/Page size: \$[0-9.]\$ x $[0-9.]\$.$/ -. as pdfpiccommand .nr pdfpicwidth (p;\\1)\\n -. as pdfpiccommand .nr pdfpicheight (p;\\2)/p' +. as pdfpiccommand . \\\\R@pdfpicwidth (p;\\1)@ +. as pdfpiccommand " \\\\R@pdfpicheight (p;\\2)@ +. as pdfpiccommand /p' . ec . as pdfpiccommand " > \[pdfpictemporary-file] . sy \[pdfpic*command] This works for me on Solaris 11 (running my "fake-pdfinfo" output generator instead of the real thing, which isn't installed). Dave, can you give this a crack on your Mac OS X 10.11.6 host?	G. Branden Robinson <gbranden>
Wed 19 Apr 2023 10:03:17 PM UTC, comment #4: It turns out we need the non-portability.	G. Branden Robinson <gbranden>
Mon 17 Apr 2023 05:33:00 PM UTC, comment #3: comment #1: > original submission: > > 2. It uses the sequence '\n' in the replacement part of a sed > > 's' command. This is non-standard and undefined on some seds. > > I raised this as a side note in comment 11 of bug 58206, but once that bug's primary issue was addressed, I didn't follow up on this point. So I apologize for dropping the ball there. It's okay. It turns out that the side note was misleading. But you could be forgive to draw the conclusion you did; I think almost anyone would. There's deviltry afoot, but I now have a cast of the offending hoofprint. > Some of the email threads linked elsewhere in that bug report point out the redundancy of the grep command, and offer other ways to skin that cat, such as an awk equivalent of the sed (which may or may not be more portable). Awk is also notorious for its number of dialects, and requires Autoconf checks. sed is practically always available and as far as I know there is a portable dialect of it that is still feature-complete as far as POSIX goes--you just have to be insanely careful. Often this seems to mean breaking lines at every opportunity.	G. Branden Robinson <gbranden>
Mon 17 Apr 2023 05:29:22 PM UTC, comment #2: It turns out some of the "excess" escaping isn't excess, and the "\n" is not only portable but also necessary. (Hint: sed never sees the "\n".) More on this when I finish proving things out and have a tested patch ready.	G. Branden Robinson <gbranden>
Mon 17 Apr 2023 05:25:19 PM UTC, comment #1: original submission: > 2. It uses the sequence '\n' in the replacement part of a sed > 's' command. This is non-standard and undefined on some seds. I raised this as a side note in comment 11 of bug 58206, but once that bug's primary issue was addressed, I didn't follow up on this point. So I apologize for dropping the ball there. Some of the email threads linked elsewhere in that bug report point out the redundancy of the grep command, and offer other ways to skin that cat, such as an awk equivalent of the sed (which may or may not be more portable).	Dave <barx>
Mon 17 Apr 2023 08:33:25 AM UTC, original submission: Affects groff 1.22.4. pdfpic.tmac has several problems. 1. It unnecessarily uses double backslashes in a single-quoted sed expression. Since the roff escape character has been changed (to '@'), this means that the doubled backslashes appear as-is to sed's input, and some seds don't support this non-standard usage. 2. It uses the sequence '\n' in the replacement part of a sed 's' command. This is non-standard and undefined on some seds. 3. It wastefully uses grep to filter the output when sed is readily capable of performing this task itself without the expense of launching a separate process in the pipeline. See recent discussion on the groff mailing list.	G. Branden Robinson <gbranden>

(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 deri (Posted a comment)

-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.

Please enter the title of George Orwell's famous dystopian book (it's a date):

Follow 6 latest changes.

Date	Changed by	Updated Field	Previous Value	Replaced by
2023-07-10	gbranden	Status	Ready for Merge	Fixed
		Open/Closed	Open	Closed
		Planned Release	None	1.24.0
2023-04-28	gbranden	Status	In Progress	Ready for Merge
2023-04-22	gbranden	Summary	pdfpic.tmac resorts to undocumented magic	pdfpic.tmac requires non-standard sed feature
2023-04-19	gbranden	Summary	pdfpic.tmac uses sed badly and non-portably	pdfpic.tmac resorts to undocumented magic

GNU roff - Bugs: bug #64061, pdfpic.tmac requires non-standard...

bug #64061: pdfpic.tmac requires non-standard sed feature

<a href="#postcomment">Post a Comment</a>

<a href="#discussion">Discussion</a>

<a href="#attached">Attached Files</a>

<a href="#dependencies">Dependencies</a>

<a href="#cc">Mail Notification Carbon-Copy List</a>

<a href="#votes">Votes</a>

<a href="#history">History</a>