Skip to content

Conversation

@lalvarezt
Copy link
Contributor

solves #586

@lalvarezt
Copy link
Contributor Author

waiting for #589

@lalvarezt lalvarezt force-pushed the shell-integration-fix branch 2 times, most recently from 3537f4c to ae3477d Compare July 4, 2025 12:09
@lalvarezt
Copy link
Contributor Author

lalvarezt commented Jul 4, 2025

@alexpasmantier please have a look, makes the shell_integration usable with a partial height

EDIT: unfortunately we have to make this dance because crossterm Inline is a second class citizen, perhaps it will improve in the future.

@alexpasmantier alexpasmantier force-pushed the main branch 9 times, most recently from fb42495 to f60b492 Compare July 4, 2025 17:51
@alexpasmantier alexpasmantier force-pushed the shell-integration-fix branch from ae3477d to 17c21f0 Compare July 4, 2025 18:19
@lalvarezt lalvarezt force-pushed the shell-integration-fix branch from a4a8186 to 02ae59c Compare July 5, 2025 19:26
@alexpasmantier alexpasmantier force-pushed the shell-integration-fix branch from 6736e47 to cbf50dd Compare July 5, 2025 21:20
@alexpasmantier
Copy link
Owner

alexpasmantier commented Jul 5, 2025

@lalvarezt Found a working solution cbf50dd

Side note: Also figured that updating this makes the UX a bit nicer for zsh.

Would you mind giving this a try and telling me if it works as expected on your machine?

@alexpasmantier alexpasmantier force-pushed the shell-integration-fix branch from cbf50dd to e98c375 Compare July 5, 2025 21:39
@lalvarezt
Copy link
Contributor Author

lalvarezt commented Jul 6, 2025

oh nice 👏, but I'm seeing this weird new line again (in fish), specifically when right at the bottom of the screen

inline

And this in zsh

on   ➜ zsh
FedoraLinux42-wsl% source ./television/utils/shell/completion.zsh
FedoraLinux42-wsl% nvim
Error: The cursor position could not be read within a normal duration
FedoraLinux42-wsl% nvim $'\033'\[6n

@lalvarezt
Copy link
Contributor Author

made the resizing a much simpler implementation, yes we lose the context from the shell previous commands but being honest while resizing it's not a big deal, IMO
This way it works just as the full screen version and we don't have to manually adjust for every situation

@lalvarezt lalvarezt force-pushed the shell-integration-fix branch from 5d1bcd1 to 4aec703 Compare July 6, 2025 08:23
make inline and fixed start x position consistent
@lalvarezt lalvarezt force-pushed the shell-integration-fix branch from 4aec703 to 201b98b Compare July 6, 2025 08:47
@alexpasmantier
Copy link
Owner

For the cursor position error:

Are you sure you're recompiling the release binary ? Because that's what's being used by the shell script.

@alexpasmantier
Copy link
Owner

Regarding resizing:

Was the actual implementation broken? I don't feel we gain that much simplicity by dropping the feature so might as well keep it if it's working as intended.

@lalvarezt
Copy link
Contributor Author

lalvarezt commented Jul 6, 2025

For the cursor position error:

Are you sure you're recompiling the release binary ? Because that's what's being used by the shell script.

I patched the zsh file to use the actual implementation, yes

Edit: as you can see I don't use zsh anymore so this just the plain installation with nothing else in it

Edit 2: there's also the thing of the passthrough escape code. For some reason is not being processed

@lalvarezt
Copy link
Contributor Author

lalvarezt commented Jul 6, 2025

Regarding resizing:

Was the actual implementation broken? I don't feel we gain that much simplicity by dropping the feature so might as well keep it if it's working as intended.

Yes I'm seeing virtual artifacts. Not sure if that's visible with asciinema. You can try on your side to replicate, open a new terminal session with the screen cleared (the term emulator not in full-screen), open tv --height 20, then horizontally expand. The lines above the prompt get mangled. If you expand vertically (expand) works ok from what I've seen.

Edit: I'm using Wezterm nightly

@alexpasmantier
Copy link
Owner

alexpasmantier commented Jul 6, 2025

Screenshot From 2025-07-06 12-01-13

Screencast.From.2025-07-06.11-58-23.mp4

I'm getting the extra line too (will take a look at why that's happening), but the --inline is working as expected with the zsh shell integration.

@alexpasmantier
Copy link
Owner

There's also a bug in the zsh script where leaving tv without selecting anything still introduces an empty completion item

@lalvarezt
Copy link
Contributor Author

There's also a bug in the zsh script where leaving tv without selecting anything still introduces an empty completion item

interesting, for in zsh, I don't get the neither the newlines (not even close to the bottom edge) nor the empty selection

television.mp4

@alexpasmantier
Copy link
Owner

Ah okay, same here, nevermind might have gotten mixed up 👍🏻

So in the current state of things I believe everything is working as intended right?

The only minor thing left to do being to figure out how to keep the current prompt when using --inline without having the newline issue.

@lalvarezt
Copy link
Contributor Author

@alexpasmantier pushed some tweaks, seem to work on my side in all conditions, can you please check

@alexpasmantier
Copy link
Owner

@lalvarezt works perfectly on my side 🎉

(except for the fact that tv overwrites the prompt which isn't the best UX)

Screencast.From.2025-07-06.16-03-02.mp4

@lalvarezt
Copy link
Contributor Author

@lalvarezt works perfectly on my side 🎉

(except for the fact that tv overwrites the prompt which isn't the best UX)

Screencast.From.2025-07-06.16-03-02.mp4

Ah, yes. Since I use a two line prompt I didn't think too much about it. I'll have a look later

@alexpasmantier
Copy link
Owner

Let's merge this then, good job that was a tough one 👍🏻

@alexpasmantier alexpasmantier merged commit dde3193 into alexpasmantier:main Jul 6, 2025
4 checks passed
@lalvarezt lalvarezt deleted the shell-integration-fix branch July 6, 2025 18:28
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