Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rg --help | less assumes the terminal has 100 columns #442

Closed
bluss opened this issue Apr 9, 2017 · 17 comments
Closed

rg --help | less assumes the terminal has 100 columns #442

bluss opened this issue Apr 9, 2017 · 17 comments
Labels
question An issue that is lacking clarity on one or more points.

Comments

@bluss
Copy link

bluss commented Apr 9, 2017

If we pipe rg's output to something, it assumes the terminal has 100 columns.

For example rg --help, which has output that is long and begs for pagination.

To reproduce: Install rg. Adjust terminal to a width of for example 90 columns. Use rg --help | less (with no shell aliases or anything in between).

(Sorry! rg --help is now fixed, so the bug report now had its schedule unblocked)

@BurntSushi
Copy link
Owner

ripgrep doesn't do anything explicitly smart with terminal size, so I assume this is a clap thing.

@BurntSushi BurntSushi added the question An issue that is lacking clarity on one or more points. label Apr 9, 2017
@BurntSushi
Copy link
Owner

I could have sworn clap had some setting or knob for this, but I can't find it after a quick skim.

@kbknapp
Copy link
Contributor

kbknapp commented Apr 9, 2017

There's App::max_term_width and App::set_term_width

max_term_width will allow freeform "smart" wrapping at whatever the user terminal width is, up to some maximum. This is what ripgrep uses currently (at 100).

set_term_width sets the virtual width and always "smart" wraps at that column width.

@kbknapp
Copy link
Contributor

kbknapp commented Apr 9, 2017

[I hit comment too early]

Not using either will just "smart" wrap at whatever width the user terminal width happens to be.

@BurntSushi
Copy link
Owner

@kbknapp Oh interesting. Welp. I was clearly wrong about ripgrep not doing anything.

With that said, I can't seem to cause clap to do the right thing here. Namely, if max_term_width is set to 100, then shouldn't it wrap text at N < 100 after I shrink my terminal? (I also just tried set_term_width, which has the same behavior.)

If I remove the max_term_width/set_term_width setting completely, then the help text sprawls across my entire screen, which I suspect is why I set it in the first place.

@BurntSushi
Copy link
Owner

I see. clap does do the right thing unless its output is piped to less. So presumably that interferes with clap figuring out the terminal width?

@kbknapp
Copy link
Contributor

kbknapp commented Apr 9, 2017

Interesting, my initial thinking is less is being detected a terminal without a width (or rather stdout not being a terminal) which is causing the fallback to 100.

@BurntSushi
Copy link
Owner

I haven't looked at clap's source code, but my best guess is that clap only does autowrapping when it thinks its printing to a tty (which seems reasonable). So if its output is piped to a file or to another program, then it won't auto-wrap because it isn't a tty. It should be possible to discriminate between "redirected to a file" and "piped to other process," but even then, it's not clear that respecting terminal size is the right choice (although it certainly seems like a pragmatic one).

Another choice is to just set the max width to 79 columns and hope that fits most use cases.

@BurntSushi
Copy link
Owner

@kbknapp Ah right, beat me to it!

@kbknapp
Copy link
Contributor

kbknapp commented Apr 9, 2017

@BurntSushi
Copy link
Owner

BurntSushi commented Apr 9, 2017

@kbknapp Oh, I suppose that is actually a different issue than what I was saying above. That's suggesting that ioctl itself will fail to find the terminal size if the process is being piped to another process? Or rather, finding the dimensions works by examining the stdout file descriptor? Interesting.

@kbknapp
Copy link
Contributor

kbknapp commented Apr 9, 2017

@BurntSushi ioctl will fail when stdout isn't a tty. The crux of the problem is that the dimensions are determined by stdout, and where stdout is piped or redirected there are no dimensions. I don't know of a way to detect what the terminal width was (i.e. before stdout is checked).

@kbknapp
Copy link
Contributor

kbknapp commented Apr 9, 2017

@BurntSushi I found a solution that seems to work! 🎉

term_size can read stdout, and if that fails try stdin...if that fails then try stderr...then give up. Works in my tests so far.

@BurntSushi
Copy link
Owner

BurntSushi commented Apr 9, 2017 via email

@kbknapp
Copy link
Contributor

kbknapp commented Apr 10, 2017

Just did a cargo install ripgrep (to update term_size) and this is working.

@eproxus
Copy link

eproxus commented Jun 22, 2020

I have this issue running rg -h and rg --help using 12.1.1 running on macOS. Was this really fixed?

@mattiase
Copy link

mattiase commented Dec 8, 2021

I have this issue running rg -h and rg --help using 12.1.1 running on macOS. Was this really fixed?

No, you are right – it seems that clap isn't used with the wrap_help feature. Presumably it should be added to the list of features in Cargo.toml.

With that done, it works even when stdout is a pipe, as in rg -h | less since it seems to look at the tty config for stdin (which is standard practice for interactive tools). Nice!

However, the --help output is less pleasing: clap doesn't reflow paragraphs but wraps each line separately instead and this leads to the infamous "comb" appearance of alternating long and short lines. To deal with this, we either need to remove newlines inside paragraphs (replacing each with a single space), or detect blank lines as paragraph delimiters. The latter means less boring legwork but some changes to clap it seems.

Oh, and it would be nice with a right margin of a few spaces (2, say) for readability. I suppose that would go into clap too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question An issue that is lacking clarity on one or more points.
Projects
None yet
Development

No branches or pull requests

5 participants