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

assets/shell-integration: improve the integration with ble.sh #1586

Closed
wants to merge 3 commits into from

Conversation

akinomyoga
Copy link

@akinomyoga akinomyoga commented Jan 24, 2022

Here are fixes for the shell integration. These are additional adjustments to 56e9a9a and 78ea214. Let me open this as a draft PR. There can be even more fixes because the check of the shell integration with ble.sh is still ongoing (Cc: @SuperSandro2000).

  • 608aad4 In case that the shell option set -u is set, the optional variables are referenced as ${varname-}
  • 9df1b53 The precmd/preexec hooks are registered only when it is not yet registered. This avoids duplicate precmd/preexec handlers after wezterm.sh is sourced multiple times. Maybe we also want to modify the code for bash-preexec?
  • e2c0816 By default, ble.sh assumes that the cursor position and the terminal contents are not changed by calling PRECMD/PROMPT_COMMAND. However, it seems the escape sequence sent from the wezterm precmd handler causes a newline when the cursor position is not at the beginning of the line. This breaks the internal layout calculation of ble.sh. To tell ble.sh that PRECMD can change the layout, we set a blesh option.

Update 2022-03-04: I'm currently waiting for rcaloras/bash-preexec#129 being processed.

@akinomyoga akinomyoga force-pushed the shell-integration-blesh branch from e2c0816 to 02708e2 Compare March 4, 2022 13:27
@wez
Copy link
Owner

wez commented Jun 16, 2022

I'm going to close this for now: when you're ready to submit it, please rebase and reopen.

@wez wez closed this Jun 16, 2022
@akinomyoga
Copy link
Author

OK! Sorry for keeping this open for a long time. I actually wanted to adjust it after rcaloras/bash-preexec#129 is settled, but it seems to take some time for it to receive another review from some other contributor of bash-preexec. I'll reopen it after rcaloras/bash-preexec#129 is finally merged or closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants