dnkl/foot
41
2.0k
Fork
You've already forked foot
243

WIP: show close dialog on toplevel exit #2176

Closed
saeedark wants to merge 2 commits from saeedark/foot:master into master
pull from: saeedark/foot:master
merge into: dnkl:master
dnkl:master
dnkl:osc-5522
dnkl:sixel-heap-buffer-overflow
dnkl:releases/1.27
dnkl:releases/1.26
dnkl:releases/1.25
dnkl:releases/1.24
dnkl:multi-cursor
dnkl:releases/1.23
dnkl:pixman-16f-2
dnkl:releases/1.22
dnkl:releases/1.21
dnkl:releases/1.20
dnkl:releases/1.19
dnkl:releases/1.18
dnkl:releases/1.17
dnkl:releases/1.16
dnkl:releases/1.15
dnkl:releases/1.14
dnkl:releases/1.13
dnkl:releases/1.12
dnkl:releases/1.11
dnkl:releases/1.10
dnkl:releases/1.9
dnkl:releases/1.8
dnkl:releases/1.7
dnkl:releases/1.6
dnkl:releases/1.5
dnkl:releases/1.4
dnkl:releases/1.3
dnkl:releases/1.2
dnkl:releases/1.1
dnkl:releases/1.0
First-time contributor
Copy link

Hello,
Firstly thanks for developing foot terminal.

Well, there are some people out they're so united with their window manager's key binding, that they press close keys accidentally almost too often.
This PR shows a close dialog on top level exit request.

I had no clue what I was doing; so it needs refactoring a lot.
The rendering part is almost identical to the "url-mode". The hardest part is to check if it's ok to shutdown the term or not.
Gnome terminal and Ghostty functions are here and here. One uses the shell integration the other uses thetcgetpgrp() function. I used both.

Thetcgetpgrp() function gives the foreground process, if it's the slave process, then we use shell integration to check if cursor is at the prompt. we also need to check if shell have background process or not. which I have no idea how to do that! One option is to check whether slave has at least a child process or not. Good option in theory but not accurate enough (weird shells with child process) and it's OS dependent. The better option is to ask shells to report their jobs statues. Something like shell integration to detect where prompt is.

And of course it doesn't change the default behavior and should be activated with config file.
Thanks

Hello, Firstly thanks for developing foot terminal. Well, there are some people out they're so united with their window manager's key binding, that they press close keys accidentally almost too often. This PR shows a close dialog on top level exit request. I had no clue what I was doing; so it needs refactoring a lot. The rendering part is almost identical to the "url-mode". The hardest part is to check if it's ok to shutdown the term or not. Gnome terminal and Ghostty functions are [here](https://gitlab.gnome.org/GNOME/gnome-terminal/-/blob/master/src/terminal-screen.cc?ref_type=heads#L2927) and [here](https://github.com/ghostty-org/ghostty/blob/0f0a61c38dbebbd70979afbd3df81bf143efca9d/src/terminal/Terminal.zig#L1032). One uses the shell integration the other uses the`tcgetpgrp()` function. I used both. The`tcgetpgrp()` function gives the foreground process, if it's the slave process, then we use shell integration to check if cursor is at the prompt. we also need to check if shell have background process or not. which I have no idea how to do that! One option is to check whether slave has at least a child process or not. Good option in theory but not accurate enough (weird shells with child process) and it's OS dependent. The better option is to ask shells to report their jobs statues. Something like shell integration to detect where prompt is. And of course it doesn't change the default behavior and should be activated with config file. Thanks
Merge remote-tracking branch 'refs/remotes/origin/master'
Some checks are pending
ci/woodpecker/pr/woodpecker Pipeline is pending approval
builds.sr.ht/freebsd-x64 Job completed
ci/woodpecker/pull_request_closed/woodpecker Pipeline is pending approval
6fd357a46e
Collaborator
Copy link

This was discussed previously in #1473.

This was discussed previously in #1473.
Author
First-time contributor
Copy link

@craigbarnes wrote in #2176 (comment):

This was discussed previously in #1473.

Hmm.
So two problems: 1)GUI! 2)Guessing game!

  1. This PR doesn't use any GUI toolkit. (Yay!)
  2. This is still a problem.

But hear me out:
I completely agree the default behavior of foot is the best at the moment. So in the doc it can be stated that this config option is not flawless.

Tackling the issue, however, I think the best we can do is to have a boolean variable and two specific OSC one indicating that it's Ok to exit and the other not. In this way shells or others processes can speaks with foot. This would be other programs' responsibility.
In this case, we should replace this.

Thank you for considering this.

@craigbarnes wrote in https://codeberg.org/dnkl/foot/pulls/2176#issuecomment-7228177: > This was discussed previously in #1473. Hmm. So two problems: 1)GUI! 2)Guessing game! 1) This PR doesn't use any GUI toolkit. (Yay!) 2) This is still a problem. But hear me out: I completely agree the default behavior of foot is the best at the moment. So in the doc it can be stated that this config option is not flawless. Tackling the issue, however, I think the best we can do is to have a boolean variable and two specific OSC one indicating that it's Ok to exit and the other not. In this way shells or others processes can speaks with foot. This would be other programs' responsibility. In this case, we should replace [this](https://codeberg.org/saeedark/foot/src/commit/6fd357a46e85bff7c0781b51eb2fd422e0e75057/wayland.c#L1001). Thank you for considering this.
Author
First-time contributor
Copy link

As an experiment, something like this new branch can do the trick.

and these functions should be added to fish for example:

function foot_cmd_start --on-event fish_preexec
 if jobs -q
 echo -en "\e]133;N\e\\"
 else
 echo -en "\e]133;O\e\\"
 end
end
function mark_prompt_start --on-event fish_prompt
if jobs -q
 echo -en "\e]133;N\e\\"
 else
 echo -en "\e]133;O\e\\"
 end
end

In the end other slave programs should report their status too.

Edit:
Also we can add config for this.

As an experiment, something [like this new branch](https://codeberg.org/saeedark/foot/commit/91c97c9618283a59462098a23e2b922acf6d4b4f) can do the trick. and these functions should be added to fish for example: ```fish function foot_cmd_start --on-event fish_preexec if jobs -q echo -en "\e]133;N\e\\" else echo -en "\e]133;O\e\\" end end function mark_prompt_start --on-event fish_prompt if jobs -q echo -en "\e]133;N\e\\" else echo -en "\e]133;O\e\\" end end ``` In the end other slave programs should report their status too. Edit: Also we can add config for [this](https://codeberg.org/saeedark/foot/src/branch/newosc/terminal.c#L1401).
Owner
Copy link

Sorry to say that this is a PR I'm not interested in merging. It's large, adding lots of logic and complexity for a feature there hasn't been much interest in. There's also still no way to really determine whether the close-warning should be displayed or not. I'm not sure relying on an escape sequence is a good idea.

Sorry to say that this is a PR I'm not interested in merging. It's large, adding lots of logic and complexity for a feature there hasn't been much interest in. There's also still no way to _really_ determine whether the close-warning should be displayed or not. I'm not sure relying on an escape sequence is a good idea.
dnkl closed this pull request 2025年10月10日 12:03:52 +02:00
Some checks are pending
ci/woodpecker/pr/woodpecker Pipeline is pending approval
builds.sr.ht/freebsd-x64 Job completed
ci/woodpecker/pull_request_closed/woodpecker Pipeline is pending approval

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
dnkl/foot!2176
Reference in a new issue
dnkl/foot
No description provided.
Delete branch "saeedark/foot:master"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?