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

Tracking clicks on 'persist' blocklets #228

Closed
Mange opened this issue Mar 25, 2017 · 9 comments
Closed

Tracking clicks on 'persist' blocklets #228

Mange opened this issue Mar 25, 2017 · 9 comments

Comments

@Mange
Copy link

Mange commented Mar 25, 2017

persist blocklets cannot detect clicks currently, which limits their use.

I suggest a click on a persist blocklet should restart it with the environment variable set normally. This would allow scripts to detect the changes on startup. Existing blocks that does not deal with this would gain the ability to be restarted without having to restart the whole i3blocks process.

My use for this is that I've written a program that detects the current Pulse Audio sink and reads its volume, then it subscribes using pactl subscribe so it can refresh itself immediately when volume changes. I'd like a click on it to show the name of the sink, sleep a second and then go back to normal operation.

// Fake code
main() {
  if scrolled_down { decrease_volume; }
  if scrolled_up { increase_volume; }
  if left_button_clicked { display_sink_name; sleep 1; }

  display_sink_volume;
  on_each_event { display_sink_volume; }
}

What do you think about this?

@Mange
Copy link
Author

Mange commented Mar 25, 2017

An alternative would be to send events on stdin. That might be more useful for heavier scripts that has long startup time (which is probably more common with persist blocks).

@un-def
Copy link

un-def commented Mar 18, 2018

Restarting a block process on each click is inappropriate in most cases. stdin is one of the possible solutions. How about using signals (e.g., SIGUSR1, SIGUSR1, and SIGHUP) for this purpose?

@Mange
Copy link
Author

Mange commented Mar 18, 2018

Just signals limit the number of events and also cannot attach any metadata to events (like x,y coordinates of click).

STDIN is more extensible that way, but also a bigger change.

(STDIN is superior to the OP IMO.)

@pvsr
Copy link

pvsr commented Jul 20, 2018

I opened #306 to implement this. I'm not hopeful it'll be merged, but feel free to try it out and give feedback.

@vivien
Copy link
Owner

vivien commented Sep 10, 2018

This is a consequent update for i3blocks, so I'll have to think carefully about this. Thanks @pvsr for giving it a shot.

Signals are a clean way to trigger changes to a process in execution, but are indeed limited and more importantly, may cause the signaled program to terminate (depending on the signal for sure) if it isn't handled correctly, which is very likely.

Using standard input is also a clean solution for this kind of long running processes. It requires to define a convention though (e.g. JSON or inline positioned arguments) but at least it has the convenience that a program can blindly ignore it if it wants to (e.g. ignoring clicks.)

I'll make a few tests and above all double check this cannot break existing behavior.

vivien added a commit that referenced this issue Sep 17, 2018
Similarly to stdout and stderr, open a pipe between the parent process
and a child process (for persistent blocks only) so that data (clicks)
can be send to them in a future change.

At the same time, error out on close failure for stdout and stderr.

Refs #228
@vivien vivien closed this as completed in 6e8b51d Sep 17, 2018
@vivien
Copy link
Owner

vivien commented Sep 17, 2018

Clicks on persistent blocks in now implemented.

Note that this is a work in progress and may be subject to change

Simple blocks will only get the click button value sent on a single line. Here's an example:

[persist-simple]
full_text=click me!
command=while read button; do echo "read click $button"; done
interval=persist

Blocks using the JSON format will get all variables sent as a single inline object. Here's an example:

[persist-json]
full_text=click me!
command=ruby -r json -n -e '$_ = JSON.parse($_)' -e '$_["full_text"] = "Click %s at %s:%s" % $_.slice("button", "x", "y").values' -e 'puts JSON.dump($_)' -e 'STDOUT.flush'
interval=persist
format=json

I'll be waiting for feedback for a bit and see if the master branch doesn't break any user scripts.
Do the formats suit you guys? Thank you!

@pvsr
Copy link

pvsr commented Sep 18, 2018

I've got my scripts working with the new format and I like it. On top of fixing my biggest complaint with i3blocks it adds a lot of new flexibility to boot! Allowing any properties of the block to be changed is a neat idea and I think people will be able to do some interesting things by changing the command on the fly. For instance, I came up with this silly little pseudo-quine while experimenting:

[alternate]
command=while true; do echo '{"full_text": "hello world", "color":"#00ff00"}' && read original && echo '{"command":"read second && echo $original", "color":"#ff0000", "full_text":"goodbye world"}' && read; done
interval=persist
format=json

@vivien
Copy link
Owner

vivien commented Sep 18, 2018

Hi @pvsr, your block was driving me nuts! ha ha. I didn't understand at first how this was even possible, since the command cannot be updated at runtime. But it turns out that your block simply does this in fact:

[alternate]
command=while true; do echo '{"full_text": "hello world", "color":"#00ff00"}'; read; echo '{"color":"#ff0000", "full_text":"goodbye world"}'; read; done
interval=persist
format=json

Which makes more sense. Indeed passing clicks to a block via their standard input is intuitive and allows simple things like your "toggle" example above.

However regarding your comment, the values of i3blocks-specific keys (i.e. command, interval, signal and format) are read on startup and updating them afterwards won't have any effect. I though about supporting the update of e.g. the command property, because well it is technically feasible and thus interesting, but this would just bring headaches and a lot of inconsistencies. For example, if that is supported, interval must be too, but persistent blocks are not instantiated the same way as normal blocks, the timer must be changed as well, etc. Also imagine changing the format property on the fly! That'd be weird. So I prefer to KISS and setup a block from its config values, not runtime values.

I also wondered if I must pass all the properties to the process or filter them (passing $command does not really make sense), but people shew some legit interest in having access to the $format and $interval variables, so I just pass them all at the moment.

Same goes for the i3bar-specific keys used to identify a block (i.e. name and instance sent on click). I'm currently ignoring the update of these properties because this could break click detection. (But I might support it though and simply warn instead.)

See these "special" properties as normal variables during runtime, their values are only relevant to i3blocks at startup time, when reading the user configuration file.

I hope it makes sense.

@pvsr
Copy link

pvsr commented Sep 18, 2018

Ah, I saw that name and instance were ignored in block_update_json and I thought that those were the only ones. I guess I wasn't quite as clever as I thought, ha ha. That's one of the fun things about using code before it's properly documented. Your explanation as to why those properties can't be changed makes perfect sense.

The approach of sending everything, including command and format, with the click and then expecting the response to be in the same JSON format instead of plain text did confuse me at first, but once I wrapped my head around your ruby example and reading your rationale now, I definitely do appreciate the simplicity of sending and receiving the same format.

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

No branches or pull requests

4 participants