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

feat(cli): added Ctrl+Z (SIGTSTP) support #2836

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Andrew15-5
Copy link
Contributor

Fixes one entry from #2788 (comment) by adding support for Ctrl+Z. I did face a few problems. But overall it works.

It adds (signal) to the zsh's suspend job log message, which isn't there when SIGTSTP is used in other programs. Another thing is that when it is stopped in the middle of some building process (at least during Wasm shenanigans) it can ruin the build (with a panic message) and the dx serve must be restarted. This should be explored more because suspending shouldn't break anything ideally.

The quirk that is interesting is where the execution is continued. This is nice to know as I used a condition check in the main loop before and didn't understand why no logs from it showed up. The less code, the cleaner it gets!

It looks like there is only 1 new package according to Cargo.lock. Nice.

I added a few notes in different places. Like using private field from screen variable isn't great.

@Andrew15-5
Copy link
Contributor Author

Andrew15-5 commented Aug 16, 2024

I finally remembered to test it from just recipe. Yeah, it doesn't work like that. Or rather, I have to press Ctrl+Z twice. I think, once to suspend dx and once to suspend just. So I'm not sure if it's OK to use this implementation of the feature. It's definitely better than nothing, but it isn't perfect (it's very close though).

I can try another approach, which basically means remove the root cause instead of making a workaround.

@Andrew15-5
Copy link
Contributor Author

@jkelleyrtp, ready for initial review, thoughts, verdict. Still has some debugging stuff.

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.

1 participant