fix(script): output timing inconsistencies (#2667)

Fixes #2650
This commit is contained in:
Maxim Kolesnikov 2022-04-03 18:11:13 +07:00 committed by GitHub
parent abfc6b7f91
commit 41d41ca8df
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 82 additions and 58 deletions

View File

@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Positioning in awesomeWM ([`#2651`](https://github.com/polybar/polybar/pull/2651))
- `internal/xworkspaces`: The module sometimes crashed polybar when windows were closed. ([`#2655`](https://github.com/polybar/polybar/pull/2655))
- Mouseover error when only one cursor is defined ([`#2656`](https://github.com/polybar/polybar/pull/2656))
- Timing inconsistencies with `custom/script` output ([`#2650`](https://github.com/polybar/polybar/issues/2650), first described at [`#2630`](https://github.com/polybar/polybar/pull/2630))
## [3.6.1] - 2022-03-05
### Build

View File

@ -11,9 +11,18 @@ POLYBAR_NS
class script_runner {
public:
struct data {
int counter{0};
int pid{-1};
int exit_status{0};
string output;
};
using on_update = std::function<void(const data&)>;
using interval = std::chrono::duration<double>;
script_runner(std::function<void(void)> on_update, const string& exec, const string& exec_if, bool tail,
interval interval, const vector<pair<string, string>>& env);
script_runner(on_update on_update, const string& exec, const string& exec_if, bool tail, interval interval,
const vector<pair<string, string>>& env);
bool check_condition() const;
interval process();
@ -22,16 +31,11 @@ class script_runner {
void stop();
int get_pid() const;
int get_counter() const;
int get_exit_status() const;
string get_output();
bool is_stopping() const;
protected:
bool set_output(string&&);
bool set_exit_status(int);
interval run_tail();
interval run();
@ -39,7 +43,7 @@ class script_runner {
private:
const logger& m_log;
const std::function<void(void)> m_on_update;
const on_update m_on_update;
const string m_exec;
const string m_exec_if;
@ -47,13 +51,8 @@ class script_runner {
const interval m_interval;
const vector<pair<string, string>> m_env;
std::mutex m_output_lock;
string m_output;
std::atomic_int m_counter{0};
data m_data;
std::atomic_bool m_stopping{false};
std::atomic_int m_pid{-1};
std::atomic_int m_exit_status{0};
};
POLYBAR_NS_END

View File

@ -26,6 +26,8 @@ namespace modules {
bool check_condition();
private:
void handle_runner_update(const script_runner::data&);
static constexpr auto TAG_LABEL = "<label>";
static constexpr auto TAG_LABEL_FAIL = "<label-fail>";
static constexpr auto FORMAT_FAIL = "format-fail";
@ -39,6 +41,10 @@ namespace modules {
label_t m_label;
label_t m_label_fail;
int m_exit_status{0};
script_runner::data m_data;
std::mutex m_data_mutex;
};
} // namespace modules

View File

@ -11,7 +11,7 @@
POLYBAR_NS
script_runner::script_runner(std::function<void(void)> on_update, const string& exec, const string& exec_if, bool tail,
script_runner::script_runner(on_update on_update, const string& exec, const string& exec_if, bool tail,
interval interval, const vector<pair<string, string>>& env)
: m_log(logger::make())
, m_on_update(on_update)
@ -52,23 +52,6 @@ void script_runner::stop() {
m_stopping = true;
}
int script_runner::get_pid() const {
return m_pid;
}
int script_runner::get_counter() const {
return m_counter;
}
int script_runner::get_exit_status() const {
return m_exit_status;
}
string script_runner::get_output() {
std::lock_guard<std::mutex> guard(m_output_lock);
return m_output;
}
bool script_runner::is_stopping() const {
return m_stopping;
}
@ -79,19 +62,28 @@ bool script_runner::is_stopping() const {
* Returns true if the output changed.
*/
bool script_runner::set_output(string&& new_output) {
std::lock_guard<std::mutex> guard(m_output_lock);
if (m_output != new_output) {
m_output = std::move(new_output);
m_on_update();
if (m_data.output != new_output) {
m_data.output = std::move(new_output);
return true;
}
return false;
}
/**
* Updates the current exit status
*
* Returns true if the exit status changed.
*/
bool script_runner::set_exit_status(int new_status) {
auto changed = (m_data.exit_status != new_status);
m_data.exit_status = new_status;
return changed;
}
script_runner::interval script_runner::run() {
auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_counter));
auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_data.counter));
m_log.info("script_runner: Invoking shell command: \"%s\"", exec);
auto cmd = command_util::make_command<output_policy::REDIRECTED>(exec);
@ -124,13 +116,17 @@ script_runner::interval script_runner::run() {
return 0s;
}
m_exit_status = cmd->wait();
auto exit_status_changed = set_exit_status(cmd->wait());
if (!changed && m_exit_status != 0) {
if (!changed && m_data.exit_status != 0) {
clear_output();
}
if (m_exit_status == 0) {
if (changed || exit_status_changed) {
m_on_update(m_data);
}
if (m_data.exit_status == 0) {
return m_interval;
} else {
return std::max(m_interval, interval{1s});
@ -138,7 +134,7 @@ script_runner::interval script_runner::run() {
}
script_runner::interval script_runner::run_tail() {
auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_counter));
auto exec = string_util::replace_all(m_exec, "%counter%", to_string(++m_data.counter));
m_log.info("script_runner: Invoking shell command: \"%s\"", exec);
auto cmd = command_util::make_command<output_policy::REDIRECTED>(exec);
@ -148,15 +144,23 @@ script_runner::interval script_runner::run_tail() {
throw modules::module_error("Failed to execute command: " + string(err.what()));
}
auto pid_guard = scope_util::make_exit_handler([this]() { m_pid = -1; });
m_pid = cmd->get_pid();
auto pid_guard = scope_util::make_exit_handler([this]() {
m_data.pid = -1;
m_on_update(m_data);
});
m_data.pid = cmd->get_pid();
int fd = cmd->get_stdout(PIPE_READ);
assert(fd != -1);
while (!m_stopping && cmd->is_running() && !io_util::poll(fd, POLLHUP, 0)) {
if (io_util::poll_read(fd, 25)) {
set_output(cmd->readline());
auto changed = set_output(cmd->readline());
if (changed) {
m_on_update(m_data);
}
}
}

View File

@ -10,8 +10,8 @@ namespace modules {
: module<script_module>(bar, move(name_))
, m_tail(m_conf.get(name(), "tail", false))
, m_interval(m_conf.get<script_runner::interval>(name(), "interval", m_tail ? 0s : 5s))
, m_runner([this]() { broadcast(); }, m_conf.get(name(), "exec", ""s), m_conf.get(name(), "exec-if", ""s), m_tail,
m_interval, m_conf.get_with_prefix(name(), "env-")) {
, m_runner([this](const auto& data) { handle_runner_update(data); }, m_conf.get(name(), "exec", ""s),
m_conf.get(name(), "exec-if", ""s), m_tail, m_interval, m_conf.get_with_prefix(name(), "env-")) {
// Load configured click handlers
m_actions[mousebtn::LEFT] = m_conf.get(name(), "click-left", ""s);
m_actions[mousebtn::MIDDLE] = m_conf.get(name(), "click-middle", ""s);
@ -76,29 +76,35 @@ namespace modules {
* Generate module output
*/
string script_module::get_format() const {
if (m_runner.get_exit_status() != 0 && m_conf.has(name(), FORMAT_FAIL)) {
if (m_exit_status != 0 && m_conf.has(name(), FORMAT_FAIL)) {
return FORMAT_FAIL;
}
return DEFAULT_FORMAT;
}
string script_module::get_output() {
auto script_output = m_runner.get_output();
if (script_output.empty()) {
auto data = [this]{
std::lock_guard<std::mutex> lk(m_data_mutex);
return m_data;
}();
m_exit_status = data.exit_status;
if (data.output.empty()) {
return "";
}
if (m_label) {
m_label->reset_tokens();
m_label->replace_token("%output%", script_output);
m_label->replace_token("%output%", data.output);
}
if (m_label_fail) {
m_label_fail->reset_tokens();
m_label_fail->replace_token("%output%", script_output);
m_label_fail->replace_token("%output%", data.output);
}
string cnt{to_string(m_runner.get_counter())};
string cnt{to_string(data.counter)};
string output{module::get_output()};
for (const auto& a : m_actions) {
@ -112,9 +118,8 @@ namespace modules {
* The pid token is only for tailed commands.
* If the command is not specified or running, replacement is unnecessary as well
*/
int pid = m_runner.get_pid();
if (pid != -1) {
action_replaced = string_util::replace_all(action_replaced, "%pid%", to_string(pid));
if (data.pid != -1) {
action_replaced = string_util::replace_all(action_replaced, "%pid%", to_string(data.pid));
}
m_builder->action(btn, action_replaced);
}
@ -139,6 +144,15 @@ namespace modules {
return true;
}
void script_module::handle_runner_update(const script_runner::data& data) {
{
std::lock_guard<std::mutex> lk(m_data_mutex);
m_data = data;
}
broadcast();
}
} // namespace modules
POLYBAR_NS_END