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

[kitty] ble.sh not working with the kitty terminal #110

Closed
NoahGorny opened this issue May 15, 2021 · 35 comments
Closed

[kitty] ble.sh not working with the kitty terminal #110

NoahGorny opened this issue May 15, 2021 · 35 comments
Labels
External Problem/Bug Problems/Bugs of other projects

Comments

@NoahGorny
Copy link
Contributor

ble version: 0.4.0-devel3+750ca38
Bash version: 5.0.17(1)-release

Hello there!
I tried to use ble.sh with the kitty terminal, but when ble.sh was loaded, the terminal got messed up. Ran commands, yet I could not see any output, or any line executed..

If you want, I can try to add screenshots. In any case, I think it is easily reproducible by running kitty and loading ble.sh

In any case, this seems like a very cool project. I think about adding it as a plugin for bash-it 😄

@akinomyoga
Copy link
Owner

akinomyoga commented May 15, 2021

Hello! Thank you for the report! It seems there is no problem in my Kitty on Ubuntu 20 LTS in VirtualBox. I have questions:

  • Q1: Have you tried other terminals? Is the problem only reproduced in Kitty?
  • Q2: What is your TERM environment variable? Edit Do you have a corresponding entry in terminfo/termcap?
$ echo "$TERM"
$ tput cols; echo $?
  • Q3: What is your operating system? What is the output of echo $MACHTYPE? If it is Linux, which distribution do you use?
$ echo "$MACHTYPE"
$ uname -a
$ cat /etc/*-release # if you are in Linux
  • Q4: Does it reproduce with a plain Bash?
$ INPUTRC=/dev/null bash --norc --noprofile
$ source /path/to/ble.sh

@NoahGorny
Copy link
Contributor Author

Hello! Thank you for the report! It seems there is no problem in my Kitty on Ubuntu 20 LTS in VirtualBox. I have questions:

  • Q1: Have you tried other terminals? Is the problem only reproduced in Kitty?
  • Q2: What is your TERM environment variable? Edit Do you have a corresponding entry in terminfo/termcap?
$ echo "$TERM"
$ tput cols; echo $?
  • Q3: What is your operating system? What is the output of echo $MACHTYPE? If it is Linux, which distribution do you use?
$ echo "$MACHTYPE"
$ uname -a
$ cat /etc/*-release # if you are in Linux
  • Q4: Does it reproduce with a plain Bash?
$ INPUTRC=/dev/null bash --norc --noprofile
$ source /path/to/ble.sh

Hi @akinomyoga
It seems like I can not reproduce the problem, after opening a new instance of kitty, everything works as expected 😄
Thank you for your detailed answer, and I look forward to use ble.sh on a regular basis 😃

@NoahGorny
Copy link
Contributor Author

Hmm, after another look, it looks like the problem persists..
It works well with gnome-terminal however.

TERM=xterm-kitty
tput cols=134
Linux version 5.8.0-050800, Ubuntu 20.04.2 LTS
It is reproducible with plain bash.
simplescreenrecorder-2021-05-16_09 26 46

@NoahGorny NoahGorny reopened this May 16, 2021
@akinomyoga
Copy link
Owner

akinomyoga commented May 16, 2021

Hmm, thank you. Maybe it is related to the prompt.

  • Q5: What happens if yous set PS1='\$ ' when the problem is occurring?
  • Q6: Edit: What is your PS1?

@NoahGorny
Copy link
Contributor Author

PS1=\[\e[0;32m\]\u\[\e[0m\]@\[\e[34;1m\]\h\[\e[0m\]:\[\e[0;33m\]\w\[\e[0m\]\[\e[0;32m\] $ \[\e[0m\]

The problem is still there when I set PS1 to this sadly 😢

@akinomyoga
Copy link
Owner

akinomyoga commented May 16, 2021

Thank you! Hmm... ble.sh actually doesn't use the [ builtin command but always uses the [[ construct. So it must be injected by something external.

  • Q7: What if you cleared all the environments?
$ env -i INPUTRC=/dev/null bash --norc --noprofile
$ source /path/to/ble.sh --norc --noinputrc

Edit: Maybe you have already set up something in ~/.blerc? In that case, please also try --norc and --noinputrc options on sourcing ble.sh (as above).

@NoahGorny
Copy link
Contributor Author

Still does not work. It seems to happen when I reach the end of the screen with the prompt, or with the first two lines.

@akinomyoga
Copy link
Owner

akinomyoga commented May 16, 2021

Thanks. It still doesn't reproduce in my environment with a long prompt (though I found an unrelated problem) and also at the top and bottom lines of Kitty.

  • Q8: What if you change TERM such as TERM=xterm-256color in Kitty when the problem is happening?

@akinomyoga
Copy link
Owner

akinomyoga commented May 16, 2021

  • Q9: What is your Kitty version? Have you tried the latest version of Kitty?
  • Q10: What is your kitty.conf? Does the problem reproduce by an empty kitty.conf? This is mine:
$ kitty --version
kitty 0.20.3 created by Kovid Goyal
$ cat ~/.config/kitty/kitty.conf
font_family Ubuntu Mono
bold_font auto
italic_font auto
bold_italic_font auto

akinomyoga added a commit that referenced this issue May 16, 2021
@akinomyoga
Copy link
Owner

akinomyoga commented May 16, 2021

I have created a commit 2e14354 for testing and pushed it to debug110 branch.

  • Q11: Could you checkout the branch debug110 and see how the output looks like when the error message bash: [: =: unary operator expected is present? Screen capture is OK. I would like to identify where the error message bash: [: =: unary operator expected is produced.

@NoahGorny
Copy link
Contributor Author

I have created a commit 2e14354 for testing and pushed it to debug110 branch.

  • Q11: Could you checkout the branch debug110 and see how the output looks like when the error message bash: [: =: unary operator expected is present? Screen capture is OK. I would like to identify where the error message bash: [: =: unary operator expected is produced.

My kitty terminal was outdated- and after updating to kitty 0.20.3, the problem was fixed 😄

However, there is still this output from the unary operator after I echo. Here is the output from the branch:

noah@noahs-ubunto[±|debug110 → noah ✓]:~/open-source/ble.sh $ echo bababa
bababa
[U1][U2][U2a][U2b]bash: [: =: unary operator expected
[U2c][U2d][U3][U4][I1][I2][I3][I3a][I3b][I4][I4d][U5]
[U6]
[R.W1][R.W2][R.W3][R.W4a][R.W5]
noah@noahs-ubunto[±|debug110 → noah ✓]:~/open-source/ble.sh $ [U1][U2][U2a][U2b]bash: [: =: unary operator expected
[U2c][U2d][U3][U4][I1][I2][I3][I3a][I3b][I4][I4d][U5]
[U6]
[R.W1][R.W2][R.W3][R.W4a][R.W5]

noah@noahs-ubunto[±|debug110 → noah ✓]:~/open-source/ble.sh $ echo bobo
bobo
[U1][U2][U2a][U2b]bash: [: =: unary operator expected
[U2c][U2d][U3][U4][I1][I2][I3][I3a][I3b][I4][I4d][U5]
[U6]
[R.W1][R.W2][R.W3][R.W4a][R.W5]
noah@noahs-ubunto[±|debug110 → noah ✓]:~/open-source/ble.sh $ 

@akinomyoga
Copy link
Owner

akinomyoga commented May 16, 2021

My kitty terminal was outdated- and after updating to kitty 0.20.3, the problem was fixed 😄

Oh, OK! Great.

However, there is still this output from the unary operator after I echo. Here is the output from the branch:

OK. It means that it was actually a combined problem of two problems. It's confusing!

[U2b]bash: [: =: unary operator expected
[U2c]

This indicates that PROMPT_COMMAND has some problems.

  • Q12: Do the unary operator error messages disappear after PROMPT_COMMAND=?
$ PROMPT_COMMAND=
  • Q13: What is your PROMPT_COMMAND (and its attributes)?
$ declare -p PROMPT_COMMAND
  • Q14: Do the error messages reproduce when ble.sh is loaded but not attached?
$ bash # start new session
$ source /path/to/ble.sh --noattach

@NoahGorny
Copy link
Contributor Author

So, after debugging the problem a bit:
I use pre-exec vendored from bash-it, so my prompt command:

declare -- PROMPT_COMMAND="__bp_precmd_invoke_cmd
preexec_set_exit;preexec_invoke_cmd
__bp_interactive_mode"

I saw that running __bp_precmd_invoke_cmd caused the wierd unary ... message to appear. After looking at the code, it called pure_prompt which is the theme I use in my bash shell. Inside, there was a call to scm_prompt:

scm_prompt() {
    CHAR=$(scm_char)
    if [ $CHAR = $SCM_NONE_CHAR ]
        then
            return
        else
            echo "[$(scm_char)$(scm_prompt_info)]"
    fi
}

It seemed like the echo call was the problematic one.. After adding -e it was fixed 😄
Thanks for all the help @akinomyoga!

There is another problem that I encountered. When you call exec bash from the terminal after loading ble.sh, all key events are just printed to the terminal, and not processed 😢
I can open this on another issue if you wish

@akinomyoga
Copy link
Owner

OK!

scm_prompt() {
    CHAR=$(scm_char)
    if [ $CHAR = $SCM_NONE_CHAR ]
        then
            return
        else
            echo "[$(scm_char)$(scm_prompt_info)]"
    fi
}

It seemed like the echo call was the problematic one.. After adding -e it was fixed 😄

Hmm... is it really -e that solved the issue? I don't see the reason. Naively thinking, I guess it was caused by [ $CHAR = $SCM_NONE_CHAR ] when $CHAR becomes an empty string. [ = xxx ] causes the error message bash: [: =: unary operator expected.

I'm also wondering why it didn't reproduced with the plain Bash but only reproduced in ble.sh.

Thanks for all the help @akinomyoga!

You're welcome!

There is another problem that I encountered. When you call exec bash from the terminal after loading ble.sh, all key events are just printed to the terminal, and not processed 😢
I can open this on another issue if you wish

Oh, thank you for the information! Hmm... again it doesn't reproduce in my environment. Do you use romkatv/gitstatus? If so, could you try to update it to the latest git version as I have recently reported an issue of exec replaced by gitstatus?

@NoahGorny
Copy link
Contributor Author

I indeed use gitstatus, but even after updating it, or even when disabling it.
I also tried to use a clean environment with plain bash, and it also happened there. I tried to use gnome-terminal, and the bug did not happen there at all!

Seems like another kitty problem, yet this time it is with the latest version 😢

@akinomyoga
Copy link
Owner

Hmm, interesting. There is no problem with exec bash in my Kitty. I'm not sure if it is again the KItty issue. Does the problem still reproduce with the following combination of commands?

$ stty sane; exec bash

Also,

$ ble-detach
[ble: detached]
Please run `stty sane' to recover the correct TTY state.
$ stty sane;
$ exec bash

@NoahGorny
Copy link
Contributor Author

It happens after ble-detach, so I can not run stty sane; exec bash afterwards. Even without detaching, stty sane; exec bash doesnt work either. It seems like most of the keyboard events are not interpreted, and just printed as special characters (pressing enter, pressing ctrl+C, etc..). Restarting the terminal using ctrl+shift+del removes the misbehaviour

@akinomyoga
Copy link
Owner

OK, I could partially reproduce the issue. This is a bug of Kitty or a quirk of Kitty. I added a work around f599525.

It seems like most of the keyboard events are not interpreted, and just printed as special characters

So, this doesn't mean something trivial like ^C for C-c but more non-trivial ones? Also, normal characters are not affected.

@NoahGorny
Copy link
Contributor Author

OK, I could partially reproduce the issue. This is a bug of Kitty or a quirk of Kitty. I added a work around f599525.

It seems like most of the keyboard events are not interpreted, and just printed as special characters

So, this doesn't mean something trivial like ^C for C-c but more non-trivial ones? Also, normal characters are not affected.

This commit indeed fixed the problem. Maybe this is something we should report to kitty?

Thanks for all of your help @akinomyoga! your dedication is truly amazing 😄

@NoahGorny
Copy link
Contributor Author

By the way, by adding ble.sh to Bash-it, I mean adding a plugin that locates and loads ble.sh. This is purely for comfort of Bash-it users, and I hope it will encourage more people to use ble.sh 😄

Closing this issue now, expect a PR with a new plugin in Bash-it soon 😃

@akinomyoga
Copy link
Owner

akinomyoga commented May 16, 2021

This commit indeed fixed the problem.

OK! Thank you for your confirmation!

Maybe this is something we should report to kitty?

Maybe, but I'm not sure if Kovid will change the behavior of Kitty. The problem is the behavior of the terminal feature called modifyOtherKeys. In particular the interpretation of the escape sequence \e[>4;1m. This escape sequence was first introduced by Xterm, and other terminals followed its behavior. But actually, the behavior is not well documented in the manual of Xterm. Nevertheless, the implementations by Mintty, RLogin, etc. treat \e[>4;1m pretty much the same as Xterm (Roughly speaking, \e[>4;1m turns off modifyOtherKeys mode). Kitty fails to reproduce the behavior of Xterm (It seems \e[>4;1m turns on modifyOtherKeys mode in Kitty). But I'm not sure Kovid will follow others. In my impression, he doesn't like to follow others unless there is clearly established standard. Here are also the related discussions and materials by terminal people:

By the way, by adding ble.sh to Bash-it, I mean adding a plugin that locates and loads ble.sh. This is purely for comfort of Bash-it users, and I hope it will encourage more people to use ble.sh 😄

Oh! Thank you very much! When you add ble.sh configuration to bash-it, please let me know that. Actually, basically ble.sh wants to be loaded directly from ~/.bashrc. If one wants to load ble.sh from other places, there are several points that the user needs to be careful.

Fix (2021-11-23): Fix typos \e[4;1m -> \e[>4;1m

@akinomyoga akinomyoga changed the title ble.sh not working with the kitty terminal [Kitty] ble.sh not working with the kitty terminal May 16, 2021
@akinomyoga akinomyoga added the External Problem/Bug Problems/Bugs of other projects label May 17, 2021
@akinomyoga akinomyoga changed the title [Kitty] ble.sh not working with the kitty terminal [kitty] ble.sh not working with the kitty terminal May 29, 2021
@kovidgoyal
Copy link

kovidgoyal/kitty#4075

kitty will no longer respond to CSI > 4; 1 m

And FYI if you want proper key support in kitty, see https://sw.kovidgoyal.net/kitty/keyboard-protocol/ (also being implemented in foot and iterm2 kovidgoyal/kitty#3248)

@akinomyoga
Copy link
Owner

@kovidgoyal Thank you for the information! ble.sh currently uses CSI > 4 m to turn off modifyOtherKeys for kitty as a workaround, so it does not use CSI > 4; 1 m inside kitty.

It seems kitty still responds to CSI > 4 m and CSI > 4 ; 2 m (in kitty 0.23.1), but will the support for CSI > 4 m and CSI > 4; 2 m also be removed as well as that for CSI > 4; 1 m? --> OK, I found the commit kovidgoyal/kitty@d6a43a7. CSI > Pm m will be completely removed in the next release version, kitty 0.24?

I have added the support for the kitty keyboard protocol (using CSI > 1 u and CSI < u) in commit ec91574. Now ble.sh uses the kitty protocol for kitty version 0.23+. The older kitty may have also been supported the kitty protocol, but I haven't tested ble.sh against the older versions of kitty, so the support is currently only enabled from 0.23.

Question on the number range reserved by kitty in the DA2 response

@kovidgoyal I have a question on the terminal identification of kitty. Currently, kitty (of the version 0.XX.Y) responds to DA2 request with the response of the form CSI > 1; 4000; XX c. Is there any plan that the number "4000" will be changed (for example, when kitty version becomes 1.XX.Y)? I'd like to know the possible range of the number used by kitty such as 4000--4010 or 4000--4100, etc. I'm currently accepting 4000--4100 as kitty, but a wider range increases the chances of mistakenly recognizing other terminals as kitty, so I'd like to minimize the range if possible.

Question/suggestion on ctrl+shift+* keybindings of kitty

@kovidgoyal In testing the behavior of ble.sh in kitty, I was first confused by the fact that the most of the key events of ctrl+shift+* cannot be captured by ble.sh even when modifyOtherKeys or kitty's comprehensive keyboard mode is enabled. I finally realized that actually most of the key combinations of ctrl+shift+<xxxx> are already used by kitty UI. I have finally tested the behavior of ble.sh by putting a bunch of map ctrl+shift+<xxxx> no_op in .config/kitty/kitty.conf, but I think it is useful to provide an option to turn off these kitty UI bindings when the comprehensive keyboard mode is enabled. What do you think?

Another possibility might be to provide an option for a kind of kitty-UI modifier key just like the prefix key used in tmux and GNU screen.

If you don't like all of them, I would like to have a configuration of disabling kitty-UI keybindings by a couple of lines (instead of a bunch of lines of ctrl+shift+a..ctrl+shift+z, ctrl+shift+0...ctrl+shift+9, ...). Something like

# kitty.conf
map ctrl+shift+<all> no_op

@kovidgoyal
Copy link

kovidgoyal commented Nov 23, 2021 via email

@kovidgoyal
Copy link

Oh and since ble.sh is a readline substitute, you may want to add support for the upcoming shell integration in kitty, see kovidgoyal/kitty#3948 (read the shell_integration.rst docs to learn how to support it in your code)

@akinomyoga
Copy link
Owner

It's not going to change.

OK, thank you! Probably I will narrow the range to something like 4000..4009.

However, if you want to do terminal detection via escape code use XTGETTCAP with which you can query the actual
terminal name instead of DA2. kitty even implements extensions to XTGETTCAP to query the values of various user settable options. https://sw.kovidgoyal.net/kitty/kittens/query_terminal/

Hmm, actually I'm willing to switch to a new terminal-identification scheme once the standard one is settled and most terminal supports the scheme. However, it seems like the new terminal identification protocol doesn't converge well in the mintty discussion and the Terminal WG discussion (I thought XTVERSION CSI > 1 q implemented by mintty and xterm will be supported by other terminals, but still many terminals don't support them). Unfortunately, currently the terminal identification scheme that the terminals implement most is still DA2, so ble.sh uses it at present (Of course, we might combine multiple schemes, but I don't want to unnecessarily increase the round trips).

https://sw.kovidgoyal.net/kitty/conf/#opt-kitty.kitty_mod

Oh! Thank you for the pointer! I haven't noticed the option! This is exactly what I imagined from the prefix keys of tmux and screen.

Oh and since ble.sh is a readline substitute, you may want to add support for the upcoming shell integration in kitty, see kovidgoyal/kitty#3948 (read the shell_integration.rst docs to learn how to support it in your code)

OK! I'd later look at it. I'd expect the integration code for Bash shell-integration/kitty.bash just works with ble.sh, but there may be some troubles. I'll later try it.

@kovidgoyal
Copy link

You dont need multiple round trips. What you do is send all the queries you want, the last being a DA or DA2. Then you wait till you get the response for the last one. And check to see if you have responses for any others as well. So only one roundtrip is needed.

And yes 4000...4009 should be fine for a long time. Though I stronly encourage you to use XTVERSION, XTGETTCAP and friends in additions to DA2

@akinomyoga
Copy link
Owner

You dont need multiple round trips. What you do is send all the queries you want, the last being a DA or DA2. Then you wait till you get the response for the last one. And check to see if you have responses for any others as well. So only one roundtrip is needed.

Ah, maybe you're right for most practical cases. The problem is that still some old terminals and kernel consoles print the unrecognized control sequences on the screen, in which case modern control sequences can be sent only when DA2 is ambiguous. Also, ble.sh (which is implemented by shell script) is slow, so I'm afraid the parsing time of many control sequences might affect the user experience on the shell startup. But, yeah, I can evaluate the impact after I implement the multiple terminal-identification sequences, but I just don't have time to test them in various terminals.

Thank you for your advice.

@kovidgoyal
Copy link

kovidgoyal commented Nov 23, 2021 via email

@akinomyoga
Copy link
Owner

@kovidgoyal Thank you!

@dankamongmen
Copy link

Just FYI, @dankamongmen recently improved the parser for the linux kernel console and some other terminals so that he could implement exactly this strategy in notcurses.

this isn't generally a problem, because you can:

  • go to wherever you're going to start emitting glyphs
  • issue your query string
  • following replies, cup back wherever you started

the cup might be a no-op, or it might place you back where the missed escapes got echoed. either way, it's usually enough to wipe them out, since you're going to start emitting glyphs there. throw in a cursor location request at the beginning of the query string if you don't otherwise know where you intend to start output. good luck!

@akinomyoga
Copy link
Owner

@dankamongmen Thanks! Yeah, that is also a valid approach. Actually, ble.sh uses that approach at the startup to determine the terminal's East_Asian_Width=A mode and the Unicode version on which the terminal or wcwidth, etc. relies on (you can find the related codes here). The reasons that I still use DA2 are, as I have already mentioned, 1) partly because ble.sh is slow and I wanted to reduce the size of communications at the startup and 2) partly because I don't have enough time but there are no real problems now with the current implementation.

Also, maybe I can say it is related to the current structure of ble.sh, i.e., the code to print something to the terminal and the code to identify the terminal are currently placed in different modules (src/canvas.sh and src/util.sh, respectively), so that the terminal-identification code can be reused. Because of this structure, actually the module util does not know what and where the other modules print characters to the terminal. I think it is not so hard to properly handle this in this case, but it increases additional bindings between the modules or constraints on the usage.

Anyway, I may implement XTVERSION, etc. if I have enough time, but the priority is currently low because there are no real problems with DA2 now. Probably, I will finally become to need to implement them in future for the proper grapheme-cluster support (cf #117), etc., where every terminal seems to have slightly different behavior as far as I have tested several months before.

@dankamongmen
Copy link

dankamongmen commented Nov 24, 2021

best of luck; i would recommend you simply use Notcurses, which i have zealously optimized with regards to the initial terminal detection (and most other things), but i understand your project is aiming for an all-shell implementation =].

it makes me wonder whether it would make sense to publish a little utility with notcurses which simply exported all detected capabilities (using queries or terminfo or both) in a machine-readable form, saving projects like yours the dirty work therein. would you have any interest in such a tool, or are you determined to use only your own scaffolding?

@akinomyoga
Copy link
Owner

Thank you.

it makes me wonder whether it would make sense to publish a little utility with notcurses which simply exported all detected capabilities (using queries or terminfo or both) in a machine-readable form, saving projects like yours the dirty work therein.

I believe that would be definitely useful in general.

would you have any interest in such a tool, or are you determined to use only your own scaffolding?

Specifically for ble.sh, I think I won't add dependencies on external non-POSIX tools (other than Bash). The reason for the all-shell implementation of ble.sh is that I wanted to reduce the barrier of the introduction [Sorry, I'm not sure what is the proper word choice in English] of ble.sh. So, anyway I implement necessary terminal things with shell script. Nevertheless, when ble.sh detects the Notcurses version of tput-like or infocmp-like utility, I can think of using it as a more reliable choice.

Oh, I think I can use the utility from Notcurses as another implementation of tput. ble.sh caches the result of tput when tput is available. Currently, ble.sh supports the tput accepting terminfo names (such as the one from ncurses) and the tput accepting termcap names (such as that of BSD). I think it would be useful to add the Notcurses utility as the third option. In that case, "the machine-readable form" should be very simple one that can be easily handled by shell scripts (instead of JSON, XML or other structured formats).

@dankamongmen
Copy link

i shall hack it up, then!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Problem/Bug Problems/Bugs of other projects
Projects
None yet
Development

No branches or pull requests

4 participants