Patch for man page viewer

[ZlatkO]

The enclosed patch gets rid of the annoying red "<standard input>:1978: warning [p 24, 5.7i]: can't break line" error boxes when viewing man pages with long lines (see attached screenshot) on terminals that are actually wide enough.

The first variant of the patch is short and concise, but probably only works with bash.

The second variant is a bit more elaborate and readable, and also works on older shells that only know the deprecated backtick notation instead of the modern $() notation.

The net effect of both variants is exactly the same: they lookup $COLUMNS from the terminal that mc has been started on, and export MANWIDTH=${COLUMNS} accordingly.

Disclaimer: the patch has only been tested on gnome-terminal, xterm, uxterm & the (VGA) linux console on my 32bit Slackware 14.2 systems, so it is absolutely possible (but rather unlikely) that it will fail anywhere else. ;-)

[ZlatkO]

Screenshot of the error box

[ZlatkO]

Patch (variant #1)

[ZlatkO]

Patch (variant #2)

[zaytsev]

I think you don't need to implement awk detection - you can just use the values from configure, like all other mc scripts:

​https://github.com/MidnightCommander/mc/search?q=awk

This will make the second version considerably cleaner.

[ZlatkO]

Oh, cool, you're right of course - thanks for the hint! :-) I'll upload a modified patch against misc/ext.d/text.sh.in as soon as I get around to it.

[ZlatkO]

Short update: I noticed that do_open_action() actually has the same problem (I never hit <ENTER> on man pages myself, I always do <F3>). Moving the patch part up to the beginning of text.sh.in so that it gets executed for both do_view_action() and do_open_action() didn't fix it, though, so I'll investigate a little further.

[ZlatkO]

Found the answer on the always helpful ​Unix StackExchange. So, the trick would be to add -rLT=${MANWIDTH}n -rLL=${MANWIDTH}n to all nroff calls in text.sh.in.

I'm a bit hesitant tough to go ahead and update @MAN_FLAGS@ accordingly, as ${MANWIDTH} (or even the more generic ${COLUMNS}) might be unset in other places where @MAN_FLAGS@ is used, leading to a syntax error in the nroff call. Do you have any idea how to handle this properly? Should this be handled by autoconf rather that "hardcoded" in the patch?

Anyway, the current version of the patch is attached. Not that I switched from using the man-specific ${MANWIDTH} to the more generic ${COLUMNS}, as this is more likely to be read and interpreted/followed by any of the pagers.

[andrew_b]

The COLUMNS variable is already in the environment:

$ echo $COLUMNS
144

[ossi]

use of assignment in the export command is bash-specific.
(this particular) use of the /proc file system is linux-specific.
using the full terminal width is counter to accepted typesetting standards, which say that flowed text should not be wider than 70, maybe 80 columns for optimal legibility.

[ZlatkO]

Replying to andrew_b:

The COLUMNS variable is already in the environment:

$ echo $COLUMNS
144

Yes, in an interactive shell with a controlling TTY, but not in the shell that is spawned to execute libexec/mc/ext.d/text.sh:

[zlatko@disclosure:~]$ echo $COLUMNS
206
[zlatko@disclosure:~]$ /bin/sh -c "echo \$SHELL"
/bin/bash
[zlatko@disclosure:~]$ /bin/sh -c "echo \$COLUMNS"

[zlatko@disclosure:~]$ 

If it already were in the environment, then man and less would pick it up automagically and not display the can't break line error in the first place, and this patch would not be needed at all.

Go ahead and try it, add something like echo "\$COLUMNS is set to >$COLUMNS<" >> /tmp/mcview-debug.txt at the beginning of text.sh and have a look at the resulting /tmp/mcview-debug.txt. If you do find the proper value of $COLUMNS there, then your system/bash/sh/mc is cleverer than mine, and you won't need this patch. :-)

[ZlatkO]

Replying to ossi:

use of assignment in the export command is bash-specific.
(this particular) use of the /proc file system is linux-specific.
using the full terminal width is counter to accepted typesetting standards, which say that flowed text should not be wider than 70, maybe 80 columns for optimal legibility.

Okay, the first two points are easy to fix:

1. Split export COLUMNS=-... into COLUMNS=...; export COLUMNS
2. Wrap the whole thing in if [ "`uname -s`" = "Linux" ]; then ...; fi

Concerning the third one, I'd rather be knowingly violating the accepted typesetting standards than having to put up with annoying error messages in little red boxes that I have to shush away with an extra key press every time one pops up into my face. We're not layouting a book here that should be as pleasing to the eye to read as possible, after all, we're only trying to display a manual page in a terminal window with as little annoyance for the user as possible.

If adhering to typesetting standards is actually a goal of mcview's man page reader mode, then we could also simply redirect troff ... 2> /dev/null and man ... 2> /dev null and be done with it. As long as the annoying red error popups are gone I don't really care all that much. ;-)

[zaytsev]

So how about calling troff with -Wbreak instead? I somehow don't like the idea of suppressing standard error, which might contain something completely different and important...

​https://man7.org/linux/man-pages/man1/troff.1.html

[ZlatkO]

Replying to zaytsev:

So how about calling troff with -Wbreak instead? I somehow don't like the idea of suppressing standard error, which might contain something completely different and important...
[...]

Yes, fully agreed about redirecting stderr. Again, if typesetting standards is something that mc's man viewer actually cares about (which, if it does, is something I was not aware of), then -Wbreak would be the better option here. Personally, I'd prefer having my man page displayed over the whole available terminal width than only using ~40% of it (80 vs. 206), but that's just me. As long as the error popups are gone that's fine with me either way.

I'll do another version of the patch including fixes for your and ossi's concerns. Would you agree that the -Wbreak option is safe to be added to autoconf's @MAN_FLAGS@ setup (with a proper test, of course)?

What's your opinion regarding the rest of the patch? With -Wbreak the whole ${COLUMNS} hack in text.sh.in would not be needed anymore, as the error box would already be gone even without it. The only difference it would make would be using the whole available terminal width vs. limiting it to some compiled-in default (probably 80). OTOH, when calling man from the command line directly, it would actually take ${COLUMNS} into cosideration and use the whole available width.

There are a couple of possibilites that come to my mind.

1. leave it out completely, and let all invoked tools use whatever compiled-in default settings they have
2. cap the exported ${COLUMNS} at 80 and thus stick to the typesetting standards issue raised by ossi
3. leave it in as-is and thus mimick the as-if-run-from-plain-cli-behaviour of the invoked tools
4. make it user-configurable via checking env vars like ${MC_VIEW_MAN_FULLWIDTH} and/or ${MC_VIEW_MAN_CAPPED} or some such. This would probably be the most nerdiest solution, as (I guess) 99% of all users would not care either way as long as the error popups are gone. :-D

[zaytsev]

Would you agree that the -Wbreak option is safe to be added to autoconf's @MAN_FLAGS@ setup (with a proper test, of course)?

Sure!

In as far as COLUMNS are concerned, I don't really have a strong opinion on that. I'd probably leave it out and let the defaults kick in. I think this is currently the status quo.

Leaving for vacation now, see you in September!