Bat: Terminal width seems to be unused, resulting in very small output

[itkovian]

What version of bat are you using?
bat 0.15.0

Describe the bug you encountered:

bat is no longer recognising the terminal width on iTerm2 3.3.9.

It seems like it is using 44 as the width, while my terminal is wider than 200 characters.

~~
───────┬────────────────────────────────────
│ File: LICENSE
───────┼────────────────────────────────────
1 │ GNU LIBRARY GENER
│ AL PUBLIC LICENSE
2 │ Version 2, J
│ une 1991
3 │
4 │ Copyright (C) 1991 Free Software F
│ oundation, Inc.
5 │ 51 Franklin Street, Fifth Floor, B
│ oston, MA 02110-1301 USA
6 │ Everyone is permitted to copy and
│ distribute verbatim copies
7 │ of this license document, but chan
│ ging it is not allowed.
8 │
9 │ [This is the first released version
│ of the library GPL. It is
10 │ numbered 2 because it goes with ve
│ rsion 2 of the ordinary GPL.]
11 │
12 │ Preambl
│ e
13 │
14 │ The licenses for most software ar
│ e designed to take away your
15 │ freedom to share and change it. By
│ contrast, the GNU General Public
16 │ Licenses are intended to guarantee
│ your freedom to share and change
17 │ free software--to make sure the sof
│ tware is free for all its users.
18 │
19 │ This license, the Library General
│ Public License, applies to some
20 │ specially designated Free Software
│ Foundation software, and to any
21 │ other libraries whose authors decid
│ e to use it. You can use it for
22 │ your libraries, too.
23 │
24 │ When we speak of free software, w
~~

Describe what you expected to happen?

~~
───────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
│ File: LICENSE
───────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
1 │ GNU LIBRARY GENERAL PUBLIC LICENSE
2 │ Version 2, June 1991
3 │
4 │ Copyright (C) 1991 Free Software Foundation, Inc.
5 │ 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
6 │ Everyone is permitted to copy and distribute verbatim copies
7 │ of this license document, but changing it is not allowed.
8 │
9 │ [This is the first released version of the library GPL. It is
10 │ numbered 2 because it goes with version 2 of the ordinary GPL.]
11 │
12 │ Preamble
13 │
14 │ The licenses for most software are designed to take away your
15 │ freedom to share and change it. By contrast, the GNU General Public
16 │ Licenses are intended to guarantee your freedom to share and change
17 │ free software--to make sure the software is free for all its users.
18 │
19 │ This license, the Library General Public License, applies to some
20 │ specially designated Free Software Foundation software, and to any
21 │ other libraries whose authors decide to use it. You can use it for
22 │ your libraries, too.
23 │
24 │ When we speak of free software, we are referring to freedom, not
25 │ price. Our General Public Licenses are designed to make sure that you
26 │ have the freedom to distribute copies of free software (and charge for
27 │ this service if you wish), that you receive source code or can get it
28 │ if you want it, that you can change the software or use pieces of it
29 │ in new free programs; and that you know you can do these things.
30 │
31 │ To protect your rights, we need to make restrictions that forbid
32 │ anyone to deny you these rights or to ask you to surrender the rights.
33 │ These restrictions translate to certain responsibilities for you if
34 │ you distribute copies of the library, or if you modify it.
35 │
36 │ For example, if you distribute copies of the library, whether gratis
37 │ or for a fee, you must give the recipients all the rights that we gave
38 │ you. You must make sure that they, too, receive or can get the source
~~

How did you install bat?

Through cargo install

---

system

$ uname -srm
Darwin 19.3.0 x86_64

$ sw_vers
ProductName: Mac OS X
ProductVersion: 10.15.3
BuildVersion: 19D76

bat

$ bat --version
bat 0.15.0

$ env

bat_config

bat_wrapper

No wrapper script.

bat_wrapper_function

No wrapper function for 'bat'.

No wrapper function for 'cat'.

tool

$ less --version
less 487 (POSIX regular expressions)

[sharkdp]

Thank you for reporting this!

I can not reproduce this with bat 0.15 on iTerm2 3.2. Unfortunately, I can not install iTerm2 3.3.9 in my VirtualBox with MacOS X 10.11.6.

Do you happen to know the last version of bat which did not show this behavior?

[itkovian]

Yes.

~~~~
―― 14:38:57 - Cargo ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
Updating registry 'https://github.com/rust-lang/crates.io-index'

Package Installed Latest Needs update
bat v0.14.0 v0.15.0 Yes
~~~~

[itkovian]

I'm going to see if git bisect yields anything.

[sharkdp]

I'm going to see if git bisect yields anything.

that would be great. We did not update the console dependency since 0.14. This would indicate that it is a bug in bat itself (or something else changed on your system, like the version of iTerm2).

[sharkdp]

Oh.. I can actually reproduce this on Linux. I have no idea how I missed this before releasing 0.15.

For some reason, I can not reproduce it in debug builds. Weird.

[sharkdp]

Now this is just great:

actual terminal size: 105

debug build:

[src/bin/bat/app.rs:173] Term::stdout().size().1 as usize = 105

release build:

[src/bin/bat/app.rs:173] Term::stdout().size().1 as usize = 61

[sharkdp]

It gets better: the binaries on the release page work just fine. They were built in release mode as well. That might explain why I didn't see this when releasing the new version.

[sharkdp]

The wrong number that shows up is actually the terminal height instead of the terminal width.

debug:

[src/bin/bat/app.rs:173] Term::stdout().size() = (
    61,
    105,
)

release:

[src/bin/bat/app.rs:173] Term::stdout().size() = (
    105,
    61,
)

[sharkdp]

Here is the code to get the terminal size (https://github.com/mitsuhiko/console/blob/0ce68cfbbff14391421a481eb8eafcb67c2af9b7/src/unix_term.rs#L22-L38):

unsafe {
    if libc::isatty(libc::STDOUT_FILENO) != 1 {
        return None;
    }

    let mut winsize: libc::winsize = mem::zeroed();

    // FIXME: ".into()" used as a temporary fix for a libc bug
    // https://github.com/rust-lang/libc/pull/704
    #[allow(clippy::identity_conversion)]
    libc::ioctl(libc::STDOUT_FILENO, libc::TIOCGWINSZ.into(), &mut winsize);
    if winsize.ws_row > 0 && winsize.ws_col > 0 {
        Some((winsize.ws_row as u16, winsize.ws_col as u16))
    } else {
        None
    }
}

the only thing I can see is that the return code of ioctl is not checked. On the other hand, there is the check for winsize.ws_row > 0 && winsize.ws_col > 0, which hopefully also acts as a guard.

[eth-p]

The wrong number that shows up is actually the terminal height instead of the terminal width.

debug:

[src/bin/bat/app.rs:173] Term::stdout().size() = (
    61,
    105,
)

release:

[src/bin/bat/app.rs:173] Term::stdout().size() = (
    105,
    61,
)

Uh... the order of the tuple is backwards in release builds? This is going to be a fun one to debug.

If there's not a #cfg(build = release) doing something strange, we might need to be doing some disassembly to figure out why that's happening.

[itkovian]

I cannot reproduce from the repo. Git bisect gave all good builds :) I did a new install and there it fails, so it might be in the dependencies? Not sure if I should post that list here, since it is quite long.

[sharkdp]

For me, Git bisect only gives me bad builds, unless I go back really far in history. Even if I build bat 0.12.1 from last year, I see the issue. I see the issue with 0.13.0 as well.

[itkovian]

I did a --release --all-features build.

[itkovian]

Differences in used crates between the git build and the crates.io build:

~~~~
✗ diff -u gits crates | rg "^[-+]"
--- gits 2020-04-27 23:04:57.000000000 +0200
+++ crates 2020-04-27 23:05:07.000000000 +0200

• Compiling bit-set v0.5.1
• Compiling bit-vec v0.5.1
• Compiling console v0.10.0
• Compiling console v0.10.1
• Compiling fancy-regex v0.3.3
• Compiling git2 v0.13.3
• Compiling git2 v0.13.5
• Compiling libgit2-sys v0.12.4+1.0.0
• Compiling libgit2-sys v0.12.5+1.0.0
• Compiling ryu v1.0.3
• Compiling ryu v1.0.4
• Compiling smallvec v1.3.0
• Compiling smallvec v1.4.0
• Compiling syn v1.0.17
• Compiling syn v1.0.18
• Compiling terminal_size v0.1.11
• Compiling xml-rs v0.8.2
• Compiling xml-rs v0.8.3
  ~~~~

[sharkdp]

• Compiling console v0.10.0
• Compiling console v0.10.1

They definitely changed the implementation in 0.10.1.

I forgot that cargo install doesn't respect the lock file anymore. I thought we were on 0.10.0

[itkovian]

Well, I can confirm my terminal had 44 rows :)

[itkovian]

They definitely changed the implementation in 0.10.1.

Versioning is hard.

[eth-p]

Conveniently, console = 0.11 came out 3 minutes ago.

[itkovian]

@eth-p How can I test this using cargo install?

[eth-p]

@sharkdp I found the commit that changed the terminal size implementation: https://github.com/mitsuhiko/console/commit/d34516686c067895f6f00ec6269219431537e28b

[sharkdp]

Ok, the issue is the following:

• console 0.10.1 was released yesterday. it changes the order of width/height (I wish both libraries would just return a struct instead of a tuple. errors like this could be easily prevented)
• before, we were building against 0.10.0 which worked fine

we can run cargo install --path . --force --locked to get the version from the Cargo.lock file (0.10.0)

[eth-p]

@itkovian

diff --git a/Cargo.toml b/Cargo.toml
index d449df6..8bfcdb4 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -39,7 +39,7 @@ regex-fancy = ["syntect/regex-fancy"] # Use the rust-only "fancy-regex" engine
 atty = { version = "0.2.14", optional = true }
 ansi_term = "^0.12.1"
 ansi_colours = "^1.0"
-console = "0.10"
+console = "0.11"
 dirs = { version = "2.0", optional = true }
 lazy_static = { version = "1.4", optional = true }
 wild = { version = "2.0", optional = true }

[eth-p]

@sharkdp If it's fine with you, I think we should just update to console = 0.11.0 and work with the new implementation (edit: assuming it was an intentional change). It would be a simple fix for us, and would eliminate the possibility of package maintainers unknowingly publishing a package with this bug.

[eth-p]

Or, alternatively, we can make an upstream issue about the breaking change and see if there are any plans to revert back to the old order?

[sharkdp]

But 0.11 is broken, too. This has to be fixed upstream. I will open an issue.

[eth-p]

@sharkdp That would depend on if the change was intentional, though. If it was, we would just have to change the logic on our end. If not, we'll have to wait a 0.11.1 release and update the dependency version.

[sharkdp]

That would depend on if the change was intentional, though

There is no release notes, no changelog, so it's hard to tell. Changing the behavior from 0.10.0 to 0.10.1 is clearly a (versioning) bug though.

[eth-p]

There is no release notes, no changelog, so it's hard to tell. Changing the behavior from 0.10.0 to 0.10.1 is clearly a (versioning) bug though.

Unfortunately, yeah.

[itkovian]

How about changing the version for console to "=0.10.0"?

Installing using the above cargo install --path . --force works nicely then.

Thanks for looking into this btw.

[sharkdp]

I now see the appeal of I now see the appeal https://github.com/sharkdp/bat/pull/881 :smile:

How about changing the version for console to "=0.10.0"?

done :+1:

[sharkdp]

Just to clarify: the weird behavior I was seeing above was not related to the difference between debug and release, but to the difference between cargo run/cargo build and cargo install. I really don't like this new behavior.

I'm going to close this ticket. Thank you @itkovian and @eth-p!