Follow-up Comment #17, bug #67372 (group groff): You are saying there is no regression because only undefined behaviour changed, and the mandoc(1) test suite is over-testing.
So, the first question is whether \w\*v is indeed undefined. I believe the main place where to look for an answer is: https://www.gnu.org/software/groff/manual/groff.html.node/Using-Escape-Sequenc es.html Citing selectively, Whereas requests must occur on control lines, escape sequences can occur intermixed with text and may appear in arguments to requests, macros, and other escape sequences. [...] Escape sequence interpolation is of higher precedence than escape sequence argument interpretation. This rule affords flexibility in using escape sequences to construct parameters to other escape sequences. Admittedly, this is not completely conclusive, so the judge must interpret it resorting to legislative intent, or precedent, or analogy, or common sense. Legislative intent seems to contrast requests, which can only be invoked in one very particular place, to escape sequences, which can be invoked (almost?) anywhere. Formally, it may be possible to interpret a long list that, on first sight, seems all-encompassing, in particular when it does not even allude to a single restriction, as "this is intended as an exhaustive enumeration, nothing else is allowed" - but that doesn't seem particularly compelling. Precedent is unambigous from the dawn of time to groff-1.22.4. Several examples also support the idea that the first sentence is not intended as exhaustive. For example: * .ds s B .\*s bold text invokes the B macro, printing bold text * .\*sR bold text invokes the BR macro, printing only bold in bold * .R\*s text bold invokes the RB macro, printing only bold in bold * .ds t bold text .RB \*t passes *two* arguments, printing only bold in bold * .ds t ld te .BR bo\*txt passes *two* arguments, printing only bold in bold Now arguably, the wording "may appear in arguments" might include escape sequences contributing to multiple arguments - though the more natural interpretation of that wording would be "may appear in all arguments" (i.e. any argument may invoke escape sequences.) Even if you argue that the first sentence *is* exhaustive and you are free to break all the above examples (and more), the question remains open whether "argument interpretation" includes argument delimiter interpretation - i would maybe argue that it does. At least, that is supported by examples like the following: * .ds s [B] \f\*stext * .ds s (CB \f\*stext Both print the text in bold. This section 5.6.4 mentions none of the terms "interpolation depth", "nesting depth", or "input level", nor could i find any definition of these terms anywhere (by grepping over all of groff git master). Unless i'm missing something, they are only mentioned in passing in obscure corners of the documentation (mostly about compatibility with historic implementations) without ever properly defining them and without ever specifying what their effect is, which leads me to consider them implementation details that the user should not have to worry about. I admit that the original idea behind "interpolation depth" looks interesting, i.e. getting \w'\*s' to work for arbitrary s, including s containing single quotes, but it appears it was never really thought through nor designed consistently, let alone implemented consistently. It *may* be possible to define and implement it in a consistent manner as follows: 1. If an escape sequence function character that (optionally or mandatorily) takes a delimited argument is immediately followed by a valid delimiter that does not result from interpolation, than any instances of the delimiter resulting from interpolation while parsing the argument do not end the argument. 2. If an escape sequence function character that (optionally or mandatorily) takes a delimited argument is immediately followed by the beginning of another escape sequence, rule 1 does not apply. That would possibly provide the intended benefit for \w'\*v' without breaking \w\*v. I'm not completely sure given how complicated the logic is (and since how it is actually intended to work is undocumented). Also, i'm not convinced exposing the user to such complicated logic is ideal, in particular since this is only one (relatively minor) instance typical for interpreted languages that support macros (notable and notorious example, sh(1)) and certainly for roff(7). The roff(7) language contains more severe instances of the same class of problems. For example, you can define a macro that starts are macro definition without ending it, then call it and end the macro definition in the surrounding code. And given how much roff(7) revolves around macro interpolation, i'm sure there is more. For now, i'm *not* planning to commit my revert patch to groff, mostly because a revert is adequate when there is consensus that there is a regression, but there is no such consensus. Besides, i don't currently have a working port of groff from git, and i remember from the last time that i tried to build one, it was extremely difficult to get it to build at all, and i neither want to get distracted right now from working on 1.23 nor commit anything without proper testing. Repeatedly getting distracted with git-master during the last two years was one contributing factor to the intolerable delay dealing with 1.23, so not getting near git-master is part of my (at this point, close to desparate) strategy for finally getting 1.23 out of the door. So i'll temporarily use my revert patch in the OpenBSD port - not because i'm convinced that is the proper long-term solution, but as an interim stopgap until we develop a consensus what the intended behaviour actually is. Once groff grows sane and consistent behaviour, i'm not opposed to changing mandoc(1) yet again. One remark regarding over-testing. You are right that over-testing can be a real problem. For example, that is the reason why the mandoc(1) testsuite does not do any -T ps tests just yet: i simply couldn't figure out any way so far how to avoid that all PostScript tests would contain more over-testing than intentional testing. But testing only documented features is (sadly) not an option for groff-mandoc compatibility testing, for multiple reasons. 1. Even though groff documentation is significantly better than lots of other software documentation (that was already true during wl@ times), and has continued to improve over time, in particular during the last couple of years, it does still have its gaps and ambiguities, as this very ticket demonstrates. When documentation is less than perfect, testing for some undocumented features is unavoidable. 2. Manual page authors are generally not known for diligently reading groff documentation before writing or maintaining manual pages. Even the authors of the vast majority of man(7) code generators are not known for that - probably with the exception of the author(s) of pod2man(1), which quite reliably produces decent code. But even a generally exceptionally diligent and prolific manual page author as jmc@ has told me "if what i write doesn't work as i hoped, i simply tweak it until it does"; he only (partially) changed that stance and relied a bit more on mdoc(7) and roff(7) documentation some time after the advent of mandoc. Most authors of man(7) code generators clearly do the same. The end result is that not only documented behaviour matters, but it can easily happen that manual pages rely on undocumented behaviour. 3. Mandoc(1) does not only aim for compatibilty with groff documentation, but for compatibility with groff output, aka bug-for-bug compatibility. That requires some testing of undocumented features. 4. While groff(1) documentation is good, it is not complete and detailed enough for guiding a reimplementation. When developing mandoc(1), i almost always have to run groff to test how it behaves before i can select algorithms for use in mandoc. Once that is done, the test suite has to make sure mandoc(1) remains compatible. 5. The test suite is primarily a mandoc(1) test suite, that is, its main purpose is to make sure that mandoc(1) output does not accidentally change, not even in cases where groff behaviour is undocumented. Consequently, the test suite aims to exercise all code paths in mandoc(1) and all possible combinations of input tokens related to the feature being tested, with no regard to which combinations are documented. While test suite completeness is hard to define and hard to achieve, it is a worthy goal and should not be confused with overtesting. When the test suite fails on the mandoc side, you generally don't hear about it, i simply fix it myself. When it fails on the groff side, i try to classify it. The reason may be: a) A groff regression that must be fixed in the groff port and reported upstream. b) A minor groff behaviour change that doesn't matter much either way and that mandoc(1) can simply follow. c) A bug fix in groff that mandoc(1) should follow. d) A feature improvement in groff that mandoc(1) should follow. I don't think adding the following class would make much sense: e) Change in undefined behaviour, just delete the test. In mandoc(1), i definitely don't want accidental output changes even for invalid input. Even for invalid input, i always want to make sure that the new output makes more sense than the old output. I think aiming for that would make sense in groff, too. I don't think the commit that caused this regression (or behaviour change, if you want - but given that the behaviour change was accidental rather than intentional, the term "regression" seems fitting to me, no matter whether the behaviour is defined or not) did not meet that standard. I do not see how it made the output or the diagnostics better. Rather, both degraded - the output lost text present in the input file, and the new warning message was quite confusing and did not help the user in diagnosing the problem. I find it quite surprising that the root cause for this fallout turned out to be "a jjc@ feature that still isn't properly documented nor unambiguously designed nor consistently implemented". Had you asked me, i would hardly have thought that issues of this kind still exist in groff. Really surprising for groff to still suffer from what can essentially be described as a "paedriatic disease". This regression now looks like the runny nose and the rash that unavoidably comes with such a childhood illness. _______________________________________________________ Reply to this item at: <https://savannah.gnu.org/bugs/?67372> _______________________________________________ Message sent via Savannah https://savannah.gnu.org/
signature.asc
Description: PGP signature
