~tsdh/public-inbox

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
4 3

[PATCH swayr v2] swayrbar: add cmd module

Details
Message ID
<6n2wbrs3bcbk4rddionvih5u4hjbst6zd5awokj47vbhwxnw33@dilgfmdghjnn>
DKIM signature
pass
Download raw message
Patch: +131 -10
Add `cmd` module which can be used to run shell commands and display
their output.

Changes the behaviour of on_click configuration options to pass them
through `sh -c`.

Signed-off-by: Luca Matei Pintilie <luca@lucamatei.com>
---
 Thanks for the feedback on my previous patch! The tip on `sh -c` in
 particular was great, and really tied up everything nicely.

 I've decided against adding any parameters to the module at the moment.
 The `{pid}` argument can be taken with a "simple" shell command, such
 as

 swaymsg --type get_tree --raw | jq '.. | (.nodes? // empty)[] | select(.pid and .focused) | .pid'

 I think it also makes the html_escape config option useless for this
 module as well, but I think that's just fine.

 I also added some lines in the README.md about this new module that I
 had forgotten to do last time.

 I also changed the behaviour of on_click to pass the arguments to
 `sh -c`, to match the behaviour of this new `cmd` module. In particular
 for my needs it allows me to use $HOME in config options instead of the
 full path to scripts. If this is too much then it can be removed, as it
 is a change to existing code.

 Also also I took the liberty of fixing some comments in some unrelated
 modules. I hope that wasn't too much trouble.

 Thanks again for your time!
 
 README.md                      |  20 ++++--
 swayrbar/src/bar.rs            |   6 +-
 swayrbar/src/module.rs         |   1 +
 swayrbar/src/module/battery.rs |   2 +-
 swayrbar/src/module/cmd.rs     | 110 +++++++++++++++++++++++++++++++++
 swayrbar/src/module/sysinfo.rs |   2 +-
 6 files changed, 131 insertions(+), 10 deletions(-)
 create mode 100644 swayrbar/src/module/cmd.rs

diff --git a/README.md b/README.md
index 4589eb5..429b9e3 100644
--- a/README.md
+++ b/README.md
@@ -814,12 +814,12 @@ tables](https://toml.io/en/v1.0.0#array-of-tables) in TOML where a module's
  markup](https://docs.gtk.org/Pango/pango_markup.html).  Obviously, if you
  make use of this feature, you want to set `html_escape = true` for that
  module.  This option is optional and may be omitted.
* `on_click` is a table defining actions to be performed when you click on a
  module's space in `swaybar`.  All placeholders available in `format` are
  available here, too.  The action for each mouse button is specified as an
  array `['command', 'arg1', 'arg2',...]`.  The available button names to be
  assigned to are `Left`, `Middle`, `Right`, `WheelUp`, `WheelDown`,
  `WheelLeft`, and `WheelRight`.
* `on_click` is a table defining shell commands to be performed when you
  click on a module's space in `swaybar`.  All placeholders available in
  `format` are available here, too.  The action for each mouse button is
  specified as an array `['command', 'arg1', 'arg2',...]`.  The
  available button names to be assigned to are `Left`, `Middle`,
  `Right`, `WheelUp`, `WheelDown`, `WheelLeft`, and `WheelRight`.

The `on_click` table can also be written as inline table

@@ -911,6 +911,14 @@ The `date` module shows the date and time by defining the `format` using
[chrono's strftime
format](https://docs.rs/chrono/0.4.19/chrono/format/strftime/index.html#specifiers).

#### The `cmd` module

The `cmd` module can be used to run shell commands and display their
output. The command can be passed through the `format` configuration
option, which will be executed by `sh -c`.

This module has no placeholders or default configuration.

### <a id="swayr-version-changes">Version changes</a>

Version changes are summarized in the [NEWS](swayrbar/NEWS.md) file.  If
diff --git a/swayrbar/src/bar.rs b/swayrbar/src/bar.rs
index 7f2c0c5..b2eabca 100644
--- a/swayrbar/src/bar.rs
+++ b/swayrbar/src/bar.rs
@@ -99,6 +99,7 @@ fn create_modules(config: config::Config) -> Vec<Box<dyn BarModuleFn>> {
            "pactl" => module::pactl::create(mc),
            "nmcli" => module::wifi::create(module::wifi::WifiTool::Nmcli, mc),
            "iwctl" => module::wifi::create(module::wifi::WifiTool::Iwctl, mc),
            "cmd" => module::cmd::create(mc),
            unknown => {
                log::warn!("Unknown module name '{unknown}'.  Ignoring...");
                continue;
@@ -202,8 +203,9 @@ fn handle_click(

fn execute_command(cmd: &[String]) {
    log::debug!("Executing command: {cmd:?}");
    let child = p::Command::new(&cmd[0])
        .args(&cmd[1..])
    let child = p::Command::new("sh")
        .arg("-c")
        .arg(&cmd.join(" "))
        // We must not write to stdout because swaybar interprets that!
        // Redirect command output to /dev/null.
        .stdout(Stdio::null())
diff --git a/swayrbar/src/module.rs b/swayrbar/src/module.rs
index d050658..fe2d39f 100644
--- a/swayrbar/src/module.rs
+++ b/swayrbar/src/module.rs
@@ -20,6 +20,7 @@ use swaybar_types as s;
use swayipc as si;

pub mod battery;
pub mod cmd;
pub mod date;
pub mod pactl;
pub mod sysinfo;
diff --git a/swayrbar/src/module/battery.rs b/swayrbar/src/module/battery.rs
index 47be8ba..a10a344 100644
--- a/swayrbar/src/module/battery.rs
+++ b/swayrbar/src/module/battery.rs
@@ -13,7 +13,7 @@
// You should have received a copy of the GNU General Public License along with
// this program.  If not, see <https://www.gnu.org/licenses/>.

//! The date `swayrbar` module.
//! The battery `swayrbar` module.

use crate::config;
use crate::module::{BarModuleFn, RefreshReason};
diff --git a/swayrbar/src/module/cmd.rs b/swayrbar/src/module/cmd.rs
new file mode 100644
index 0000000..1375f86
--- /dev/null
+++ b/swayrbar/src/module/cmd.rs
@@ -0,0 +1,110 @@
// Copyright (C) 2024 Luca Matei Pintilie <luca@lucamatei.com>
//
// This program is free software: you can redistribute it and/or modify it
// under the terms of the GNU General Public License as published by the Free
// Software Foundation, either version 3 of the License, or (at your option)
// any later version.
//
// This program is distributed in the hope that it will be useful, but WITHOUT
// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
// FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
// more details.
//
// You should have received a copy of the GNU General Public License along with
// this program.  If not, see <https://www.gnu.org/licenses/>.

//! The cmd `swayrbar` module.

use crate::config;
use crate::module::{BarModuleFn, RefreshReason};
use std::process::Command;
use std::string::String;
use std::sync::Mutex;
use swaybar_types as s;

const NAME: &str = "cmd";

struct State {
    cached_text: String,
}

pub struct BarModuleCmd {
    config: config::ModuleConfig,
    state: Mutex<State>,
}

fn refresh_state(program: &str) -> String {
    match Command::new("sh").arg("-c").arg(program).output() {
        Ok(output) => String::from_utf8_lossy(&output.stdout).to_string(),
        Err(err) => {
            log::error!("Could not run command: {err}");
            String::new()
        }
    }
}

pub fn create(config: config::ModuleConfig) -> Box<dyn BarModuleFn> {
    Box::new(BarModuleCmd {
        config,
        state: Mutex::new(State {
            cached_text: String::new(),
        }),
    })
}

impl BarModuleFn for BarModuleCmd {
    fn default_config(instance: String) -> config::ModuleConfig
    where
        Self: Sized,
    {
        config::ModuleConfig {
            name: NAME.to_owned(),
            instance,
            format: String::new(),
            html_escape: Some(true),
            on_click: None,
        }
    }

    fn get_config(&self) -> &config::ModuleConfig {
        &self.config
    }

    fn build(&self, reason: &RefreshReason) -> s::Block {
        let mut state = self.state.lock().expect("Could not lock state.");

        if match reason {
            RefreshReason::TimerEvent => true,
            RefreshReason::ClickEvent { name, instance } => {
                name == &self.config.name && instance == &self.config.instance
            }
            _ => false,
        } {
            state.cached_text = refresh_state(&self.config.format);
        }

        s::Block {
            name: Some(NAME.to_owned()),
            instance: Some(self.config.instance.clone()),
            full_text: state.cached_text.to_owned(),
            align: Some(s::Align::Left),
            markup: Some(s::Markup::Pango),
            short_text: None,
            color: None,
            background: None,
            border: None,
            border_top: None,
            border_bottom: None,
            border_left: None,
            border_right: None,
            min_width: None,
            urgent: None,
            separator: Some(true),
            separator_block_width: None,
        }
    }

    fn subst_cmd_args<'a>(&'a self, cmd: &'a [String]) -> Vec<String> {
        cmd.to_vec()
    }
}
diff --git a/swayrbar/src/module/sysinfo.rs b/swayrbar/src/module/sysinfo.rs
index a2b534b..35f4490 100644
--- a/swayrbar/src/module/sysinfo.rs
+++ b/swayrbar/src/module/sysinfo.rs
@@ -13,7 +13,7 @@
// You should have received a copy of the GNU General Public License along with
// this program.  If not, see <https://www.gnu.org/licenses/>.

//! The date `swayrbar` module.
//! The sysinfo `swayrbar` module.

use crate::config;
use crate::module::{BarModuleFn, RefreshReason};
-- 
2.45.2

[swayr/patches/arch.yml] build success

builds.sr.ht <builds@sr.ht>
Details
Message ID
<D2NIR6A5M65J.16QIY4FPYOT30@fra01>
In-Reply-To
<6n2wbrs3bcbk4rddionvih5u4hjbst6zd5awokj47vbhwxnw33@dilgfmdghjnn> (view parent)
DKIM signature
missing
Download raw message
swayr/patches/arch.yml: SUCCESS in 1m17s

[swayrbar: add cmd module][0] v2 from [Luca Matei Pintilie][1]

[0]: https://lists.sr.ht/~tsdh/public-inbox/patches/53824
[1]: luca@lucamatei.com

✓ #1274375 SUCCESS swayr/patches/arch.yml https://builds.sr.ht/~tsdh/job/1274375
Details
Message ID
<87zfqk8l4t.fsf@gnu.org>
In-Reply-To
<6n2wbrs3bcbk4rddionvih5u4hjbst6zd5awokj47vbhwxnw33@dilgfmdghjnn> (view parent)
DKIM signature
pass
Download raw message
Hi Luca,

>  Thanks for the feedback on my previous patch! The tip on `sh -c` in
>  particular was great, and really tied up everything nicely.

Applied and pushed.

>  I've decided against adding any parameters to the module at the
>  moment.  The `{pid}` argument can be taken with a "simple" shell
>  command, such as
>
>  swaymsg --type get_tree --raw | jq '.. | (.nodes? // empty)[] | select(.pid and .focused) | .pid'

No problem, that can be added on demand when needed.

>  I think it also makes the html_escape config option useless for this
>  module as well, but I think that's just fine.

Yup.

>  I also added some lines in the README.md about this new module that I
>  had forgotten to do last time.

Could you please improve the docs a bit?  It should at least say that
for this module, the `format` value is the shell command whose output is
shown in the bar.  And that environment variables are accessible (in
case they are defined in a way that `sh -c` sees them).

>  I also changed the behaviour of on_click to pass the arguments to `sh
>  -c`, to match the behaviour of this new `cmd` module. In particular
>  for my needs it allows me to use $HOME in config options instead of
>  the full path to scripts. If this is too much then it can be removed,
>  as it is a change to existing code.

I've added a change in execute_command() so that it uses args() again
instead of join(" ")-ing cmd into a single arg again.  Shouldn't make a
difference.

>  Also also I took the liberty of fixing some comments in some
>  unrelated modules. I hope that wasn't too much trouble.

Fixing my copy&pasta errors is not trouble. :-)

Thanks!
  Tassilo
Details
Message ID
<xvs6pcowwn5qc42fqdrskipzlccyzoem5nwatywy3uibin6rpg@6wc7fmt3lsjk>
In-Reply-To
<87zfqk8l4t.fsf@gnu.org> (view parent)
DKIM signature
pass
Download raw message
On Sun, Jul 14, 2024 at 09:53:38AM GMT, Tassilo Horn wrote:
>Hi Luca,
>
>>  Thanks for the feedback on my previous patch! The tip on `sh -c` in
>>  particular was great, and really tied up everything nicely.
>
>Applied and pushed.

\o/

>>  I also added some lines in the README.md about this new module that I
>>  had forgotten to do last time.
>
>Could you please improve the docs a bit?  It should at least say that
>for this module, the `format` value is the shell command whose output is
>shown in the bar.  And that environment variables are accessible (in
>case they are defined in a way that `sh -c` sees them).

I'll get to it, thanks for the feedback!

>>  I also changed the behaviour of on_click to pass the arguments to `sh
>>  -c`, to match the behaviour of this new `cmd` module. In particular
>>  for my needs it allows me to use $HOME in config options instead of
>>  the full path to scripts. If this is too much then it can be removed,
>>  as it is a change to existing code.
>
>I've added a change in execute_command() so that it uses args() again
>instead of join(" ")-ing cmd into a single arg again.  Shouldn't make a
>difference.

Hi, I'm afraid this will break existing configurations. For example
volume down doesn't work anymore

>[2024-07-14T08:13:10Z DEBUG swayrbar::bar] Executing command: ["pactl", "set-sink-volume", "@DEFAULT_SINK@", "-1%"]
>[2024-07-14T08:13:10Z DEBUG swayrbar::bar] Sending refresh event ClickEvent { name: "pactl", instance: "0" }
>No valid command specified.

I didn't test this before pushing, but I assume what happens is that the
command sent now is 

>sh -c "pactl" "set-sink-volume" "@DEFAULT_SINK@", "-1%"

And `pactl` outputs the error message, as it never gets the other
parameters.

$ pactl 
No valid command specified.

--
Luca
Details
Message ID
<87r0bwwf3k.fsf@gnu.org>
In-Reply-To
<xvs6pcowwn5qc42fqdrskipzlccyzoem5nwatywy3uibin6rpg@6wc7fmt3lsjk> (view parent)
DKIM signature
pass
Download raw message
Luca Matei Pintilie <luca@lucamatei.com> writes:

>>I've added a change in execute_command() so that it uses args() again
>>instead of join(" ")-ing cmd into a single arg again.  Shouldn't make
>>a difference.
>
> Hi, I'm afraid this will break existing configurations. For example
> volume down doesn't work anymore

Oh, too bad.  Reverted!

Hm, in that case, it would be nice if the on_click actions were
specified as just a string rather than a vector of strings, i.e.,
instead of

[modules.on_click]
Left = ['swayr', 'switch-to-urgent-or-lru-window']
Right = ['kill', '{pid}']

it were just

[modules.on_click]
Left = 'swayr switch-to-urgent-or-lru-window'
Right = 'kill {pid}'

Maybe they could be

  on_click: Option<HashMap<String, Result<String,Vec<String>>>>

instead of

  on_click: Option<HashMap<String, Vec<String>>>

and we could issue a warning (using swaynag) that the vector form is
deprecated...

Bye,
  Tassilo
Reply to thread Export thread (mbox)