Add expanding of home directory symbol (~) in the "command" parameter
of ":pipe" command.
Signed-off-by: Vitaly Ovchinnikov <v@postbox.nz>
---
Good for piping stuff to local aerc-specific helpers that are not in the
paths. Here's a sample binding I need:
o = :pipe -b ~/.config/aerc/helpers/do-stuff<Enter>
Instead I have to put the absolute file name of the helper there. The
patch fixes that.
commands/msg/pipe.go | 3 +++
1 file changed, 3 insertions(+)
diff --git a/commands/msg/pipe.go b/commands/msg/pipe.go
index fc1ac8f..7cb2378 100644
--- a/commands/msg/pipe.go+++ b/commands/msg/pipe.go
@@ -10,6 +10,7 @@ import (
"time"
"git.sr.ht/~rjarry/aerc/commands"
+ "git.sr.ht/~rjarry/aerc/lib/xdg" "git.sr.ht/~rjarry/aerc/log"
"git.sr.ht/~rjarry/aerc/widgets"
mboxer "git.sr.ht/~rjarry/aerc/worker/mbox"
@@ -64,6 +65,8 @@ func (Pipe) Execute(aerc *widgets.Aerc, args []string) error {
return errors.New("Usage: pipe [-mp] <cmd> [args...]")
}
+ cmd[0] = xdg.ExpandHome(cmd[0])+ provider := aerc.SelectedTabContent().(widgets.ProvidesMessage)
if !pipeFull && !pipePart {
if _, ok := provider.(*widgets.MessageViewer); ok {
--
2.39.2 (Apple Git-143)
Vitaly Ovchinnikov, Sep 16, 2023 at 18:47:
> Add expanding of home directory symbol (~) in the "command" parameter
> of ":pipe" command.
>
> Signed-off-by: Vitaly Ovchinnikov <v@postbox.nz>
> ---
> Good for piping stuff to local aerc-specific helpers that are not in the
> paths. Here's a sample binding I need:
>
> o = :pipe -b ~/.config/aerc/helpers/do-stuff<Enter>
>
> Instead I have to put the absolute file name of the helper there. The
> patch fixes that.
This is nice but a bit limited since it only expands for the first
argument.
I would prefer if the command could be a shell command. That way you'd
get ~ expansion anywhere in the command and you could chain pipes with
other commands as well.
:pipe -m foo -C ~/tmp xyz | tr '\n' ' ' | tac
Filter commands already work that way and the man page already mentions
that :pipe takes a *shell* command. The patch should look something like
this:
diff --git a/commands/msg/pipe.go b/commands/msg/pipe.go
index fc1ac8f8693a..20dbfb4fa27e 100644
--- a/commands/msg/pipe.go+++ b/commands/msg/pipe.go
@@ -7,6 +7,7 @@ import (
"os/exec"
"regexp"
"sort"
+ "strings" "time"
"git.sr.ht/~rjarry/aerc/commands"
@@ -61,9 +62,12 @@ func (Pipe) Execute(aerc *widgets.Aerc, args []string) error {
}
cmd := args[optind:]
if len(cmd) == 0 {
- return errors.New("Usage: pipe [-mp] <cmd> [args...]")+ return errors.New("Usage: pipe [-bmp] <cmd> [args...]") }
+ name := cmd[0]+ cmd = []string{"sh", "-c", strings.Join(cmd, " ")}+ provider := aerc.SelectedTabContent().(widgets.ProvidesMessage)
if !pipeFull && !pipePart {
if _, ok := provider.(*widgets.MessageViewer); ok {
@@ -106,11 +110,11 @@ func (Pipe) Execute(aerc *widgets.Aerc, args []string) error {
} else {
if ecmd.ProcessState.ExitCode() != 0 {
aerc.PushError(fmt.Sprintf(
- "%s: completed with status %d", cmd[0],+ "%s: completed with status %d", name, ecmd.ProcessState.ExitCode()))
} else {
aerc.PushStatus(fmt.Sprintf(
- "%s: completed with status %d", cmd[0],+ "%s: completed with status %d", name, ecmd.ProcessState.ExitCode()), 10*time.Second)
}
}
@@ -130,7 +134,7 @@ func (Pipe) Execute(aerc *widgets.Aerc, args []string) error {
} else {
doTerm(reader,
fmt.Sprintf("%s <%s",
- cmd[0], title))+ name, title)) }
})
return nil
@@ -205,7 +209,7 @@ func (Pipe) Execute(aerc *widgets.Aerc, args []string) error {
if background {
doExec(reader)
} else {
- doTerm(reader, fmt.Sprintf("%s <%s", cmd[0], title))+ doTerm(reader, fmt.Sprintf("%s <%s", name, title)) }
}()
} else if pipePart {
@@ -222,7 +226,7 @@ func (Pipe) Execute(aerc *widgets.Aerc, args []string) error {
doExec(reader)
} else {
name := fmt.Sprintf("%s <%s/[%d]",
- cmd[0], p.Msg.Envelope.Subject, p.Index)+ name, p.Msg.Envelope.Subject, p.Index) doTerm(reader, name)
}
})
> The patch should look something like this:
Got it, so what is the plan now? Do you want me to re-submit this patch
instead or you can apply it directly?
Vitaly Ovchinnikov, Sep 19, 2023 at 14:08:
> > The patch should look something like this:>> Got it, so what is the plan now? Do you want me to re-submit this patch> instead or you can apply it directly?
Can you send a v2 inspiring from this? I didn't test it. It was just an
example/idea.
Thanks
> Can you send a v2 inspiring from this? I didn't test it. It was just an> example/idea.
After looking at the code I see that there is a couple of placed where
the similar approach is already used, so before I take any further
steps, I want to discuss one thing.
Imagine that I want to use :open-link to copy one of the links to
clipboard. Something like this:
:open-link <link> echo -n {} | pbcopy
(pbcopy is a mac thing that copies stdin to clipboard)
It doesn't work this way because the command is running without shell,
so let's add one manually:
:open-link <link> sh -c "echo -n {} | pbcopy"
(note that I added quotes around the real command in order for the pipe
to work, this might affect your patch idea, yet I haven't tested it yet)
This way it starts working, but in the clipboard I get "<link>\n" and
"unknown argument -n" goes into stderr. The reason is that "echo" in
"sh" is not aware of "-n" command.
Rewriting the command this way does the job:
:open-link <link> bash -c "echo -n {} | pbcopy"
So my questions are:
1. Should we hard-code sh?
2. Souldn't we take the user's shell from the environment?
2a. This might also bring shell-specific commands and other stuff the
user configured....
3. As :open-link is another place where the shell is not used, maybe we
need a dedicated function for launching stuff this way?
What do you think?
Vitaly Ovchinnikov, Sep 19, 2023 at 14:37:
> After looking at the code I see that there is a couple of placed where> the similar approach is already used, so before I take any further> steps, I want to discuss one thing.>> Imagine that I want to use :open-link to copy one of the links to> clipboard. Something like this:>> :open-link <link> echo -n {} | pbcopy>> (pbcopy is a mac thing that copies stdin to clipboard)>> It doesn't work this way because the command is running without shell,> so let's add one manually:>> :open-link <link> sh -c "echo -n {} | pbcopy">> (note that I added quotes around the real command in order for the pipe> to work, this might affect your patch idea, yet I haven't tested it yet)>> This way it starts working, but in the clipboard I get "<link>\n" and> "unknown argument -n" goes into stderr. The reason is that "echo" in> "sh" is not aware of "-n" command.>> Rewriting the command this way does the job:> :open-link <link> bash -c "echo -n {} | pbcopy"
echo -n is not POSIX compliant. If you want to print something without
a trailing new line, you should use printf which is available on all
standard POSIX shells.
:open-link <link> sh -c "printf '%s' '{}' | pbcopy"
> So my questions are:>> 1. Should we hard-code sh?
This is the standard way of doing things. `sh` is guaranteed to be
present in the exec PATH on POSIX systems.
> 2. Souldn't we take the user's shell from the environment?
No, $SHELL is not always defined and calling getpwnam_r() to determine
the user's shell seems overkill.
> 2a. This might also bring shell-specific commands and other stuff the> user configured....
Yes, I would really prefer if we stuck to `sh -c`.
> 3. As :open-link is another place where the shell is not used, maybe we> need a dedicated function for launching stuff this way?>> What do you think?
I would not mind if the commands invoked by :open and :open-link would
be passed via a shell. That way you could do:
:open-link <link> printf "%s" "{}" | pbcopy
Cheers,
> Can you send a v2 inspiring from this? I didn't test it. It was just an> example/idea.
OK, so I had a bit of trying with this idea, and it sort of works, but
not for everything. Simple things like `:pipe | sort | less` work, but
something more complex doesn't. Here's an example:
:pipe tr "\n" "!" | less
The problem is that the initial command parser transforms this into:
args := []string {
"pipe",
"tr",
"n", // !!
"!", // !
"|",
"less",
}
Note that newline is converted to "n" and the quotes around the tr's
arguments are lost. After joining the command back we're getting:
:pipe tr n ! | less
Which is pretty far away from the original thing. We might need to add
an "--" option to the parser, so it stops processing the line and leaves
everything after it as is, then we could probably get closer to the
expected behavior:
:pipe -- tr "\n" "!" | less
Should become:
args := []string {
'pipe',
'tr "\n" "!" | less',
}
The closest thing we can do right now without changing the parser is this:
:pipe "tr '\n' '!' | less"
This keeps the pipe's argument almost intact, it just replaces "\n" with
"n". This way it works as expected:
:pipe 'tr "\n" "!" | less'
That's pretty much it. I don't think we can simply get away with just
adding "sh -c" at the beginning of the existing command. We will still
need proper manual quoting of the command and if we do quoting anyway we
could easily add "sh -c" manually at the beginning, as well.
The right way would be to either modify the parser, so it stops
splitting the command at some point as I suggested above.
Another option is to provide the original command line to the command
for optional parsing. I am personally not sure if this is a good idea
though.
Comments?
Vitaly Ovchinnikov, Sep 19, 2023 at 18:56:
> OK, so I had a bit of trying with this idea, and it sort of works, but> not for everything. Simple things like `:pipe | sort | less` work, but> something more complex doesn't. Here's an example:>> :pipe tr "\n" "!" | less>> The problem is that the initial command parser transforms this into:>> args := []string {> "pipe",> "tr",> "n", // !!> "!", // !> "|",> "less",> }>> Note that newline is converted to "n" and the quotes around the tr's> arguments are lost. After joining the command back we're getting:>> :pipe tr n ! | less>> Which is pretty far away from the original thing. We might need to add> an "--" option to the parser, so it stops processing the line and leaves> everything after it as is, then we could probably get closer to the> expected behavior:>> :pipe -- tr "\n" "!" | less>> Should become:>> args := []string {> 'pipe',> 'tr "\n" "!" | less',> }>> The closest thing we can do right now without changing the parser is this:>> :pipe "tr '\n' '!' | less">> This keeps the pipe's argument almost intact, it just replaces "\n" with> "n". This way it works as expected:>> :pipe 'tr "\n" "!" | less'>> That's pretty much it. I don't think we can simply get away with just> adding "sh -c" at the beginning of the existing command. We will still> need proper manual quoting of the command and if we do quoting anyway we> could easily add "sh -c" manually at the beginning, as well.>> The right way would be to either modify the parser, so it stops> splitting the command at some point as I suggested above.>> Another option is to provide the original command line to the command> for optional parsing. I am personally not sure if this is a good idea> though.>> Comments?
I have a solution for this but it will require some refactoring. I'll
send a patch later tonight.