holzschu / a-shell

A terminal for iOS, with multiple windows
BSD 3-Clause "New" or "Revised" License
2.62k stars 116 forks source link

`less`: Garbled text without `-r` option (send input escape sequences to output) given #251

Open personalizedrefrigerator opened 3 years ago

personalizedrefrigerator commented 3 years ago

Summary

Screenshots

jsi manpage

Help text of less

personalizedrefrigerator commented 3 years ago

I'm not experiencing the issue when using vim as my manpager (setenv MANPAGER "vim -m +MANPAGER").

Notably, this is a different value for the environment variable than is specified in vim's :help Man — the trailing dash was removed.

It would be nice to have this either be the default (which might not work because users may need to have source $VIMRUNTIME/ftplugin/man.vim in their .vimrc) or documented somewhere.

personalizedrefrigerator commented 3 years ago

Interestingly, running setenv LESS "-r" fixes the issue. (-r passes escape sequences directly to the terminal, which is not ideal (see below).).

See https://stackoverflow.com/questions/13294059/preserving-color-of-text-piped-through-less-or-more

holzschu commented 3 years ago

Thanks for finding the answer. I don't think the security issues about -r apply to this specific terminal. I could activate the option by default in less, but it seems that a less disruptive option is to have setenv("PAGER", "less -r", 1) in the initialization code of a-Shell.

personalizedrefrigerator commented 3 years ago

less raw option may lead to improper line wrapping

The part of this excerpt from man less warning about incorrect wrapping as a result of using -r is interesting...

personalizedrefrigerator commented 3 years ago

At least for man pages, it looks like -r can cause the very beginning of input to be clipped on some screens: 0D3ADA08-3311-4F8E-909B-3E7707CF88B2

It looks like it's still printing the hidden lines (scrolling up with hterm's scrolling): B2BC7B43-42A2-4C9D-8BC0-18A3D63693AE

Note that this only happens when my phone is in portrait mode. When in landscape mode, it displays properly.

Our issue (with and without -r) seems very similar to this: https://unix.stackexchange.com/questions/424696/less-discards-first-lines-s-does-not-work ...but it hasn't been answered.

personalizedrefrigerator commented 3 years ago

From https://unix.stackexchange.com/questions/93173/how-does-less-know-the-terminal-resolution, it looks like less doesn't necessarily use the LINES and COLUMNS environment variables to get the terminal size (from env, these look correct). Could it be that ioctl(2, TIOCGWINSZ, ... or ioctl(2, WIOCGETD, ... aren't correctly reporting the terminal size?

holzschu commented 3 years ago

After checking with the debugger, ioctl(2, TIOCGWINSZ, ...) returns 0 (as it should), so less uses getenv("LINES") (as it should). And yet the position on screen is wrong with less -r, and correct with less.

That's all I know for now. I removed the setenv("PAGER", "less -r", 1) from the code until we have something better.

personalizedrefrigerator commented 3 years ago

ioctl(2, TIOCGWINSZ, ...) returns 0 (as it should)

screen.c `ioctl` returns zero -> uses it

It looks like if ioctl returns zero, its result is used by less... Does ioctl(2, TIOCGWINSZ, &w) make w have w.ws_row = 0 and w.ws_col = 0?

Thank you for looking into this!

holzschu commented 3 years ago

Sorry, I meant that ioctl returns non-zero (we don't enter the branch).