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

Add support for hot-reloading config #33

Open
MuhammedZakir opened this issue Jul 16, 2021 · 7 comments
Open

Add support for hot-reloading config #33

MuhammedZakir opened this issue Jul 16, 2021 · 7 comments
Labels
enhancement New feature or request

Comments

@MuhammedZakir
Copy link

Perhaps through USR1 signal? Or, by watching file using inotify? Latter could be enabled using a cli flag. For inotify, libfluffy[1] seems good enough?

[1] https://github.com/tinkershack/fluffy

@MuhammedZakir MuhammedZakir changed the title Add support for reloading config Add support for hot-reloading config Jul 16, 2021
@florentc florentc added the enhancement New feature or request label Jul 17, 2021
@florentc
Copy link
Owner

Hello,

Thank you for the suggestion. It is a nice feature indeed but I think it may be a bit overkill (the second method based on listeners using inotify/libfluffy) for xob: it makes sense for software which is cumbersome to restart, where it implies losing some work, etc. or when you change the configuration a lot.

The work and changes required to implement this do not seem worth it and I doubt most users actually care about such a feature. I may be completely wrong however and I am leaving this open just in case.

The USR1 signal method would require that you send the signal with kill and it is no more different than having to do another action: restart xob. However I see the use case when you launched it from your window manager/desktop environment, already linked to an input feed, and you don't want to restart that entirely manually or logoff just for this. I am not against merging the SIGUSR-based feature as long as it remains light on the codebase. It seems like a reasonable compromise.

Along with #32, I am keeping this issue as a reminder to make clearer in future versions of the documentation how to tweak your configuration using another instance of xob in a terminal that you start and stop between each change in the config.

@MuhammedZakir
Copy link
Author

MuhammedZakir commented Jul 17, 2021

[...] However I see the use case when you launched it from your window manager/desktop environment, already linked to an input feed, and you don't want to restart that entirely manually or logoff just for this.

This is what I had in mind when suggesting this feature. At first, I thought signal method was sufficient because I always thought these kind of programs are usually used by users who're willing to get their hands dirty. But, as you already know, that isn't the case.

The problem with signal method is that you'll have to first find the pid of the xob instance and then use kill to send a signal. Compare this with inotify method, where you just need to use a cli flag, say -r, and xob will reload the config when it changes. I felt latter to be more "user-friendly".

@MuhammedZakir
Copy link
Author

MuhammedZakir commented Jul 17, 2021

Maybe we don't need all these? I just read man page and found this sentence interesting:

[...] The program ends when it reads "end" or "quit" (or actually anything else than a number).

Maybe we can use this for sending "commands" to running xob? E.g.

echo :reload >>/path/to/pipe

This could also be (ab)used for things like changing style of an already running xob:

echo :style dark >>/path/to/pipe

@florentc
Copy link
Owner

I thought about that at some point when designing the configuration module (to the point of using that instead of the current alternate state feature). In the end I think the standard input should remain only for the actual input value and not also meta instructions.

The named-pipe method is a fallback method. The ideal way to use xob is launching some_command | xob at startup and forgetting xob even exists. Now that the example pulseaudio script has been corrected and made more user friendly, I hope this is how most users will use xob. In comparison the named-pipe method is cumbersome and misses events that have not been initiated by pressing the keys the update command is bound to.

I don't think there are actual use cases where you need to change the style of the bar while running. Either you want to display some alternate state and you use the builtin alternate state feature with ! after a value, or you want to tweak the appareance/position and you edit the config file then send USR1 to get some feedback.

Using killall there is no need to find the pid of xob first and it aplies to all xob instances. E.g. killall -USR1 xob

Therefore I think the best compromise is the SIGUSR method.

@florentc
Copy link
Owner

I was thinking of another use case of hot-reloading: change in output layout on multihead setups.

@MuhammedZakir
Copy link
Author

I don't think there are actual use cases where you need to change the style of the bar while running. Either you want to display some alternate state and you use the builtin alternate state feature with ! after a value, or you want to tweak the appareance/position and you edit the config file then send USR1 to get some feedback.

You're right. I forgot about !.

Using killall there is no need to find the pid of xob first and it aplies to all xob instances. E.g. killall -USR1 xob

Are startup scripts run as current user or root? If it's latter, it should be noted somewhere.

Therefore I think the best compromise is the SIGUSR method.

👍

@florentc
Copy link
Owner

They are (and should be) run as current user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants