Bug #120229 for Pod-Perldoc: perldoc rudely interferes with pager configuration

On Mon Feb 13 09:25:32 2017, zefram@fysh.org wrote: Show quoted text
Yes, sorry. I have published 3.27_02 to CPAN which addresses this issue. Please try that release. If it fixes the problem, I will publish _02 as 3.28 official.

On Wed Mar 01 23:46:59 2017, zefram@fysh.org wrote: Show quoted text
OK, would you help me understand how you would prefer I address this issue?

Mark Allen via RT wrote: Show quoted text
Well, I'd *prefer* that you leave the user's pager configuration untouched and pick a default formatter that's compatible with that. But that's a bigger change than I've sought with this ticket. If we take as read that this environment diddling is required with the ToTerm formatter, then it ought to be applied *only* with that formatter. The question then is merely how to detect the ToTerm formatter, to which there are two obvious answers. Firstly, there could be a method in the generic formatter API, akin to is_pageable, indicating whether to modify the environment. Or secondly, Pod::Perldoc could perform an exact string comparison on the whole of the class name (where 3.27_02 has a case-insensitive substring match). -zefram

Mark Allen via RT wrote: Show quoted text
There are two parts to this: the pager_configuration API, and the reduced amount of environmental change. The pager_configuration interface is an obvious generalisation of having the formatter declare whether it needs the configuration change. (Incidentally, I was surprised that you didn't add an empty pager_configuration to Pod::Perldoc::BaseTo.) If the choice is made to embrace the idea of per-formatter customisation of the pager, then I would certainly go in this direction of generalisation. But this is not the place to stop: I would generalise further. Whatever provides the code to actually change the environment settings needs to know about specific pagers and how to configure them. The individual formatters are very poorly placed to be burdened with this kind of knowledge: it's way off their core concern. That's a maintainability problem. There's also a maintainability problem where pager customisations ought to be shared between formatters, as is arguably the case with ToANSI and ToTerm. So if I were going the generalisation route, I'd introduce another layer of abstraction: have the formatter declare *what kind of textual features appear in its output*. I'd leave it up to framework code (in Pod::Perldoc or factored out) to mediate between those requirements and the pagers available. This system would supersede the existing is_pageable flag, and would give the framework a lot of flexibility in how to respond to output features, all of which could be influenced by user configuration of which the formatters do not need knowledge. ToTerm would declare "output contains ANSI text attribute controls", and in response to that the framework might decide (a) that the pager configuration can already handle that, so just pass it through; (b) that the pager configuration needs a change to handle it, so tweak environment and pass the output through; (c) that the pager can't handle it but the terminal can, so output it without a pager; (d) that the ANSI controls should be translated into some other kind of controls, with filtered output going to a pager; (e) that the ANSI controls should be removed, with filtered output going to a pager; or (f) that a different formatter should be used instead. Options to perldoc(1) or in the environment could influence this, and it could perform test executions of pagers, and so on. It can get arbitrarily sophisticated in future versions, without the individual formatters having to change. However, if I were in your place I would probably go to the opposite extreme. I don't think there's enough value in doing the pager configuration properly to offset its complexity and the burden of incorporating an unbounded amount of knowledge about specific pagers. As discussed elsewhere, I'd be looking to remove the pager tweaking entirely (along with the need for it), if not for Perl 5.26 then for 5.27.1. With that expectation, I'd look to minimise the present hack, on the basis that it's a stopgap that will shortly go away. I'd keep the hack entirely in Pod::Perldoc, and base it on exact match of the formatter class name. As regards the proposed reduction in the amount of enviornmental change, I don't think this achieves anything useful. It retains all the downside of dishonouring the user's configuration, including that it will entirely break some reasonable configuration on a current OS (more(1) on Debian with $ENV{MORE} unset). As far as I can see the change just makes it less successful at setting the -R option. -zefram

Sawyer X via RT wrote: Show quoted text
I presume that by "reverting all of these changes" you mean returning to defaulting to the ToMan formatter. It would help if you were explicit about which formatter you mean; terms like "the old pipeline" and "the new pipeline" are unnecessarily unclear. This groff issue was presumably the original motivation for changing the default away from ToMan. No one is proposing returning to that default (at least in its unconditional form). The problems we see now arise from the question of which formatter should be the default given that ToMan is not acceptable. The main candidates for this are ToTerm and ToText. Show quoted text
The context for the stopgap was the work immediately prior to the release of 5.26. The question of what version of Pod::Perldoc would go into 5.26 was distinct from the question of how it should be fixed in the long term. An actual fix, of changing the default formatter, was vetoed for 5.26, so for 5.26 we were looking just at tweaks to the environment-mutation approach. It's a stopgap in the sense that that was expected to be used only for 5.26, with a proper fix coming along for 5.28. Show quoted text
Altering the environment is the whole purpose of that bit of code. It writes into %ENV. Show quoted text
I'm OK with ToMan being used as the default conditional on the groff version. That's a refinement beyond what we've been discussing so far. The more important question, which we need to resolve first, is what should be the default when ToMan can't be used. Show quoted text
Anything that sticks "-R" into the environment is asking for trouble. That's a portability problem, and as you noted we don't have any volunteers to take on the full porting job. This is why I say that ToTerm is not suitable to be the default formatter: the fallback has to be ToText. -zefram

Sawyer X via RT wrote: Show quoted text
That's part of it, but there are multiple failure modes. My original complaint, before any environment twiddling was implemented, was that it renders badly, all "ESC[1mSYNOPSISESC[0m". There are other problematic outcomes too. Rough categorisation of failure modes: 0. If perldoc ever passes "-R" to the pager (via either environment or command line), then sometimes this will be applied to a pager that doesn't support -R, so you'll get an error message instead of documentation. 1. If perldoc ever refrains from passing "-R" to the pager, then it'll quite often hand the escape seqeunces off to a pager that's not configured to handle them, so you'll get nicely-paged "ESC[1mSYNOPSISESC[0m" instead of the intended rendering. 2. If the terminal is one that doesn't use the ANSI escape sequence schema, then successfully configuring the pager to pass them through (which is what -R does) yields terminal-dependent garbage instead of the intended rendering. Note that taking items 0 and 1 together means that you can't win. There is no criterion for adding "-R" that doesn't trigger at least one of these failure modes, and most criteria trigger both. There are also some situations where it's impossible to get the intended output even given perfect knowledge of how to configure pagers: user's pager can't be configured to pass through escape sequences; terminal doesn't support those escape sequences. Show quoted text
ToText produces plain text, no escape sequences. That's the only difference, AFAICS. Both emit non-ASCII characters as UTF-8. (UTF-8 itself poses a rendering problem, but I'll let that slide for now.) Show quoted text
It's terribly unportable. We don't know what switch to pass for most pagers; for pagers where we know how to spell "-R" we don't know whether the pager actually supports that switch; and we don't know whether the terminal supports those escape sequences anyway. It's impossible for us to actually fix the issue this way. If it were actually possible to fix the rendering by sticking something in the environment, and we could in all cases know what to stick in the environment to do it, then I'd be pretty OK with that. But neither of those conditions holds. My objection to tweaking the environment isn't an objection in principle to ever touching the environment; it's an objection to altering configuration that we don't understand. -zefram

Sawyer X via RT wrote: Show quoted text
... Show quoted text
In these comments, you seem to be implying that there is a moral equivalence between the ToTerm and ToText mechanisms. (Though you don't mention either of those formatters explicitly, so it's not clear.) In fact there is no such equivalence in their portability. There is a sharp difference between ToTerm raising these huge portability problems and ToText which reliably works. Show quoted text
I'm fine with conditional use of ToMan, indeed I welcome a return to the ToMan default on the platform that I use personally, provided that the fallback where ToMan is not usable is a properly portable one. Once again, your discussion of ToMan here is a distraction. These tickets are concerned with the non-ToMan situation. Show quoted text
Of course, we are always relying on the user's environment to be coherent. None of this has been concerned with environments that are a priori faulty. All of the situations that I have raised that ToTerm has difficulty with are reasonable situations, with environmental configuration presumed to be correct. Show quoted text
I don't understand this comment. What exactly is broken on macOS? By "currently", do you mean perldoc with its ToTerm default (as in the CPAN version), or perldoc with the ToText default (as in blead), or something else? I'm mystified by your reference to "some other non-standard pager". To be clear, the portability issues with ToTerm arise on macOS just as on any other platform. I recommend that the CPAN instance of Pod-Perldoc should incorporate the same change to Pod::Perldoc with which I customised it in blead; then it should also remove the (now never called) pager_configuration method from Pod::Perldoc::ToTerm; then from that base it should incorporate some conditional preference for ToMan. The first part of this is the most important, and the last part should be postponed if the correct form for the condition is not immediately apparent. -zefram

Sawyer X via RT wrote: Show quoted text
OK. Defaulting to ToMan where we're confident that it'll work would seem to be the main way to avoid that downside as much as possible. We cannot avoid getting unhighlighted text in some situations, though. Don't let the (sensible) desire for nice formatting get in the way of achieving legibility for everyone. Show quoted text
Apparently I didn't understand the previous times you've noted it. This is the first time I've been aware of you defending ToTerm on the basis that we can limit its usage to a relatively small set of users. So I think what you're proposing, though you still haven't actually said it explicitly, is roughly that the default should be ToMan on platforms other than macOS, and should be ToTerm on macOS. Maybe we'd also avoid ToMan on Windows, as we used to, though I don't know what you're then proposing the default should be on Windows. Maybe also some condition based on detecting the version of the roff toolchain, though in your previous message you didn't say whether that's ANDed or ORed with macOS, so I don't know where this fits into the proposal. I think this is a poor basis on which to defend ToTerm. It's got exactly the same problems for macOS users that we have noted on other platforms. Only using it on macOS would indeed reduce the number of affected users, but this amounts to giving those users exactly the mechanism that we've rejected for the majority on the basis that it's broken. The problem here is that the condition for choosing ToTerm, namely the macOS platform, has nothing to do with whether ToTerm would actually work. For ToTerm to ever be the default, the conditions for selecting it need to be conditions that give us confidence that we can actually pull it off. This would be, roughly, * we recognise the specific pager nominated by the environment; and * by checking the specific version of the pager that will be executed we are confident that it is a version that supports -R; and * the terminal type nominated by the environment is one that we specifically recognise as taking ANSI escape sequences. If those conditions are met then we could confidently arrange to pass "-R" as a command-line argument to the pager and select the ToTerm formatter. If these conditions are not met then we're back in the usual situation of ToTerm being hopelessly unportable, and must fall back on ToText. It should be easy enough to arrange for the ToTerm-selecting conditions to include most of the macOS users about which you're concerned. Considering ToMan as well, presumably we'd still prefer ToMan, and so only look at whether ToTerm is an acceptable default if we've already decided that ToMan is not acceptable. Show quoted text
Er, what? Where I've described things breaking, that's been specifically about ToTerm breaking. The version of groff has no influence on ToTerm. I can't figure out what you're trying to ask here. Show quoted text
Right, so, to be clear, the current perldoc defaults (in either CPAN or blead versions) don't produce any broken Unicode on macOS. -zefram

Sawyer X via RT wrote: Show quoted text
I don't have firm platform knowledge. I expect that Windows ordinarily lacks a roff toolchain, making ToMan inoperable. It seems from the previous versions of Pod::Perldoc that ToTerm is also ordinarily unworkable on Windows: we know that the usual pager doesn't support -R. I don't know what its usual terminals do with the escape sequences. So if we're making flat per-platform decisions, it looks like Windows needs to default to ToText (as it does in the current CPAN version of Pod::Perldoc). But... Show quoted text
Yes. I think we're converging on the concept that we should detect what's actually workable at runtime, rather than make coarse per-platform assumptions. We need such detection logic for ToMan and for ToTerm. -zefram

On Wed Oct 25 17:47:16 2017, xsawyerx wrote: Show quoted text
Here is my pull request: https://github.com/mrallen1/Pod-Perldoc/pull/36. This is against mrallen1's account, but I can move it to the DualLife account as well.

On Thu Nov 30 15:40:55 2017, zefram@fysh.org wrote: Show quoted text
I agree. I thought about this and eventually stole it from the detection for adding "-R" to the pager. I figured it would at least be as consistent as it is now, despite being inaccurate. Show quoted text
Good point. I'll improve this by finding the execution file and then calling only that with "--version", which would remove any redirection characters which might have appeared. Show quoted text
Whoops! Fixed and PR was updated. Show quoted text
Makes sense. That part was evidently fragile since all I did was check whether one of the possible pagers available is less. I couldn't tell if it would be used. The code then tries each one indepdendently hoping it will work (literallly checking the result of "system()") so it's unassured which are available or will work. Reducing it to "which are available" and then to the first one being "less" makes more sense. Show quoted text
It is possible to simply let those values override my inspection of possible pagers. Instead it currenly only adds it as an optional pager candidate. I think the reason is that a user might have an environment configured for a certain pager by the operating system which doesn't actually exist or is installed. I am in the opinion that, if you explicitly want something, that's what you should get (even if you didn't personally explicitly stated, but some other configuration did) and that, if that fails, you should be notified. In other words, if Debian automatically set my EDITOR to vim but I don't have vim installed, I would like to know about instead of whatever used the EDITOR configuration would silently just try something else and open a different editor. If we both agree on this, I can change the guessing function to explicitly override whatever pager is avaiable by those configurations and then, as a secondary measaure, try to reduce the list to existing pagers, and if the first one is less, continue with the logic. Does that make sense? Show quoted text
I had left it as is since ToTerm sets it in its own formatter. I can move this out of the way, but this would make it a particular ToTerm change that appears outside it. Although, your suggestioon would be safer since it would be parsing the pager string and make sure it's in the right place. Not sure which is best. Show quoted text
So we would have to both make sure that we have less that can understand escape codes as escape codes and to know that the terminal supports them and can display them properly. For the first we have a solution, for the latter we will need to create a list of known terminals that support escape codes and check ENV{TERM} for them. If we do not know your terminal, we cannot be sure it supports escape codes, and thus move to ToText as a fallback. If so, this means the logic is: * If you have an old version of groff * And your less is old, or * Your terminal is unknown (meaning it's implicitly known to not support escape codes), then: -> You get ToText. * If you have an old version of groff * And your less is new enough * And we know your terminal (and implicitly know it to support escape codes), then: -> You get ToTerm. * If you have a new enough version of groff, then: -> You get ToMan Did I understand it correctly?