Repository: qutebrowser/qutebrowser
Pull request: Allow reloading config on SIGHUP
Before this commit qutebrowser would just exit when receiving a SIGHUP. Now it will reload the config using the bare config_source function, just like the :config-source command does.
Closes #8108.


Problem statement:
Allow reloading config with HUP signal
## Description

When sending the HUP signal to Qutebrowser, it should reload the config file(s) just like when running `:config-source`.

## Use cases

- Automatically reloading qutebrowser when changing a Home-manager configuration on Nix systems
- Automatically reloading qutebrowser after changing its settings from a script, to change font size and zoom when connecting a new display for example.

## Current alternatives

If you are the user running Qb, run `qutebrowser :config-source` which will run the command (side-effect: it will also focus the Qutebrowser window like when opening a web page).

Hints from issue discussion:
I think that sounds harmless enough. Regarding the caveat to the "current alternative" about the window being raised, I wanted to link the issue about that, but all I could find was this one (which was apparently fixed at one point!): https://github.com/qutebrowser/qutebrowser/issues/1328

Commit message: Allow reloading config on SIGHUP

Before this commit qutebrowser would just exit when receiving a SIGHUP.
Now it will reload the config using the bare config_source function, just like
the :config-source command does.
Closes #8108.

Patch to review:
diff --git a/qutebrowser/misc/crashsignal.py b/qutebrowser/misc/crashsignal.py
index c69dcbe2909..79dbe6bcc66 100644
--- a/qutebrowser/misc/crashsignal.py
+++ b/qutebrowser/misc/crashsignal.py
@@ -335,6 +335,8 @@ def activate(self):
             signal.SIGINT, self.interrupt)
         self._orig_handlers[signal.SIGTERM] = signal.signal(
             signal.SIGTERM, self.interrupt)
+        self._orig_handlers[signal.SIGHUP] = signal.signal(
+            signal.SIGHUP, self.reload_config)
 
         if utils.is_posix and hasattr(signal, 'set_wakeup_fd'):
             # pylint: disable=import-error,no-member,useless-suppression
@@ -430,6 +432,15 @@ def interrupt_really_forcefully(self, signum, _frame):
         print("WHY ARE YOU DOING THIS TO ME? :(")
         sys.exit(128 + signum)
 
+    def reload_config(self, _signum, _frame):
+        """Reload the config."""
+        log.signals.info("SIGHUP received, reloading config.")
+        config_commands = objreg.get('config-commands', from_command=True)
+        try:
+            config_commands.config_source()
+        except cmdutils.CommandError as e:
+            log.signals.error("Error while reloading config:", exc_info=e)
+
 
 def init(q_app: QApplication,
          args: argparse.Namespace,


Code review comments:
diff_hunk: @@ -430,6 +432,15 @@ def interrupt_really_forcefully(self, signum, _frame):
         print("WHY ARE YOU DOING THIS TO ME? :(")
         sys.exit(128 + signum)
 
+    def reload_config(self, _signum, _frame):
+        """Reload the config."""
+        log.signals.info("SIGHUP received, reloading config.")
+        config_commands = objreg.get('config-commands', from_command=True)
+        try:
+            config_commands.config_source()
+        except cmdutils.CommandError as e:
path: qutebrowser/misc/crashsignal.py
text: @user1:
I feel like we should do a `message.error()` in this except block too so that the user is aware. Or do you think these signals can sometimes come from sources outside a user's control?

diff_hunk: @@ -430,6 +432,15 @@ def interrupt_really_forcefully(self, signum, _frame):
         print("WHY ARE YOU DOING THIS TO ME? :(")
         sys.exit(128 + signum)
 
+    def reload_config(self, _signum, _frame):
+        """Reload the config."""
+        log.signals.info("SIGHUP received, reloading config.")
+        config_commands = objreg.get('config-commands', from_command=True)
path: qutebrowser/misc/crashsignal.py
text: @user1:
I'm a little suspicious that calling a command via objreg then handling CommandError isn't the right thing to do here. I think the whole cmdutils layer was originally specifically meant as an interface for the UI and calling into it from other pieces of code feels a bit odd. (Although it's also pretty powerful so I suspect we'll review it for how extensions should call into it in the future.)

Anyhow, would you mind checking if duplicating some of the internals of `config_source()` is any worse? Eg `configfiles.read_config_py(standarddir.config_py())` and catch `ConfigFileErrors`?