bugGNU roff - Bugs: bug #64360, [PATCH] [gropdf] does not...

 
 

bug #64360: [PATCH] [gropdf] does not correctly handle white space after 'w' command

Submitter:  G. Branden Robinson <gbranden>
Submitted:  Tue 27 Jun 2023 03:17:09 PM UTC
   
 
Category:  Driver gropdf Severity:  3 - Normal
Item Group:  Incorrect behaviour Status:  Need Info
Privacy:  Public Assigned to:  deri
Open/Closed:  Open Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 15 Aug 2023 01:50:57 PM UTC, comment #33: 

Hi Deri,

comment #32:

> Again a long reply will be required since you've again extended the scope bringing in heirloom, dwb, libdriver. whereas i tried to limit the scope to examine whether gropdfs grout parser was cnformant to our current docs. You may consider our current documentation inadequate, i have no issue with it, and if you alter it and libdriver


You may have missed this island in my ocean of text:

> > Fortunately, I see no need to alter libdriver in any way


And this remains the case, despite my findings in comment #30.

> to do what you want, then gropdf will no longer be conformant, because you have changed the api it relied omn.


I am telling you that gropdf is not behaving congruently with grops today.  I don't need to change anything to make that statement true.  I mention libdriver only because that is factually where the troff output parser for all of groff's other output drivers lives.

I checked the behavior of Heirloom and DWB only to see if they might shed any light on interpretations of CSTR #54.  If you're like me, you can treat them as suggestive but not dispositive, particularly given their bad behavior as shown in comment #30.

So what remains is this:

> i tried to limit the scope to examine whether gropdfs grout parser was cnformant to our current docs. You may consider our current documentation inadequate, i have no issue with it,


Then you should be able to resolve the dilemma I raised in comment #29--a dilemma which arose from two competing claims of the current documentation, which I therefore do not regard as sufficient to characterize an API.

> > does a command with one fixed-length argument get followed by a line break or not?


If it does, then this groff 1.23.0 (or 1.22.4) output demands explanation.


$ printf '\\o@abc@\n' | groff -Tascii -Z | sed -n '12p'
cacbccH24


If it doesn't, then this sentence from our docs demands explanation.

> > > in 'gtroff''s intermediate output, every command with
> > > at least one argument is followed by a line break,


If you can tell me how the foregoing facts are not in tension, I'd appreciate hearing it.  If you cannot, then I submit to you that you are putting too much stock in our documentation, and that the implementation of libdriver, which has seen little change since 2006, should be given more weight.

> Im afraikd it will be sometime before i can give f ull reply, computer gone kaput,


That sucks.  I'm sorry to hear it.  :(

> new one built (son under  guidance), but teething issues so not quite customised yet. hand typing so evben slower than noprmal. :-(

G. Branden Robinson <gbranden>
Group administrator
Tue 15 Aug 2023 01:10:00 PM UTC, comment #32: 

Again a long reply will be required since you've again extended the scope bringing in heirloom, dwb, libdriver. whereas i tried to limit the scope to examine whether gropdfs grout parser was cnformant to our current docs. You may consider our current documentation inadequate, i have no issue with it, and if you alter it and libdriver to do what you want, then gropdf will no longer be conformant, because you have changed the api it relied omn.

Im afraikd it will be sometime before i can give f ull reply, computer gone kaput,  new one built (son under  guidance), but teething issues so not quite customised yet. hand typing so evben slower than noprmal. :-(

Deri James <deri>
Group Member
Tue 15 Aug 2023 12:40:58 PM UTC, comment #31: 

Forgot the attachment, as usual.



(file #55039)

G. Branden Robinson <gbranden>
Group administrator
Tue 15 Aug 2023 12:40:09 PM UTC, comment #30: 

How about something briefer?

comment #29:

> > I believe the change Branden has on his private branch is to output a new line after a w command, but this bug concerns white space after the w. Contrary to cstr#54, which classes space/tab/newline as the same, groff does not allow newline to be used as the white space between a command and its arguments (this difference is not documented).
>
> That's a good interesting point and one I want to explore with further testing.  I see no reason groff output drivers shouldn't accept a newline thus, given the clarity of CSTR #54's wording on the subject.


Findings:

Given the attached input, which exclusively uses newlines for separation:

1. grops rejects the input.


$ grops /tmp/branden/ps-grout3 > /tmp/branden/three.ps
grops:/tmp/branden/ps-grout3:1: error: missing argument
grops:/tmp/branden/ps-grout3:1: fatal error: the first command must be 'x T'


2. Heirloom dumps core.


$ ./bin/dpost /tmp/branden/ps-grout3 >| /tmp/branden/hthree.ps
Segmentation fault (core dumped)


3. DWB 3.3 troff gets upset with GNU extensions.  Its input line counter also gets completely out of whack.


$ DWBHOME=. ./bin/dpost /tmp/branden/ps-grout3 >| /tmp/branden/hthree.ps
./bin/dpost: unknown input character 155 m (line 11)
[drop the 'm' command]
$ DWBHOME=. ./bin/dpost /tmp/branden/ps-grout3 >| /tmp/branden/hthree.ps
./bin/dpost: unknown drawing function
 (line 11)
[drop the 'D f' command]
$ DWBHOME=. ./bin/dpost /tmp/branden/ps-grout3 >| /tmp/branden/hthree.ps
./bin/dpost: unknown input character 141 a (line 11)


I was able to determine that DWB 3.3 doesn't like newlines after 'c' commands or 'x' commands, but even after "fixing" those it kept throwing errors.  I didn't keep chasing it.

(But I am assured that if my input has a problem, it's on line 11.)

It seems everyone is sinful according to the gospel of CSTR #54...

I suggest splitting this issue off into a new ticket if anyone wants it fixed.

G. Branden Robinson <gbranden>
Group administrator
Tue 15 Aug 2023 12:14:29 PM UTC, comment #29: 

comment #27:

> First I'd like to try to reduce the scope of this discussion, since it seems to have grown in multiple directions.


Sure.
 

> Am I correct in the assumption that the grout files for any given input would not be identical when produced by different roff implementations?


Yes.  Here's some simple input given to Heirloom Doctools troff and then GNU troff.


$ printf -- '.nf\na b\n-\\-\n' | ./bin/troff -Tps
x T ps
x res 72000 1 1
x init
V0
p1
x font 1 R /home/branden/heirloom/lib/doctools/font/devps/R.afm 4
x font 2 I /home/branden/heirloom/lib/doctools/font/devps/I.afm 4
x font 3 B /home/branden/heirloom/lib/doctools/font/devps/B.afm 4
x font 4 BI /home/branden/heirloom/lib/doctools/font/devps/BI.afm 4
x font 5 CW /home/branden/heirloom/lib/doctools/font/devps/CW.afm 4
x font 6 H /home/branden/heirloom/lib/doctools/font/devps/H.afm 4
x font 7 HB /home/branden/heirloom/lib/doctools/font/devps/HB.afm 4
x font 8 HX /home/branden/heirloom/lib/doctools/font/devps/HX.afm 4
x font 9 S1 /home/branden/heirloom/lib/doctools/font/devps/S1.afm 516
x font 10 S /home/branden/heirloom/lib/doctools/font/devps/S.afm 1028
s10
f1
x X LC_CTYPE en_US.UTF-8
H72000
V12000
ca
wh7770cb
n12000 0
H72000
V24000
c-
h3330C\-
n12000 0
x trailer
V792000
x stop
$ printf -- '.nf\na b\n-\\-\n' | troff -Tps
x T ps
x res 72000 1 1
x init
p1
x font 5 TR
f5
s10000
V12000
H72000
md
DFd
ta
wh2500
tb
n12000 0
V24000
H72000
t-
C\-
h5640
n12000 0
x trailer
V792000
x stop


In the Heirloom output, I find the line


wh7770cb


noteworthy for a reason I'll return to.

> I assume this is true, else the output drivers from different implementations would be interchangeable. I don't believe they are.


I have a feeling that Kernighan might have been reaching for this, but didn't quite nail down the syntax tightly enough to strictly permit it.  However, I could be retrojecting my thoughts in 2023 onto his in 1980, when (a) there was no other device-independent troff implementation, (b) one was not going to appear for nearly a decade, and (c) given the challenges he described in CSTR #97, he might have thought it unlikely that another would ever be created.  As further speculation, (d) in 1980 the eventual divestiture of the AT&T monopoly was not yet seen as inevitable, and as long as that hope held, there wasn't much need for anyone to reimplement device-independent troff since it could be had for low or zero cost.  If anyone held that belief, it was the most swiftly overturned of these.
 

> This means, whilst they may all be based on the information in cstr#54, each roff has developed its own private API between the formatter and output drivers. For this reason the decision on whether this is a change to the groff version of the API, has to be confined to what is contained in the groff documentation.


Or we can accept that groff's documentation doesn't adequately describe its implementation, which I believe I just demonstrated in bug #63544.
 

> Empirical observation shows that groff uses a simple rule of one operation per line


No.  Glyph output and horizontal motions are frequently mixed when the `tcommand` directive is not present.  If we remove it from font/devps/DESC, we get this.


$ printf -- '.nf\na b\n-\\-\n' | ./build/test-groff -T ps -Z | grep cawh
cawh6940


And it is ubiquitous when the "obsolete" (to use Bernd's term) output command is used.  (I would term it "legacy".)


$ printf -- '.nf\na b\n-\\-\n' | groff -T X100 -Z | grep caw
caw10bh7


Further, (recalling the point I promised to return to above) Heirloom breaks this rule in yet another respect, putting a `c` command after an `h` command, whose argument is not fixed-width (the integers it uses are not zero-padded on the left) with no separation at all.

> and using a single space to avoid a "clashing between the command code and the arguments without the space.", even though Kernighan states that it is permitted to use newlines as well as a space and tab for this purpose, none of our drivers support this.


I believe I have empirically refuted this claim in bug #63544, comment #3.

> The w command is not an operation, it is just a marker for a paddable word space so following with a newline is against our own documentation:-


I find it unnecessary to bifurcate the class of "output commands" into "operations" and "non-operations".  It certainly isn't required to explain present (and, I have to guess, decades-long outstanding) behavior of libdriver.


> in 'gtroff''s intermediate output, every command with
> at least one argument is followed by a line break,


This is demonstrably false, as I showed above.


> thus providing
> excellent readability.


...and that's an unnecessary sales pitch.

> The w command has no arguments so under this rule it should not have a following new line.


I think you have extrapolated an invalid rule from vague and inaccurate documentation.

> As regards having a space after the w command, our documentation says:-


> The 'gtroff' output parser, however, is smart about whitespace by making it
> maximally optional.


> Which I take to mean it only uses a space to avoid the "clashes" mentioned above,


This is another sales pitch, and vague besides.

> and it further says:-


> Commands and arguments with a known, fixed length need
> not be separated by syntactical space.


It does say that.  Unfortunately this claim contradicts one you already quoted.


> in 'gtroff''s intermediate output, every command with
> at least one argument is followed by a line break,


According to the above, does a command with at one fixed-length argument get followed by a line break or not?

I don't think that question is answerable without relying on an additional information channel, like reading the source code or experimenting.

You may be beginning to see why I am critical of this documentation.

> The w command is fixed length, so to satisfy "maximally optional" no space is used.


As noted previously, I cannot elicit any clear semantics from the modifier "maximally" here.
 

> I never said that white space cannot follow a w command, but if we change to include white space after it then that goes against how the groff version of the API has been documented for many years.


That, I agree with.  Our documentation in this area is inaccurate.  Please understand that I am exercising restraint by not saying more.

> I believe the change Branden has on his private branch is to output a new line after a w command, but this bug concerns white space after the w. Contrary to cstr#54, which classes space/tab/newline as the same, groff does not allow newline to be used as the white space between a command and its arguments (this difference is not documented).


That's a good interesting point and one I want to explore with further testing.  I see no reason groff output drivers shouldn't accept a newline thus, given the clarity of CSTR #54's wording on the subject.

> If groff 1.23++ is going to use w followed by a new line, none of the proposed patches is optimal. A loop is no longer required, since no further commands will be on that line. There is no point in producing code to cater for situations which will not arise. Gropdf is written to parse grout output from groff, if that output is altered so that it no longer complies with our own documentation and gropdf fails to handle it then it is not a bug, but a change in the API and should involve a change request and at least a wider discussion than just us three.


I propose for gropdf to accept the same inputs grops does and to interpret them in a compatible way.  That is all.
 

> Apparently, the reason for wanting to make this change is to "generate "grout" that is more easily lexically analyzed". Citing posix shell's poor lexical processing capabilities, I don't see what difference wh2500 and w\nh2500 makes. If that's what you want, would a simple filter like this help:-


> [derij@pip busgrap]$ perl -pe 's/(.)(.*)/$1\n$2/ if m/^w/; s/^(.)(\S.*)/$1 $2/mg' zfile
> x T ps
> x res 72000 1 1
> x init
> p 1
> x font 5 TR
> f 5
> s 10000
> V 12000
> H 72000
> m d
> D Fd
> t Deri
> w
> h 2500
> D l 100000 0
> n 12000 0
> x trailer
> V 792000
> x stop


I don't think writing such a tool is desirable.  It "feels" too small to be a shippable tool--not supportive of its own weight in terms of making it a proper command with `--help` and `--version` and a man page, or the effort of coming up with a good name for it; too long to expect anyone to type it; and too obscure for it to make it into many people's shell start-up files as a function.

In my opinion, GNU troff should simply produce output that is easy for humans to read in the first place.

> In fact I'd be very happy to write a proper grout tool with multiple output options (pretty print, markup, XML). Markup could look like:-


> x T ps                        # for grops
> x res 72000 1 1
> x init
> p1                      # Page 1
> x font 5 TR
> f5                      # Times-Roman
> s10000                  # ps 10
> V12000                  # V 1/6th in
> H72000                  # H 1in
> md                      # Default text colour: Black
> DFd                     # Default fill colour: Black
> tDeri
> wh2500                  # Word Space: 2.5p
> Dl 100000 0             # Line from x,y to x1,y1
> n12000 0                # New Line
> x trailer
> V792000
> x stop


> Using some of the code from gropdf which keeps track of current position, XML output could tag the x,y position of every element.


That would indeed be useful--a "grout annotator" if you will.  But I think it's outside the scope of this ticket.

> So it is unnecessary to change the format of grout to achieve what you say you want.


As noted above and in bug #63544, I don't have to.  We just need to correct the documentation and align gropdf with the other output drivers here.
 

> The danger in changing the current grout format is we do not know what tools have been written which parse our current grout format, didn't someone write a parser which output html/javascript, how do we know our changes won't affect them.


If they accept what grops, grotty, grodvi, grohtml, and so forth do, then they'll be fine.  They do risk disruption if they relied upon our badly composed documentation in this area.  If I don't treat CSTR #54 as scripture as some of our mailing list subscribers do, I'm surely not going to bring a higher level of reverence to our own, particularly where it has demonstrable problems.

> Given that there are non-intrusive methods to achieve the result you want, I hope your hankerings can be satisfactorily assuaged.


I have to reject your conclusion here as ill-premised.  Fortunately, I see no need to alter libdriver in any way (pending the "newlines everywhere" research).  My tasks are to (1) revise our erroneous and unclear documentation and (2) assemble a patch for gropdf that you're willing to accept, assuming you lack the time or desire to do so yourself.

> AOB
>
> Many people have praised Branden for his contributions to the documentation,


I don't think you need two hands to count them.  :P

> as I do, it just felt wrong to see open criticism of a fellow contributers use of english. I am more than happy for Branden to make our documentation more "pellucid", but I think it is  nicer to do it without denigrating previous efforts which were made with the best intentions.


I have tried (I do not claim always successfully) to critique the code, not the person.  As I understand it, this is an aspect of egoless programming.  You may have observed Alex Colomar expressing a pretty low opinion of the `is_family_valid()` function on the groff list recently.  He may not have known at the time that I had written it.  While I was taken aback at first, I did not get upset with him, either on the mailing list or privately.  On the contrary, I largely agreed with his assessment; the code's form arose from an unfortunate constraint problem we have with our decision (which I guess I'm the main person driving) to stick to ISO C++98.

The same goes for documentation.  We all put a bit of ourselves into our written words, but the text is not the author.  Ingo Schwarze is another groff contributor who pulls no punches when expressing opinions of code.  But I don't remember seeing him engage in personal attacks.  I expect the same latitude when evaluating documentation, but I am also prepared to endure criticism of my own product.  All 3 of us can likely remember Ralph Corderoy's withering assessments of my emails (particularly their length).  He seemed to have difficulty believing that I could write concisely.  Whether he ever actually read any of the documentation I have written for groff (when it wasn't pitched to the mailing list first--a tiny proportion), I don't know--he never offered any evidence of having done so.  Ralph's unrelentingly negative attitude about groff and my work on it (in contrast to other *roffs) irritated me but I didn't, and don't, let that stop me from considering and crediting such contributions as he makes.  In other words, I can work with him.  If Bernd should return, I would expect to be able to work with him, too, and I hope he'd reciprocate that.

> The latest incarnation of gropdf (in the deri-gropdf-ng git branch, give it a go :-)) is now 80 lines short of 5000 lines.


So far your work is getting rave reviews.  I'm envious!  :D

G. Branden Robinson <gbranden>
Group administrator
Wed 12 Jul 2023 05:19:10 PM UTC, comment #28: 

Reassigning to self since Deri deserves a reply to comment #27 and that's on me.

G. Branden Robinson <gbranden>
Group administrator
Mon 10 Jul 2023 09:45:39 PM UTC, comment #27: 

 First I'd like to try to reduce the scope of this discussion, since it seems to have grown in multiple directions.

Am I correct in the assumption that the grout files for any given input would not be identical when produced by different roff implementations? I assume this is true, else the output drivers from different implementations would be interchangeable. I don't believe they are.

This means, whilst they may all be based on the information in cstr#54, each roff has developed its own private API between the formatter and output drivers. For this reason the decision on whether this is a change to the groff version of the API, has to be confined to what is contained in the groff documentation.

Empirical observation shows that groff uses a simple rule of one operation per line and using a single space to avoid a "clashing between the command code and the arguments without the space.", even though Kernighan states that it is permitted to use newlines as well as a space and tab for this purpose, none of our drivers support this.

The w command is not an operation, it is just a marker for a paddable word space so following with a newline is against our own documentation:-

in 'gtroff''s intermediate output, every command with
at least one argument is followed by a line break, thus providing
excellent readability.

The w command has no arguments so under this rule it should not have a following new line.

As regards having a space after the w command, our documentation says:-

The 'gtroff' output parser, however, is smart about whitespace by making it
maximally optional.

Which I take to mean it only uses a space to avoid the "clashes" mentioned above, and it further says:-

Commands and arguments with a known, fixed length need
not be separated by syntactical space.

The w command is fixed length, so to satisfy "maximally optional" no space is used.

I never said that white space cannot follow a w command, but if we change to include white space after it then that goes against how the groff version of the API has been documented for many years.

I believe the change Branden has on his private branch is to output a new line after a w command, but this bug concerns white space after the w. Contrary to cstr#54, which classes space/tab/newline as the same, groff does not allow newline to be used as the white space between a command and its arguments (this difference is not documented). If groff 1.23++ is going to use w followed by a new line, none of the proposed patches is optimal. A loop is no longer required, since no further commands will be on that line. There is no point in producing code to cater for situations which will not arise. Gropdf is written to parse grout output from groff, if that output is altered so that it no longer complies with our own documentation and gropdf fails to handle it then it is not a bug, but a change in the API and should involve a change request and at least a wider discussion than just us three.

Apparently, the reason for wanting to make this change is to "generate "grout" that is more easily lexically analyzed". Citing posix shell's poor lexical processing capabilities, I don't see what difference wh2500 and w\nh2500 makes. If that's what you want, would a simple filter like this help:-

[derij@pip busgrap]$ perl -pe 's/(.)(.*)/$1\n$2/ if m/^w/; s/^(.)(\S.*)/$1 $2/mg' zfile
x T ps
x res 72000 1 1
x init
p 1
x font 5 TR
f 5
s 10000
V 12000
H 72000
m d
D Fd
t Deri
w
h 2500
D l 100000 0
n 12000 0
x trailer
V 792000
x stop

In fact I'd be very happy to write a proper grout tool with multiple output options (pretty print, markup, XML). Markup could look like:-

x T ps                        # for grops
x res 72000 1 1
x init
p1                      # Page 1
x font 5 TR
f5                      # Times-Roman
s10000                  # ps 10
V12000                  # V 1/6th in
H72000                  # H 1in
md                      # Default text colour: Black
DFd                     # Default fill colour: Black
tDeri
wh2500                  # Word Space: 2.5p
Dl 100000 0             # Line from x,y to x1,y1
n12000 0                # New Line
x trailer
V792000
x stop

Using some of the code from gropdf which keeps track of current position, XML output could tag the x,y position of every element.

So it is unnecessary to change the format of grout to achieve what you say you want.

The danger in changing the current grout format is we do not know what tools have been written which parse our current grout format, didn't someone write a parser which output html/javascript, how do we know our changes won't affect them. Given that there are non-intrusive methods to achieve the result you want, I hope your hankerings can be satisfactorily assuaged.

AOB

Many people have praised Branden for his contributions to the documentation, as I do, it just felt wrong to see open criticism of a fellow contributers use of english. I am more than happy for Branden to make our documentation more "pellucid", but I think it is  nicer to do it without denigrating previous efforts which were made with the best intentions.

Dave, I don't use dasher for coding, just a keyboard, which is slower for input, but better for navigating around the document, cut and paste, controlling indentation and such like. For a maths exam at school they gave me an amanuensis, a very nice retired lady who spent the whole time doing a birdseye view of what her ideal home would look like. Gradually adding more and more detail with a multitude of coloured pencils (each had to be sharpened before use) right down to drawing individual roses in a regular pattern on the living room blue carpet - I was fascinated! Unfortunately she did not know how to draw an integral sign, so that took some explaining! This was before personal computers, oh for groff and eqn then.

The latest incarnation of gropdf (in the deri-gropdf-ng git branch, give it a go :-)) is now 80 lines short of 5000 lines.


Deri James <deri>
Group Member
Wed 05 Jul 2023 05:11:19 PM UTC, comment #26: 

comment #25:

> * My comment #22 was addressed to Dave, not you; please don't
> feel under any expectation to present a detailed response to it.


I'll also not respond in detail.  I agree with large portions of it, but more importantly, as I said at the end of comment #21, I consider the debate largely academic.  (Not entirely, as Branden points out, because there's a higher hurdle for enacting an API change, but I don't foresee any real opposition to that in practice, either, since it's CSTR #54 conforming.)

> comment #24:
> > Try:-
> >
> >   vlc http://chuzzlewit.co.uk/screen.mp4
>
> That's a lot of work.  :-|


Indeed, and writing code must be even more of a chore using that input method (slick as it is for what it does).  Consider me more impressed that the nearly 4000-line gropdf exists and is well written.

Dave <barx>
Group Member
Thu 29 Jun 2023 03:48:45 PM UTC, comment #25: 

comment #23:

> Please can you give me a few days to answer this. It will take me a lot of hours.


Certainly.  A few points to ease your response burden.  To make an analogy to legal filings, you don't have to reply to the entire brief.

  • My comment #22 was addressed to Dave, not you; please don't feel under any expectation to present a detailed response to it.


  • I did express my interpretations of some of your words in that message, but if those interpretations are incorrect I won't be bothered if you address only those.


  • My comment #4 and comment #19 are the code changes I'm specifically seeking your feedback on as gropdf author.  If you can give me an ACK or NAK on each of them, that would be the most expeditious course from a strict development perspective.



  • Time is not of the essence anyway.  This code lives in the working copy of a private branch on my dev box, and not in the public groff repo.  Even when I eventually push it, a private branch is where it will stay for a while as I figure out how I want to stage my changes back onto master, starting with known bug fixes and other uncontroversial changes.


  • When Bertrand tags 1.23.0 I will be preoccupied with matters other than this ticket for an indefinite amount of time.


comment #24:

> For your delectation I recorded me writing the last message, to give you a rough idea how long it will take.
>
> Try:-
>
>   vlc http://chuzzlewit.co.uk/screen.mp4
>
> I'd classify it as a PG rather than a U. :-)


Fortunately for you I lived in Australia for long enough to become familiar with the U.K. rating system (as well as Oz's).  :)  (My own vernacular tends toward black-label entertainment; I have to be a good boy on FSF-hosted infrastructure.)

That's a lot of work.  :-|  I also used to live in Indiana; one of its former governors, Roger Branigin, famously said, "I never argue with a man who buys ink by the barrel."  It may be your misfortune to have a debate opponent who moves it by the supertanker.

Consider me chagrined.

G. Branden Robinson <gbranden>
Group administrator
Wed 28 Jun 2023 08:50:40 PM UTC, comment #24: 

For your delectation I recorded me writing the last message, to give you a rough idea how long it will take.

Try:-

  vlc http://chuzzlewit.co.uk/screen.mp4

I'd classify it as a PG rather than a U. :-)


Deri James <deri>
Group Member
Wed 28 Jun 2023 08:20:15 PM UTC, comment #23: 

Please can you give me a few days to answer this. It will take me a lot of hours.

Deri James <deri>
Group Member
Wed 28 Jun 2023 05:07:29 PM UTC, comment #22: 

comment #21:

> A fair point, but it only addresses my second clause about CSTR #54.  The more relevant question is, do we claim CSTR #54 as a specification for groff, aside from such noted deviations?


Not per se.  I reckon the following, from roff(7), is the closest thing we make to a claim along those lines.


    Ossanna documented the syntax of the input language to the nroff and
    troff programs in the "Troff User's Manual", first published in
    1976, with further revisions as late as 1992 by Kernighan.  (The
    original version was entitled "Nroff/Troff User's Manual", which may
    partially explain why roff practitioners have tended to refer to it
    by its AT&T document identifier, "CSTR #54".)  Its final revision
    serves as the de facto specification of AT&T troff, and all
    subsequent implementors of roff systems have done so in its shadow.


While I don't think we're bound to CSTR #54 with adamantium shackles, at the same time I don't think we should do things that it implicitly prohibits without good reason.  That would create support grief for us and our users.

>  (Maybe we do; you know the groff docs better than I do.)  Because if groff's own docs don't address this whitespace in grout, and groff code in its history has never placed whitespace here, one could argue this constitutes an API change within the groff ecosystem.


For many years, groff has documented this area of its behavior in the groff_out(5) man page.

Unfortunately, I personally find that document (as of groff 1.22.4) less than pellucid.  The last time I complained about the quality of the writing there, Deri interpreted it as a dig at a former contributor's non-native English, which kind of leaves me in something of a bind with respect to further critique.

(I did express myself sloppily.  I said "non-standard" instead of "sub-standard" [not in itself a change likely to improve matters], but more importantly left unstated whether I was referring to the English per se or the technical writing.  As a rule, I find the contributor in question to write English of satisfactory syntax and vocabulary.  My frustrations arise from its frequent failures, in my opinion, to adequately and economically communicate technical information.  But this, too, could be interpreted as a personal derogation, and if so, I remain constrained from conducting peer review to my own standards.  Our former maintainer Werner Lemberg on multiple occasions expressed fairly strong critiques of the same contributor's work [citations available upon request], so I am frustrated to receive scolding for doing the same.)

To get back to concrete matters, here's what the page says in 1.22.4.  I have not yet undertaken a deep revision of this material.


   Separation
       Classical troff output had strange requirements on whitespace.  The
       groff output parser, however, is smart about whitespace by making it
       maximally optional.  The whitespace characters, i.e., the tab, space,
       and newline characters, always have a syntactical meaning.  They are
       never printable because spacing within the output is always done by
       positioning commands.

       Any sequence of space or tab characters is treated as a single
       syntactical space.  It separates commands and arguments, but is only
       required when there would occur a clashing between the command code and
       the arguments without the space.  Most often, this happens when
       variable length command names, arguments, argument lists, or command
       clusters meet.  Commands and arguments with a known, fixed length need
       not be separated by syntactical space.

       A line break is a syntactical element, too.  Every command argument can
       be followed by whitespace, a comment, or a newline character.  Thus a
       syntactical line break is defined to consist of optional syntactical
       space that is optionally followed by a comment, and a newline
       character.

       The normal commands, those for positioning and text, consist of a
       single letter taking a fixed number of arguments.  For historical
       reasons, the parser allows stacking of such commands on the same line,
       but fortunately, in groff intermediate output, every command with at
       least one argument is followed by a line break, thus providing
       excellent readability.

       The other commands -- those for drawing and device controlling -- have
       a more complicated structure; some recognize long command names, and
       some take a variable number of arguments.  So all D and x commands were
       designed to request a syntactical line break after their last argument.
       Only one command, `x X' has an argument that can stretch over several
       lines, all other commands must have all of their arguments on the same
       line as the command, i.e., the arguments may not be split by a line
       break.

       Empty lines, i.e., lines containing only space and/or a comment, can
       occur everywhere.  They are just ignored.


I'm attaching a screenshot of CSTR #54.  In it you will see, after a listing of the output commands, the statement by Kernighan that "Blanks, tabs, and newlines may occur as separators in the input, and are mandatory to separate constructions that would otherwise be confused."

That's not as rigorous as it could be, but I find it clearer and certainly more economical than our own documentation.

The question for the purpose of this ticket is whether a newline is permissible as a "separator[] in the input" with respect to the 'w' command as with any other, or if it is not.

Deri appears to be claiming not simply the latter, but that such separation is prohibited when following the 'w' command, and I'm damned if I can find any support for that view in either of the above.

I find it difficult to infer a prohibition from groff_out(5)'s phrase "maximally optional".
 

> For the record, I'm not making an argument about this either way, just trying to see it from multiple sides.


Fair.  Maybe you can locate a perspective that un-damns me.
 

> > I think our output drivers should behave consistently,
>
> True, and they do behave consistently given the grout that groff has produced to date.


This statement is not correct.  gropdf does not behave consistently with  grops (and our other output drivers) with respect to inputs involving white space after 'w' commands, and that is why I titled this ticket as I did.

GNU or macOS sed is required for the following.


$ echo 'A B' | groff -Tps -Z | sed 's/^w/&\n/' | grops | okular -
$ echo 'A B' | groff -Tps -Z | sed 's/^w/& /' | grops | okular -
$ echo 'A B' | groff -Tps -Z | sed 's/^w/&#comment\n/' | grops | okular -
$ echo 'A B' | groff -Tpdf -Z | sed 's/^w/&\n/' | gropdf | okular -
substr outside of string at /usr/bin/gropdf line 321, <STDIN> line 13.
Use of uninitialized value $lin in substitution (s///) at /usr/bin/gropdf line 325, <STDIN> line 13.
$ echo 'A B' | groff -Tpdf -Z | sed 's/^w/& /' | gropdf | okular -
$ echo 'A B' | groff -Tpdf -Z | sed 's/^w/&#comment\n/' | gropdf | okular -


There are three use cases above, each run through grops and gropdf (from groff 1.22.4) for a total of six commands.

Case 1 puts a newline after 'w'.  gropdf renders the output correctly but provokes Perl diagnostics.

Case 2 puts a space after 'w'.  gropdf renders the output incorrectly, ignoring the subsequent 'h' command and formatting 'AB' with no space between 'A' and 'B'

Case 3 puts a comment after 'w'.  A comment in troff output must be terminated with a newline to prevent misinterpretation.  gropdf handles this correctly and I have no complaint about its behavior in this case.

>  My reference to Postel's law was intended to point out that libgroff being coded to liberally accept "grout" from other troffs (generically, "rout"?) doesn't necessarily indicate an intention on groff's part to support producing such grout.


I think that begins to wander afield from the point of this bug.  While I don't take Postel's Law as the categorical imperative that some do, and in fact my inclinations lean against it, the point at issue here is a divergence of troff output stream interpretation between output drivers that groff provides.  This is not a question of dealing with Joe's Fly-by-Night Half-Assed Formatter.

I think following Postel in this situation is sound advice because we have all learned from the many people aggrieved by Stuart Feldman's make(1) that applying syntactical important syntactical distinctions to different kinds of white space is a hazardous practice, particularly for input formats that do not closely resemble discursive human communication.

> > and if someone were to convert PostScript generated by grops
> > to PDF and PDF generated by gropdf to PostScript, one should
> > not be able to tell which output driver originally produced the
> > document (ignoring comments within the files announcing the fact).
>
> I agree with that, but I'm not sure I follow your logic, as the above seems to be about the output of the two postprocessors, whereas the API question is about their input.


I hope that I have illuminated the subject adequately.

> Anyway, this debate is academic: ultimately, I favor making grout more usable by other tools (and more readable to humans), and whitespace is important for that, regardless of whether one characterizes it as an API change.


I appreciate that, but I am alarmed at the charge of an "API change" because a code alteration receiving that label is generally regarded as highly disruptive and demanding of a far higher level of scrutiny.

I hope that the use cases illustrated above (and in the tar attachment from comment #19, as it happens) constitute decisive evidence against such a claim.


G. Branden Robinson <gbranden>
Group administrator
Wed 28 Jun 2023 03:54:29 PM UTC, comment #21: 

comment #17:

> Where we deviate from CSTR #54, we say so.


A fair point, but it only addresses my second clause about CSTR #54.  The more relevant question is, do we claim CSTR #54 as a specification for groff, aside from such noted deviations?  (Maybe we do; you know the groff docs better than I do.)  Because if groff's own docs don't address this whitespace in grout, and groff code in its history has never placed whitespace here, one could argue this constitutes an API change within the groff ecosystem.

For the record, I'm not making an argument about this either way, just trying to see it from multiple sides.

> I think our output drivers should behave consistently,


True, and they do behave consistently given the grout that groff has produced to date.  My reference to Postel's law was intended to point out that libgroff being coded to liberally accept "grout" from other troffs (generically, "rout"?) doesn't necessarily indicate an intention on groff's part to support producing such grout.

> and if someone were to convert PostScript generated by grops
> to PDF and PDF generated by gropdf to PostScript, one should
> not be able to tell which output driver originally produced the
> document (ignoring comments within the files announcing the fact).


I agree with that, but I'm not sure I follow your logic, as the above seems to be about the output of the two postprocessors, whereas the API question is about their input.

Anyway, this debate is academic: ultimately, I favor making grout more usable by other tools (and more readable to humans), and whitespace is important for that, regardless of whether one characterizes it as an API change.

Dave <barx>
Group Member
Wed 28 Jun 2023 02:54:36 PM UTC, comment #20: 

comment #19:

>         my $lin=substr($_,1);
> +       $lin=~s/^\s*//;


One nit: as long as you're already unleashing a regex, you may as well use it to also perform the edit substr() is doing, and remove that function call.

       my $lin=$_;
       $lin=~s/^.\s*//;

Similar code appears a few lines down as well and can be similarly combined (this time saving a line of code because you can eliminate an assignment).

Dave <barx>
Group Member
Wed 28 Jun 2023 01:17:50 PM UTC, comment #19: 

Here's an updated version of my proposed patch for this ticket.  It assumes the one from comment #4 is already applied, and will need manual tweaking otherwise.


diff --git a/ChangeLog b/ChangeLog
index 373fab6df..f0a12b807 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2023-06-28  G. Branden Robinson <g.branden.robinson@gmail.com>
+
+       * src/devices/gropdf/gropdf.pl: Accept newlines after 'w'
+       commands as our libdriver-using output drivers do.  ("Blanks,
+       tabs, and newlines may occur as separators in the input, and are
+       mandatory to separate constructions that would otherwise be
+       confused." --CSTR #54 (1992))  Also, when debugging, write any
+       'w' commands encountered as comments to the output stream, as is
+       done with other commands.
+
+       Fixes <https://savannah.gnu.org/bugs/?64360>.
+
 2023-06-27  G. Branden Robinson <g.branden.robinson@gmail.com>

        * src/devices/gropdf/gropdf.pl: Fix code style nits.  Rename
diff --git a/src/devices/gropdf/gropdf.pl b/src/devices/gropdf/gropdf.pl
index 074a725c5..e86644da5 100644
--- a/src/devices/gropdf/gropdf.pl
+++ b/src/devices/gropdf/gropdf.pl
@@ -369,16 +369,17 @@ while (<>)
        my $cmd=substr($_,0,1);
        next if $cmd eq '#';    # just a comment
        my $lin=substr($_,1);
+       $lin=~s/^\s*//;

-       while ($cmd eq 'w')
+       # 'w' is the only zero-argument command.
+       while ($lin and $cmd eq 'w')
        {
+           $stream.="\% w\n" if $debug;
+           $w_flg=1 if $t_flg;
            $cmd=substr($lin,0,1);
            $lin=substr($lin,1);
-           $w_flg=1 if $t_flg;
+           $lin=~s/^\s*//;
        }
-
-       $lin=~s/^\s+//;
-#              $lin=~s/\s#.*?$//;      # remove comment
        $stream.="\% $_\n" if $debug;

        do_x($lin),next if ($cmd eq 'x');


I'm also attaching a tar archive of test cases I used to develop and check the logic.  It's a set of 6 specimens of GNU troff output (hand-modified in 5 cases to exercise the latitude afforded by CSTR #54), and the patched-gropdf-generated PDFs, all of which produce correct output as far as I can tell.

Annotating with "[PATCH]".  Assigning to Deri.  Is this an acceptable change?

(file #54892)

G. Branden Robinson <gbranden>
Group administrator
Wed 28 Jun 2023 12:33:07 PM UTC, comment #18: 

If it's a bug for a 'w' command to be missing when it should be there, then it appears my change in comment #8 is fixing one.

I'm not sure what the root-cause is, but I saw the following when updating my test case expectations to accommodate it.


diff --git a/tmac/tests/doc_heading-font-remapping-works.sh b/tmac/tests/doc_heading-font-remapping-works.sh
index 27f51964b..b033cd46a 100755
--- a/tmac/tests/doc_heading-font-remapping-works.sh
+++ b/tmac/tests/doc_heading-font-remapping-works.sh
@@ -34,25 +34,27 @@ input='.Dd 2022-12-26
 .Dt foo 1
 .Os "groff test suite"
 .Sh Name
 .Nm foo
 .Nd frobnicate a bar
 .Sh Hacking Xr groff
 Have fun!'

 output=$(printf "%s\n" "$input" | "$groff" -mdoc -Tascii -Z)
 echo "$output"

 echo "$output" | sed -n '/tHacking/{n
+/w/{n
 /x font 4 BI/{n
 /f4/{n
 /h/{n
 /tgroff/{n
 /n/{n
 /f1/p;}
 }
 }
 }
 }
+}
 }' | grep -Fqx f1

 # vim:set ai et sw=4 ts=4 tw=72:


I trust I will not draw contradiction when I claim that there is definitely a word space between "Hacking" and "groff", given the input.  (Perhaps this actually arises from the other change, the one in comment #5; mdoc may be putting unbreakable spaces in section headings, or maybe in cross reference link text.  If so, I'm not sure either choice is categorically right, but I'll fry that fish some other time.)

Just a data point.


G. Branden Robinson <gbranden>
Group administrator
Wed 28 Jun 2023 10:50:42 AM UTC, comment #17: 


comment #16:

> comment #15:
> > I strongly object to your characterization of this as a
> > proposed API change.  It is not.  It honors the letter and
> > spirit of CSTR #54,
>
> If the layout of groff's output language hasn't changed in 30+ years, there's arguably a de facto API separate from the published one.  For that matter, groff didn't publish CSTR #54 and deviates from it in various ways.  So whether it is or isn't an API change may depend on which angle you're looking at it from.


Where we deviate from CSTR #54, we say so.  (And we don't, very much.  More often, we document errata in it.)

> > and groff's other output drivers are not tripped up by the change,
>
> A good dose of Postel's law is often advisable.


I think this ticket is entirely in the same spirit as bug #63757, bug #62950, bug #62934, bug #62923, bug #62346, bug #62294, bug #61908, and bug #51568, all of which contrasted gropdf's behavior with grops (or, in one case, could have done).

I think our output drivers should behave consistently, and if someone were to convert PostScript generated by grops to PDF and PDF generated by gropdf to PostScript, one should not be able to tell which output driver originally produced the document (ignoring comments within the files announcing the fact).

I am baffled as to why achieving parity and consistency in this respect is being tarred with the "API change" brush.

G. Branden Robinson <gbranden>
Group administrator
Wed 28 Jun 2023 01:29:47 AM UTC, comment #16: 

comment #15:

> I strongly object to your characterization of this as a
> proposed API change.  It is not.  It honors the letter and
> spirit of CSTR #54,


If the layout of groff's output language hasn't changed in 30+ years, there's arguably a de facto API separate from the published one.  For that matter, groff didn't publish CSTR #54 and deviates from it in various ways.  So whether it is or isn't an API change may depend on which angle you're looking at it from.

> and groff's other output drivers are not tripped up by the change,


A good dose of Postel's law is often advisable.

Dave <barx>
Group Member
Tue 27 Jun 2023 07:59:00 PM UTC, comment #15: 


comment #14:

> Exactly, it is not a word gap its a horizontal move.


It seems we are in agreement.  But that is not an argument against this ticket.
 

> Please can you explain the practical benefit for this change.


Please review bug #63544, which I linked to earlier.  I want to make it easier for a wider variety of tools to parse GNU troff output, namely including shell scripts, which (under POSIX shell, at rate) have fairly clumsy character-based lexical processing capabilities.

> Perhaps on the list given that it is a change the current api between groff and the output drivers.


We can discuss it on the list but I strongly object to your characterization of this as a proposed API change.  It is not.  It honors the letter and spirit of CSTR #54, and groff's other output drivers are not tripped up by the change, which suggests that it this troff output is well within James Clark's expectations.

Here is an example of your output from comment #12 after this change.


$ echo "A Berty" | groff -Tpdf -fU-T -Z
x T pdf
x res 72000 1 1
x init
p1
x font 5 U-TR
f5
s10000
V12000
H72000
md
DFd
tA
w
h2500
tBer
h230
tty
n12000 0
x trailer
V792000
x stop


Since Perl can easily do multi-line matching (though you certainly don't have to use it), I am not understanding what about the above you find objectionable.

G. Branden Robinson <gbranden>
Group administrator
Tue 27 Jun 2023 07:41:22 PM UTC, comment #14: 

Exactly, it is not a word gap its a horizontal move.

Please can you explain the practical benefit for this change. Perhaps on the list given that it is a change the current api between groff and the output drivers.

Deri James <deri>
Group Member
Tue 27 Jun 2023 07:23:56 PM UTC, comment #13: 

Thanks.  Mmmm, crunchy.

But the presence of an 'h' command after 't' (or 'N' or 'C' or a legacy move-and-print command) does not necessarily imply kerning.

It indicate motion to the next tab stop.


$ printf 'A\tB\n' | ./build/test-groff -Z
x T ps
x res 72000 1 1
x init
p1
x font 5 TR
f5
s10000
V12000
H72000
md
DFd
tA
h28780
tB
n12000 0
x trailer
V792000
x stop


G. Branden Robinson <gbranden>
Group administrator
Tue 27 Jun 2023 06:11:21 PM UTC, comment #12: 

Concrete provided:-

[derij@pip build (new-gropdf)]$ echo "A Berty" | groff -Tpdf -fU-T -Z
x T pdf
x res 72000 1 1
x init
p1
x font 5 U-TR
f5
s10000
V12000
H72000
md
DFd
tA
wh2500
tBer
h230
tty
n12000 0
x trailer
V792000
x stop

Using the U- foundry because it has a lot more kern pairs. The wh2500 is the word gap, the h230 is a kern adjustment between r and t.

Deri James <deri>
Group Member
Tue 27 Jun 2023 05:48:59 PM UTC, comment #11: 


comment #10:

> Yes, I have seen multiple "w", but never one followed by a space.


I've never seen multiple 'w' commands in sequence without some other command in between.

I can't remember if I've seen 'w' followed by a space.
 

> I thought commands and arguments with a known fixed length (such as the w command) aren't separated by syntactical space. Well, that was true until now.


As far as I know that's never been true, but it seems that AT&T troff didn't often avail itself of optional white space.  CSTR #97 suggests that there was some pressure to make the formatter's output as low in byte count as possible.
 

> Previously, before you added /n,


Well, it's not exactly live code where anyone can get to it but me.

> a w is always followed by another command, which is what my code expects.


It certainly should always be followed by another command, even if it's "x trailer".  The question is whether it can be followed by white space, and CSTR #54 seems unequivocal about that.

You can see an example of what I think is a Kernighan parser at the following.

https://github.com/n-t-roff/DWB3.3/blob/master/postscript/dpost/dpost.c#L723

> I can't see a reason for the change, it is actually helpful, when looking at troff output, to quickly distinguish word spaces whnnn and kerning hnnn.


Can you show me a concrete example of what you mean?

> I have not seen your discussion on changing this on the list, I did see your suggestion about adding a word mark to \~ but not changing the syntax of the file produced by troff.


It's an older ticket, from December.

https://savannah.gnu.org/bugs/?63544

G. Branden Robinson <gbranden>
Group administrator
Tue 27 Jun 2023 05:29:03 PM UTC, comment #10: 

Yes, I have seen multiple "w", but never one followed by a space.

I thought commands and arguments with a known fixed length (such as the w command) aren't separated by syntactical space. Well, that was true until now.

Previously, before you added /n, a w is always followed by another command, which is what my code expects. I can't see a reason for the change, it is actually helpful, when looking at troff output, to quickly distinguish word spaces whnnn and kerning hnnn.

I have not seen your discussion on changing this on the list, I did see your suggestion about adding a word mark to \~ but not changing the syntax of the file produced by troff.

Deri James <deri>
Group Member
Tue 27 Jun 2023 04:56:40 PM UTC, comment #9: 

Yes, I have seen multiple "w", but never one followed by a space, can you work out why?

I thought commands and arguments with a known fixed length (such as the w command) aren't separated by syntactical space. Well, that was true until now.

Deri James <deri>
Group Member
Tue 27 Jun 2023 04:40:38 PM UTC, comment #8: 


comment #3:

> How did you get groff to plant the space?


Whoops, I was momentarily smoking crack, and didn't answer the question you asked.

I didn't hack GNU troff to produce a space here, but I did change it to produce a newline, and as you will see it is obvious enough.


diff --git a/src/roff/troff/node.cpp b/src/roff/troff/node.cpp
index 341edce1f..fe18426e0 100644
--- a/src/roff/troff/node.cpp
+++ b/src/roff/troff/node.cpp
@@ -960,7 +960,7 @@ inline void troff_output_file::word_marker()
 {
   flush_tbuf();
   if (is_on())
-    put('w');
+    put("w\n");
 }

 inline void troff_output_file::right(hunits n)


I had to stash that change because it made gropdf angry, and that's what led to the current ticket.

G. Branden Robinson <gbranden>
Group administrator
Tue 27 Jun 2023 04:14:19 PM UTC, comment #7: 


comment #5:

>
> comment #3:
> > Just move this line:-
> >
> >         $lin=~s/^\s+//;
> >
> > to before the while loop.
>
> Oh.  This is what I came up with.  Is it wrong?


I guess it is.  I didn't understand you while loop.  You're prepared for 'wwwwwww'.

G. Branden Robinson <gbranden>
Group administrator
Tue 27 Jun 2023 04:13:05 PM UTC, comment #6: 

Sending back to you for comment.

G. Branden Robinson <gbranden>
Group administrator
Tue 27 Jun 2023 04:12:52 PM UTC, comment #5: 


comment #3:

> Just move this line:-
>
>         $lin=~s/^\s+//;
>
> to before the while loop.


Oh.  This is what I came up with.  Is it wrong?


diff --git a/src/devices/gropdf/gropdf.pl b/src/devices/gropdf/gropdf.pl
index 074a725c5..200221ce4 100644
--- a/src/devices/gropdf/gropdf.pl
+++ b/src/devices/gropdf/gropdf.pl
@@ -370,14 +370,14 @@ while (<>)
        next if $cmd eq '#';    # just a comment
        my $lin=substr($_,1);

-       while ($cmd eq 'w')
+       # 'w' is the only zero-argument command.
+       if ($cmd eq 'w')
        {
+           $w_flg=1 if $t_flg;
+           $lin=~s/^\s*//;
            $cmd=substr($lin,0,1);
            $lin=substr($lin,1);
-           $w_flg=1 if $t_flg;
        }
-
-       $lin=~s/^\s+//;
 #              $lin=~s/\s#.*?$//;      # remove comment
        $stream.="\% $_\n" if $debug;

 

> How did you get groff to plant the space?



diff --git a/src/roff/troff/node.cpp b/src/roff/troff/node.cpp
index 152dfda1b..341edce1f 100644
--- a/src/roff/troff/node.cpp
+++ b/src/roff/troff/node.cpp
@@ -5828,6 +5828,7 @@ int word_space_node::is_tag()
 void unbreakable_space_node::tprint(troff_output_file *out)
 {
   out->fill_color(col);
+  out->word_marker();
   if (is_html) {
     // we emit the space width as a negative glyph index
     out->flush_tbuf();


G. Branden Robinson <gbranden>
Group administrator
Tue 27 Jun 2023 04:10:28 PM UTC, comment #4: 

I'd like to propose a small code cleanup.


diff --git a/ChangeLog b/ChangeLog
index bc268e1f3..373fab6df 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2023-06-27  G. Branden Robinson <g.branden.robinson@gmail.com>
+
+       * src/devices/gropdf/gropdf.pl: Fix code style nits.  Rename
+       global scalar `gotT` to `t_flg` to align with other similar
+       flags `w_flg` and `t_flg`.  Document these where they are
+       initialized instead of later in the code.
+
 2023-06-27  G. Branden Robinson <g.branden.robinson@gmail.com>

        * src/roff/troff/node.cpp (unbreakable_space_node::tprint):
diff --git a/src/devices/gropdf/gropdf.pl b/src/devices/gropdf/gropdf.pl
index c65a1051f..074a725c5 100644
--- a/src/devices/gropdf/gropdf.pl
+++ b/src/devices/gropdf/gropdf.pl
@@ -109,10 +109,10 @@ my $fontchg=0;    # font change pending
 my $tnum=2;    # flatness of B-Spline curve
 my $tden=3;    # flatness of B-Spline curve
 my $linewidth=40;
-my $w_flg=0;
+my $w_flg=0;   # 'w' seen since last 't'
 my $nomove=0;
 my $pendmv=0;
-my $gotT=0;
+my $t_flg=0;   # 't' seen since last 'n'
 my $suppress=0; # Suppress processing?
 my %incfil;     # Included Files
 my @outlev=([0,undef,0,0]);     # Structure pdfmark /OUT entries
@@ -130,7 +130,7 @@ my %missing;    # fonts in download files which are not found/readable



-my $n_flg=1;
+my $n_flg=1;        # 't' seen since last 'n'
 my $pginsert=-1;    # Growth point for kids array
 my %pgnames;        # 'names' of pages for switchtopage
 my @outlines=();    # State of Bookmark Outlines at end of each page
@@ -374,7 +374,7 @@ while (<>)
        {
            $cmd=substr($lin,0,1);
            $lin=substr($lin,1);
-           $w_flg=1 if $gotT;
+           $w_flg=1 if $t_flg;
        }

        $lin=~s/^\s+//;
@@ -3708,7 +3708,6 @@ sub do_t

     # $pendmv = 'h' move since last 't'
     # $nomove = width of char(s) added by 'C', 'N' or 'c'
-    # $w-flg  = 'w' seen since last t

     if ($fontchg)
     {
@@ -3717,7 +3716,7 @@ sub do_t
        $stream.="/F$cft $cftsz Tf\n", $fontchg=0 if $fontchg && defined($cft);
     }

-    $gotT=1;
+    $t_flg=1;

     $stream.="% --- wht=$whtsz, pend=$pendmv, nomv=$nomove\n" if $debug;

@@ -3911,7 +3910,7 @@ sub do_N

 sub do_n
 {
-    $gotT=0;
+    $t_flg=0;
     PutLine(0);
     $pendmv=$nomove=0;
     $n_flg=1;


G. Branden Robinson <gbranden>
Group administrator
Tue 27 Jun 2023 04:05:07 PM UTC, comment #3: 

Just move this line:-

        $lin=~s/^\s+//;

to before the while loop.

How did you get groff to plant the space?

Deri James <deri>
Group Member
Tue 27 Jun 2023 03:28:13 PM UTC, comment #2: 

Whoops, no, that patch is not fully baked.  It doesn't handle the space case.

Now I need to understand the parser more.

G. Branden Robinson <gbranden>
Group administrator
Tue 27 Jun 2023 03:26:04 PM UTC, comment #1: 

I propose the following patch.


diff --git a/src/devices/gropdf/gropdf.pl b/src/devices/gropdf/gropdf.pl
index c65a1051f..6ed7e4e57 100644
--- a/src/devices/gropdf/gropdf.pl
+++ b/src/devices/gropdf/gropdf.pl
@@ -370,14 +370,17 @@ while (<>)
        next if $cmd eq '#';    # just a comment
        my $lin=substr($_,1);

-       while ($cmd eq 'w')
+       if ($lin)
        {
-           $cmd=substr($lin,0,1);
-           $lin=substr($lin,1);
-           $w_flg=1 if $gotT;
-       }
+           while ($cmd eq 'w')
+           {
+               $cmd=substr($lin,0,1);
+               $lin=substr($lin,1);
+               $w_flg=1 if $gotT;
+           }

        $lin=~s/^\s+//;
+       }
 #              $lin=~s/\s#.*?$//;      # remove comment
        $stream.="\% $_\n" if $debug;


On my private branch, with the change to 'w' output, 4 automated tests fail and gropdf (Perl, really) screams thousands of lines of error output while constructing groff-man-pages.pdf.

With this patch, all 180 tests pass (or XFAIL) and no extra error output is produced.

But it may not be the most elegant fix, as I have only a worm's-eye view of gropdf's parser.

Assigning to Deri and putting into "Need info" status for feedback.

Is this patch okay or does it have problems?

G. Branden Robinson <gbranden>
Group administrator
Tue 27 Jun 2023 03:17:09 PM UTC, original submission:  

Input:


A B


This produces the following output from GNU troff.


x T pdf
x res 72000 1 1
x init
p1
x font 5 TR
f5
s10000
V12000
H72000
md
DFd
tA
wh2500
tB
n12000 0
x trailer
V792000
x stop


If we modify this output to put a space after the 'w' command, gropdf incorrectly omits the space between "A" and "B".

If we modify this output to put a newline after the 'w' command, gropdf's parser gets into a bad state, but correct output is produced.


substr outside of string at ./build/gropdf line 376, <> line 13.
Use of uninitialized value $lin in substitution (s///) at ./build/gropdf line 380, <> line 13.


gropdf handles a comment (which must end with a newline) after 'w' correctly.

CSTR #54 (1992) permits the variations above. "Blanks, tabs, and newlines may occur in the as separators in the input, and are mandatory to separate constructions that would otherwise be confused."

grops(1) appears to behave correctly in both scenarios, and since the parsing code is in libdriver, probably all of groff's other output drivers do.

I didn't test tabs, for the not very good reason that I am dubious about the worth of tab characters ever occurring in GNU troff output.  Also, since gropdf appears to be using '\s' for matching in its logic, the way Perl regexes define '\s' should handle the tab scenario the same as the space.

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:
   

Attached Files
file #55039:  ps-grout3 added by gbranden (179B - application/octet-stream)
file #54892:  bug64360.tar added by gbranden (60KiB - application/x-tar)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by barx (Posted a comment)
  • -email is unavailable- added by deri (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 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-08-15 gbranden Attached File- Added ps-grout3, #55039
    2023-08-15 gbranden Assigned togbranden deri
    2023-07-12 gbranden Assigned toderi gbranden
    2023-06-28 gbranden Attached File- Added CSTR_54_1992_section_22.png, #54894
    2023-06-28 gbranden Attached File- Added bug64360.tar, #54892
        Assigned togbranden deri
        Summary[gropdf] does not correctly handle white space after 'w' command [PATCH] [gropdf] does not correctly handle white space after 'w' command
    2023-06-27 gbranden StatusIn Progress Need Info
    2023-06-27 gbranden StatusNeed Info In Progress
        Assigned toderi gbranden
    2023-06-27 gbranden StatusNone Need Info
        Assigned toNone deri

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code