Repository: zulip/zulip-terminal

Issue:
Add popup to confirm exiting the application
I raised a concern over this some time ago in [#zulip-terminal>Confirm before exiting? #T1341](https://chat.zulip.org/#narrow/stream/206-zulip-terminal/topic/Confirm.20before.20exiting.3F.20.23T1341); see there for motivation.

The design for this could be as follows:
- reuse our existing confirmation popup to trigger on the exit keypress, instead of it directly exiting the application
- enable this by default, but add a configuration file option to specify a preference
- perhaps also a command-line option, to both enable and disable it (eg. to support default=confirm, config=no-confirm, command-line=confirm)

The first step of these would be a good first issue, and if that goes well the later points could be included in the first PR or follow-up(s).

The exit keypress (`QUIT`) is listed in the help but doesn't connect to a specific event like most other symbolic keys (eg. `HELP`), since it is actually us setting the `Ctrl c` (`SIGINT`) signal handler in `core.py` which controls what happens here. Intercepting that flow of functions would be the first thing to try.

We may later want to explore a different approach where we block that signal completely in the same way that we block Ctrl+z/s, and then use regular keypress handling instead, though if the urwid main loop blocks then we may lose the ability to shut down as cleanly.

Discussion:
zulipbot: Hello @rsashank, it seems like you have referenced #1341 in your pull request description, but you have not referenced them in your commit message description(s). Referencing an issue in a commit message automatically closes the corresponding issue when the commit is merged, which makes the issue tracker easier to manage.

Please run `git commit --amend` in your command line client to amend your commit message description with `Fixes #1341.`.

An example of a correctly-formatted commit:

```
commit fabd5e450374c8dde65ec35f02140383940fe146
Author: zulipbot
Date:   Sat Mar 18 13:42:40 2017 -0700

    pull requests: Check PR commits reference when issue is referenced.

    Fixes #51.
```

To learn how to write a great commit message, please refer to our [guide](https://zulip.readthedocs.io/en/latest/contributing/version-control.html#commit-messages).

<!-- fixCommitWarning -->

neiljp: @rsashank Just to note that I've tagged this with 'awaiting update'; you can use @zulipbot to remove the tag - and add the similar 'review' tag or leave a message if it's ready again :+1:


neiljp: @rsashank I just saw this error - I was looking into this recently, it's not related to you :)
rsashank: > @rsashank I just saw this error - I was looking into this recently, it's not related to you :)


I appreciate the reassurance :P.  Anyways, I've made updates to the README. Do you have any additional recommendations for changes to the documentation?

Patch:
diff --git a/README.md b/README.md
index c59e351..5763df6 100644
--- a/README.md
+++ b/README.md
@@ -232,6 +232,9 @@ theme=zt_dark
 ## Autohide: set to 'autohide' to hide the left & right panels except when they're focused
 autohide=no_autohide
 
+## Exit Confirmation: set to 'disabled' to exit directly with no warning popup first
+exit_confirmation=enabled
+
 ## Footlinks: set to 'disabled' to hide footlinks; 'enabled' will show the first 3 per message
 ## For more flexibility, comment-out this value, and un-comment maximum-footlinks below
 footlinks=enabled
diff --git a/zulipterminal/cli/run.py b/zulipterminal/cli/run.py
index 87794e9..a3ccd9d 100755
--- a/zulipterminal/cli/run.py
+++ b/zulipterminal/cli/run.py
@@ -61,6 +61,7 @@ requests_logger.setLevel(logging.DEBUG)
 VALID_BOOLEAN_SETTINGS: Dict[str, Tuple[str, str]] = {
     "autohide": ("autohide", "no_autohide"),
     "notify": ("enabled", "disabled"),
+    "exit_confirmation": ("enabled", "disabled"),
 }
 
 COLOR_DEPTH_ARGS_TO_DEPTHS: Dict[str, int] = {
@@ -78,10 +79,14 @@ DEFAULT_SETTINGS = {
     "footlinks": "enabled",
     "color-depth": "256",
     "maximum-footlinks": "3",
+    "exit_confirmation": "enabled",
 }
 assert DEFAULT_SETTINGS["autohide"] in VALID_BOOLEAN_SETTINGS["autohide"]
 assert DEFAULT_SETTINGS["notify"] in VALID_BOOLEAN_SETTINGS["notify"]
 assert DEFAULT_SETTINGS["color-depth"] in COLOR_DEPTH_ARGS_TO_DEPTHS
+assert (
+    DEFAULT_SETTINGS["exit_confirmation"] in VALID_BOOLEAN_SETTINGS["exit_confirmation"]
+)
 
 
 def in_color(color: str, text: str) -> str:
@@ -537,6 +542,7 @@ def main(options: Optional[List[str]] = None) -> None:
                 incomplete_theme_text += "      (all themes are incomplete)"
             print(in_color("yellow", incomplete_theme_text))
         print_setting("autohide setting", zterm["autohide"])
+        print_setting("exit confirmation setting", zterm["exit_confirmation"])
         if zterm["footlinks"].source == ConfigSource.ZULIPRC:
             print_setting(
                 "maximum footlinks value",
diff --git a/zulipterminal/core.py b/zulipterminal/core.py
index f93d89e..2799210 100644
--- a/zulipterminal/core.py
+++ b/zulipterminal/core.py
@@ -70,12 +70,14 @@ class Controller:
         in_explore_mode: bool,
         autohide: bool,
         notify: bool,
+        exit_confirmation: bool,
     ) -> None:
         self.theme_name = theme_name
         self.theme = theme
         self.color_depth = color_depth
         self.in_explore_mode = in_explore_mode
         self.autohide = autohide
+        self.exit_confirmation = exit_confirmation
         self.notify_enabled = notify
         self.maximum_footlinks = maximum_footlinks
 
@@ -107,7 +109,12 @@ class Controller:
         self._exception_pipe = self.loop.watch_pipe(self._raise_exception)
 
         # Register new ^C handler
-        signal.signal(signal.SIGINT, self.exit_handler)
+        signal.signal(
+            signal.SIGINT,
+            self.prompting_exit_handler
+            if self.exit_confirmation
+            else self.no_prompt_exit_handler,
+        )
 
     def raise_exception_in_main_thread(
         self, exc_info: ExceptionInfo, *, critical: bool
@@ -621,7 +628,10 @@ class Controller:
         self.client.deregister(queue_id, 1.0)
         sys.exit(0)
 
-    def exit_handler(self, signum: int, frame: Any) -> None:
+    def no_prompt_exit_handler(self, signum: int, frame: Any) -> None:
+        self.deregister_client()
+
+    def prompting_exit_handler(self, signum: int, frame: Any) -> None:
         question = urwid.Text(
             ("bold", " Please confirm that you wish to exit Zulip-Terminal "),
             "center",


Test patch:
diff --git a/tests/cli/test_run.py b/tests/cli/test_run.py
index f1fde1c..7b4fe02 100644
--- a/tests/cli/test_run.py
+++ b/tests/cli/test_run.py
@@ -199,6 +199,7 @@ def test_valid_zuliprc_but_no_connection(
         "Loading with:",
         "   theme 'zt_dark' specified from default config.",
         "   autohide setting 'no_autohide' specified from default config.",
+        "   exit confirmation setting 'enabled' specified from default config.",
         "   maximum footlinks value '3' specified from default config.",
         "   color depth setting '256' specified from default config.",
         "   notify setting 'disabled' specified from default config.",
@@ -257,6 +258,7 @@ def test_warning_regarding_incomplete_theme(
         "\x1b[93m   WARNING: Incomplete theme; results may vary!",
         f"      {expected_warning}\x1b[0m",
         "   autohide setting 'no_autohide' specified from default config.",
+        "   exit confirmation setting 'enabled' specified from default config.",
         "   maximum footlinks value '3' specified from default config.",
         "   color depth setting '256' specified from default config.",
         "   notify setting 'disabled' specified from default config.",
@@ -475,6 +477,7 @@ def test_successful_main_function_with_config(
         "Loading with:",
         "   theme 'zt_dark' specified in zuliprc file (by alias 'default').",
         "   autohide setting 'autohide' specified in zuliprc file.",
+        "   exit confirmation setting 'enabled' specified from default config.",
         f"   maximum footlinks value {footlinks_output}",
         "   color depth setting '256' specified in zuliprc file.",
         "   notify setting 'enabled' specified in zuliprc file.",
diff --git a/tests/core/test_core.py b/tests/core/test_core.py
index 9f13a40..033f126 100644
--- a/tests/core/test_core.py
+++ b/tests/core/test_core.py
@@ -48,6 +48,7 @@ class TestController:
         self.in_explore_mode = False
         self.autohide = True  # FIXME Add tests for no-autohide
         self.notify_enabled = False
+        self.exit_confirmation = True
         self.maximum_footlinks = 3
         result = Controller(
             config_file=self.config_file,
@@ -60,6 +61,7 @@ class TestController:
             **dict(
                 autohide=self.autohide,
                 notify=self.notify_enabled,
+                exit_confirmation=self.exit_confirmation,
             ),
         )
         result.view.message_view = mocker.Mock()  # set in View.__init__