bat: Keyboard arrow keys not working for scrolling from STDIN, rather shows keycode of the key pressed

What version of bat are you using? bat 0.15.0

Describe the bug you encountered: Using keys to scroll through the long STDIN is emitting keycodes, rather than actually scrolling down. Some terminal details:-

terminal
--------
kitty 0.17.4 created by Kovid Goyal

exports(relevant ones)
---------------------
export BAT_PAGER="less -RF"
export EDITOR="nvim"
export TERMINAL="kitty"

Context

I was trying to pass the stdin from the node/npm package makemehapi to bat.

npx makemehapi print | bat

The content was a lengthy output. Thus i tried scrolling with arrow keys like i usually do. But currently the keycodes of the key pressed is displayed rather than actually scrolling down.

Pressing Enter key somehow forwards down the line, but still some junk characters are added
cat longFile | bat works just fine. Arrow keys work there.

Content with paging and error - keycodes of arrow keys displayed at bottom

28:23:18_2020-05-18_1366x768

Content with paging and `--show-all` and error - keycodes of arrow keys displayed at bottom

15:24:18_2020-05-18_1366x768

Things I have tried

commented the whole bashrc and error persists
tried bat --pager=never still no use
tried bat --no-config still no use

…

Describe what you expected to happen? … To be able to scroll through the long content using arrow keys or even Page Down/UP keys without having to press ENTER for each line.

How did you install bat? yay -S bat

Info.sh

system

$ uname -srm Linux 5.6.13-arch1-1 x86_64

bat

$ bat --version bat 0.15.0

$ env BAT_PAGER=less -RF

bat_config

$ cat /home/ikigai/.config/bat/config

--pager="less -RF"

bat_wrapper

No wrapper script for ‘bat’.

bat_wrapper_function

No wrapper function for ‘bat’.

No wrapper function for ‘cat’.

tool

$ less --version less 551 (PCRE regular expressions)

Full STDOUT of `npx makemehapi print`

  Create a server which responds to requests to /?name=Handling using a  
  template located at templates/index.html which outputs the following HTML:  
   
     <html>  
         <head><title>Hello Handling</title></head>  
         <body>  
             Hello Handling  
         </body>  
     </html>  
   
 ─────────────────────────────────────────────────────────────────────────────  
  ##HINTS  
   
  This exercise requires you to install the vision module, which is a hapi  
  plugin for rendering templates. You'll need to register the plugin in your  
  code in order to render your templates:  
   
     var Vision = require('vision');  
       
     await server.register(Vision);  
   
  The view key can be used to define the template to be used to generate the  
  response.  
   
     handler: {  
         view: "index.html"  
     }  
   
  server.views() is the server method that we use to configure the templates  
  used on our server. This method receives a configuration object in which  
  we can set different engines based on file extension. This object can also  
  set a directory path for your templates.  
   
     server.views({  
         engines: {  
             html: require('handlebars')  
         },  
         path: Path.join(__dirname, 'templates')  
     });  
   
  In this exercise, we'll be using Handlebars. To install handlebars:  
   
     npm install handlebars  
   
  With Handlebars templates, you can render a variable directly in HTML by  
  surrounding the variable with curly braces, e.g. {{foo}}.  
   
  The template receives some information from the request. For example, the  
  query parameters that were passed in via the URL are available in the  
  query object. These parameters can then be used in the template.  Query  
  params get automatically parsed and aren't declared in the route path.  
   
     <div>{{query.paramName}}</div>

About this issue

Original URL
State: open
Created 4 years ago
Reactions: 4
Comments: 19 (7 by maintainers)

Commits related to this issue

Fix #1006 by loading assets lazily to start pager quicker — committed to Enselic/bat by Enselic 4 years ago

Most upvoted comments

Sorry for my slow pace, but I have a full-time job and a family, so spare time is a bit limited 😉

After quite a bit of research and debugging involving custom builds with custom debug prints in both node and less, I now have a firm grasp of what’s going on. I can explain all the behavior we see. But first the main point:

Summary =======

This issue should be fixable, or at least greatly mitigated, by making bat start its pager much earlier.

Details =====

Primer on input from terminals

Terminals can be in many modes, but the most common are commonly called “raw mode” and “canonical mode”. In “raw mode”, a read() call on a terminal file descriptor will return what was typed as soon as a key is pressed, and typed characters will not be echoed back. In “canonical mode”, the read() call wait for a newline character to be typed before returning what was typed, and characters are echoed back to the terminal as they are typed.

Note that the terminal mode is per-terminal. This means that if two processes use the same terminal, the terminal mode is shared, even if different file descriptors in different processes are used.

How less uses terminal modes

When less starts, it will open a file descriptor to /dev/tty, i.e. the current terminal. Then it stores the current terminal mode in a global variable, and then change the terminal mode to “raw mode”. Then it starts waiting for key presses with read(). Changing to “raw mode” is what makes it possible to scroll in less with individual KeyDown presses. Without “raw mode”, it would be necessary to press KeyDown and then Return to scroll down one line. Just before the less process quits, it will reset the terminal mode to what it was when less was started. This is necessary since the terminal mode is per-terminal, and not per-process. It is simply a necessary restoration of global, shared state.

How node uses terminal modes

When node starts, it will also store the current terminal mode. It will do it for stdin and stderr, but since those are bound to the current terminal, it will be the same terminal as less is using. Then, right before node exists, it will reset the terminal mode to what it was when node started, since it might have changed the modes due to JS scripts. This is good practice and it should not stop doing that. As mentioned above, less also behaves like this.

How the original problem occurs

The problem occurs like this. Note that less and node operates independently, so this is all very racy.

node reads current terminal mode as “canonical mode” and stores that to restore it later
less reads current terminal mode as “canonical mode” and stores that to restore it later
less changes current terminal mode to “raw mode”
node exits and restore current terminal mode to “canonical mode”
Now when less calls read(), the method will block until a newline is typed.
less exits and restore current terminal mode to “canonical mode” (which it already is in)

How making bat start less quicker will fix the problem

Assuming bat can start less as good as quick as pure less, this is what will happen:

less reads current terminal mode as “canonical mode” and stores that to restore it later
less changes current terminal mode to “raw mode”
node reads current terminal mode as “raw mode” and stores that to restore it later
node exits and restore current terminal mode to “raw mode”
All is well. When less calls read(), it will return as soon as e.g. KeyDown is pressed, since terminal mode is still “raw mode”
less exits and restore current terminal mode to “canonical mode”

Explanation of reproducibility examples

The reason node -e 'for (var i = 0; i <= 10000; i++) { console.log(i); }' | bat fails is the same reason as above.

The reason node -e 'for (var i = 0; i <= 1000; i++) { console.log(i); }' | bat’ is NOT buggy is because node starts and exits fully BEFORE less even starts. So node can’t mess up the terminal mode for less, since they run in series and not in parallel.

The reason node -e 'for (var i = 0; i <= 100000; i++) { console.log(i); } | bat’ is NOT buggy is… false. It is actually buggy. It’s just that you have to scroll down enough lines (in my case 88000) to make node exit. If you don’t scroll down, node will wait for the stdout buffer to leave room for more data, and not quit. As soon as node has no more data to write, it will quit, and the bug will occur.

The reason node -e 'for (var i = 0; i <= 20000; i++) { console.log(i); }' | less is not buggy is that less starts quick enough to change the terminal mode before node reads it. Since less is quick enough to change the terminal mode node will restore the terminal mode to “raw mode” when it exits, since that was what it was when it started. Since less was so quick… This is the working scenario that I describe above.

You will get the same bug with pure less if you just delay the start of less. Try this and scroll down to the bottom, and you will see that pure less also is “buggy”: node -e 'for (var i = 0; i <= 20000; i++) { console.log(i); }' | (sleep 1; less) (assuming your system doesn’t have big enough stdin buffer with room for all 20000 lines from node).

The reason that std::thread::sleep(std::time::Duration::from_millis(10)); is needed is to make less start slow enough to give node time to read the current terminal mode as “canonical mode”. If you remove the sleep, less is able to start and change terminal mode before node reads it the first time, so everything works. I.e. the working scenario I describe above.

Closing words of this comment

Ok, that was a mouthful… Let me know if you have any questions, I will gladly elaborate on the details or answer any other questions you might have.

I think the next step here is a proof-of-concept patch that makes bat start less much quicker, which should solve the issue. Maybe I’ll take a stab at it. We’ll see 😃

+10

Enselic on Dec 9, 2020

Sorry for somewhat spamming this bug report, but I’ve started to obsess about getting to the bottom of this bug.

Latest news is that this seems to be a regression in node v12.5.0. I can’t reproduce with node v12.4.0, but can easily reproduce in node v12.5.0.

Looking at the changelog of node v12.5.0 I found this very suspicious bug fix: https://github.com/nodejs/node/pull/24260 src: restore stdio on program exit

They are messing around with file descriptors 0-2, i.e. stdin among other things. And since they ostensibly do it upon exiting node, it would explain the dependence upon specific timing (the exit of node) and thus the need to have sleeps in the repro code.

I’ll see if I can come up with a convincing bug report for the node project. To be continued…

Enselic on Dec 7, 2020

I came up with the following when looking for a minimal node program. Seems to be something intrinsic to node, and somehow timing/buffer-size specific, because I can reproduce the problem with this:

NODE_DISABLE_COLORS=1 node -e 'for (var i = 0; i <= 10000; i++) { console.log(i); }' | bat

but if I change the end condition to either 1000 or 100000, i.e. either higher or lower, I can not reproduce! However, taking node out of the equation, all numbers work fine: 1000, 10000 and 100000:

for i in $(seq 0 10000); do echo $i; done | bat

So, very strange. Do you see the same thing? Maybe the number used to reproduce depends on how fast your machine is or something weird like that. For the record, I use the following versions, on a MacBook Pro from 2018.

% node --version
v12.18.4
% bat --version
bat 0.17.1

Enselic on Dec 5, 2020

OS: Linux (Arch) | bat: 0.15.4

I have the same issue with the output of several Node.js (v14.9.0) programs, e.g.

Doesn’t happen with all Node.js programs, though. e.g. these are fine:

jsesc
json5

It’s not caused by a specific third-party dependency because, e.g., pidtree doesn’t have any.

chocolateboy on Sep 16, 2020

Hi, I got the same issue today with the AWS cdk cli when piping the output of cdk synth to bat

jgsqware on Sep 7, 2021