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

Fix: Interactive Commands #72

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

CamKem
Copy link
Contributor

@CamKem CamKem commented Feb 20, 2025

This pull request fixes the issues with interactive commands like Tinker by handling & removing the alert (BEL) sequence and temporarily playing the alert sound, which was getting added to the buffer and causing rendering & repetition of the alert sound issues.

TODO

  • Add test coverage to ensure conversion
  • Confirm behaviour on terminal tab focus change

fixes #54

#70 should be merged first, the rendering issues are dependant on the behavioural changes in that PR.

@CamKem CamKem marked this pull request as draft February 21, 2025 00:10
@CamKem
Copy link
Contributor Author

CamKem commented Feb 21, 2025

Need to work on how interactive mode is handled when the tab or window focus is changed and then back to the active window/tab, some odd output going on.

@CamKem
Copy link
Contributor Author

CamKem commented Feb 25, 2025

Working on input tracking for interactive mode, will track the number of keystroke & on carriage return (enter) we will clear the tracking. This will allow us to prevent the backspace, or cursor movement for interactive mode trying to go beyond where the input should let it.

Tested in a branch merged with #70 and worked well so far.

Still to fix:

  • up/down arrow to move through text, needs to update the character tracking so we know how far the cursor can move.
  • Somehow track how far up you can scroll through past input, as it gets angry if you go too far.

@CamKem CamKem force-pushed the feat/backspace-moving-cursor branch from 20d1fbb to 2022198 Compare February 25, 2025 22:26
@CamKem CamKem changed the title Fix: Convert raw backspace to ansi cursor backwards Fix: Interactive Commands Feb 28, 2025
@CamKem
Copy link
Contributor Author

CamKem commented Feb 28, 2025

Have added an intercept of the bell command at the screen level which I am hopeful will avoid having to emulate too much of the readline for interactive commands in our multiplexer.

@CamKem
Copy link
Contributor Author

CamKem commented Mar 1, 2025

The addition of the handling the bell sequent, as mentioned yesterday, actually does handle the issues caused by the command running in the screen process, which means all that emulation of the readline history state at the top level is not needed.

Will just add tests & is ready to merge, will fix interactivity.

I also want to look into adding a ESC[?25h (make cursor visible) sequence, if a command is interactive, so we know where the cursor is (currently hidden by default)

@CamKem CamKem marked this pull request as ready for review March 1, 2025 06:01
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.

Add tinker tab
1 participant