Rework command enqueing to minimize swallowing

[Astrono2]

Is your feature request related to a problem? Please describe.

It is currently not possible to call m_sig.emit(button_press{*}); more than once before m_inputdata is cleared in the next iteration of the eventqueue worker loop. This is a problem, for example, if more than one action need to be triggered in the same method call.

Why does polybar need this feature?

This feature would make the core system underlying polybar considerably more flexible, and would allow for easier and faster implementation of some features.

Describe the solution you'd like

Instead of m_inputdata there could be a queue of input data strings.

Describe alternatives you've considered

One alternative is to not emit many signals one after the other, and instead store the signals and call them after a short delay, but this increases complexity and makes everything more prone to bugs.

Additional context

Results in

[madhavpcm]

Hello I am new to polybar, I would like to contribute to this issue, is there any place where I could start?

[patrick96]

@madhavpcm Yes, the pending action string is stored in inputdata in notifications_t in controller.hpp. Making that a queue would probably be the most straightforward way to solve this.

Thanks :)

[madhavpcm]

I have did some changes to notifier_handler() and trigger_action() as these were the places where I noticed enqueuing and dequeuing takes place. I believe the notifier_handler() loop runs separately and checks if inputdata is available and call process_inputdata(). However, in order to test I'm confused not sure how to try out m_sig.emit(button_press{*}) multiple times

   notifer_handler()
  if (!data.inputdata.empty()) {
    process_inputdata(std::move(data.inputdata.front()));
  }

void controller::trigger_action(string&& input_data) {
  std::unique_lock<std::mutex> guard(m_notification_mutex);
  m_notifications.inputdata.push(std::move(input_data));
  m_log.trace("controller: Queueing input event (pending data)");
  trigger_notification();
}

[patrick96]

Yeah testing might be tricky here since it relies on the button clicks (or ipc actions) arriving very quickly after each other. I don't think there is a way to trigger this by hand.

I also don't think you can write any automated tests for this since the controller relies on an X server running which we can't handle yet in our testing infrastructure.

But if you want to just quickly verify things work for you, you could add something like this:

m_loop->timer_handle([this]() { trigger_action("notify-send A"); trigger_action("notify-send B"); })->start(500, 0);

This will trigger two actions directly after one another (they should each create a notification popup) after about half a second. Add this line directly before we set m_loop_ready to true.

[madhavpcm]

I hope this is the desired behaviour

[patrick96]

Yes, that looks alright :)

[madhavpcm]

Im terribly sorry for the messy commits in the PR, I keep forgetting one thing or the other before committing. Kind of a beginner with git and pull requests. Thanks alot for the help!

[patrick96]

Don't worry about that one bit. That's why we have PR reviews and formatting tools ;)

I appreciate the effort

[madhavpcm]

Thanks Alot! I will work on my approach, looking forward to contribute more in the near future :).