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

regression: bat fails to page - will not read from stdin if no file (or -) specified #2572

Closed
jamesodhunt opened this issue May 19, 2023 · 2 comments · Fixed by #2574
Closed
Labels
bug Something isn't working

Comments

@jamesodhunt
Copy link

jamesodhunt commented May 19, 2023

bat (no longer) reads from stdin if no file or - is specified. This breaks my git(1) workflow.

What steps will reproduce the bug?

# Works
$ bat big-file.txt

# Fails to page
$ cat big-file.txt | bat

# Fails to page
$ cat big-file.txt | bat -

# Works (because an actual file is specified)
$ cat big-file.txt | bat /dev/fd/0

What happens?

bat doesn't page - the file is just dumped to stdout a la cat(1).

What did you expect to happen instead?

bat should page the file.

How did you install bat?

$ git clone https://github.com/sharkdp/bat
$ cargo install --path .

$ bat --version
bat 0.23.0 (b420c42)
@jamesodhunt jamesodhunt added the bug Something isn't working label May 19, 2023
@jamesodhunt jamesodhunt changed the title regression: bat will not read from stdin if no file (or -) specified regression: bat fails to page - will not read from stdin if no file (or -) specified May 19, 2023
@jamesodhunt
Copy link
Author

I'm afraid I don't have time to debug the problem but the code in this block looks odd and potentially related:

It looks like files being None is handled twice: once on line 311 and again on 318, but presumably we can't get to that line?

@Nigecat
Copy link
Contributor

Nigecat commented May 22, 2023

This caused some problems for me as well, a quick bisection shows that the regression was introduced in 57cc0d8 when atty was switched to is-terminal.

This seems to have been introduced since the condition !atty::is(Stream::Stdin) was inverted to std::io::stdin().is_terminal(). This seems to be a mistake since the comment says

// If we are reading from stdin, only enable paging if we write to an
// interactive terminal and if we do not *read* from an interactive
// terminal.

but the current code enables paging if stdin is interactive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants