~sircmpwn/aerc

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

[PATCH] on macOS, :open now can use the specified application

JD
Details
Message ID
<C5RBPTZXRXVC.31QILPY07JA15@Penelope.local>
DKIM signature
pass
Download raw message
Patch: +97 -2
Differently from xdg-open, macOS's /usr/bin/open can also use non-default 
applications to open files. This patch adds -A parameter to aerc's :open
command, which allows to open message parts with custom programs. E.g. to 
open a HTML part in a non-default browser, use :open -A Safari. Or even
:open -A MacVim to open it in an editor instead of a browser.

DISCLAIMER: again, I'm not a programmer, and this probably can be done in 
a way better manner (if someone needs this functionality at all :)

---
 commands/msgview/open.go        |  2 +
 commands/msgview/open_darwin.go | 89 +++++++++++++++++++++++++++++++++++++++++
 doc/aerc.1.scd                  |  5 ++-
 lib/open_darwin.go              |  3 +-
 4 files changed, 97 insertions(+), 2 deletions(-)
 create mode 100644 commands/msgview/open_darwin.go

diff --git a/commands/msgview/open.go b/commands/msgview/open.go
index 4aa6133..ec33d60 100644
--- a/commands/msgview/open.go
+++ b/commands/msgview/open.go
@@ -1,3 +1,5 @@
// +build !darwin

package msgview

import (
diff --git a/commands/msgview/open_darwin.go b/commands/msgview/open_darwin.go
new file mode 100644
index 0000000..32909e1
--- /dev/null
+++ b/commands/msgview/open_darwin.go
@@ -0,0 +1,89 @@
package msgview

import (
	"errors"
	"fmt"
	"io"
	"io/ioutil"
	"mime"
	"os"
	"time"

	"git.sr.ht/~sircmpwn/getopt"

	"git.sr.ht/~sircmpwn/aerc/lib"
	"git.sr.ht/~sircmpwn/aerc/widgets"
)

type Open struct{}

func init() {
	register(Open{})
}

func (Open) Aliases() []string {
	return []string{"open"}
}

func (Open) Complete(aerc *widgets.Aerc, args []string) []string {
	return nil
}

func (Open) Execute(aerc *widgets.Aerc, args []string) error {
    opts, optind, err := getopt.Getopts(args, "A:")
	if err != nil {
		return err
	}
	if optind != len(args) {
		return errors.New("Usage: open [-A <application>]")
	}
	var (
		theApp   string
	)
	for _, opt := range opts {
		switch opt.Option {
		case 'A':
			theApp = opt.Value
		}
	}

	mv := aerc.SelectedTab().(*widgets.MessageViewer)
	p := mv.SelectedMessagePart()

	store := mv.Store()
	store.FetchBodyPart(p.Msg.Uid, p.Index, func(reader io.Reader) {
		extension := ""
		// try to determine the correct extension based on mimetype
		if part, err := p.Msg.BodyStructure.PartAtIndex(p.Index); err == nil {
			mimeType := fmt.Sprintf("%s/%s", part.MIMEType, part.MIMESubType)

			if exts, _ := mime.ExtensionsByType(mimeType); exts != nil && len(exts) > 0 {
				extension = exts[0]
			}
		}

		tmpFile, err := ioutil.TempFile(os.TempDir(), "aerc-*"+extension)
		if err != nil {
			aerc.PushError(" " + err.Error())
			return
		}
		defer tmpFile.Close()

		_, err = io.Copy(tmpFile, reader)
		if err != nil {
			aerc.PushError(" " + err.Error())
			return
		}

		if theApp != "" {
			theApp = "-a|" + theApp + "|"
		}
		lib.OpenFile(theApp + tmpFile.Name(), func(err error) {
			aerc.PushError(" " + err.Error())
		})

		aerc.PushStatus("Opened", 10*time.Second)
	})

	return nil
}
diff --git a/doc/aerc.1.scd b/doc/aerc.1.scd
index d77552f..45bd191 100644
--- a/doc/aerc.1.scd
+++ b/doc/aerc.1.scd
@@ -299,10 +299,13 @@ message list, the message in the message viewer, etc).
	Cycles between message parts being shown. The list of message parts is shown
	at the bottom of the message viewer.

*open*
*open* [-A <application>]
	Saves the current message part in a temporary file and opens it
	with the system handler.

	*-A* On macOS, the system handler can be overridden. For example, to open
	the current message part in a non-default editor, use :open -A MacVim

*save* [-fp] <path>
	Saves the current message part to the given path.
	If the path is not an absolute path, general.default-save-path will be
diff --git a/lib/open_darwin.go b/lib/open_darwin.go
index d98c898..83619c1 100644
--- a/lib/open_darwin.go
+++ b/lib/open_darwin.go
@@ -2,10 +2,11 @@ package lib

import (
	"os/exec"
	"strings"
)

func OpenFile(filename string, onErr func(error)) {
	cmd := exec.Command("open", filename)
	cmd := exec.Command("open", strings.Split(filename, "|")...)
	err := cmd.Start()
	if err != nil && onErr != nil {
		onErr(err)
-- 
2.7.4




-- 
JD
Details
Message ID
<20200927134946.pu3haz3slljtj7tl@feather.localdomain>
In-Reply-To
<C5RBPTZXRXVC.31QILPY07JA15@Penelope.local> (view parent)
DKIM signature
pass
Download raw message
Hi,

On Sat, Sep 19, 2020 at 02:53:33PM +0300, JD wrote:
> Differently from xdg-open, macOS's /usr/bin/open can also use non-default
> applications to open files. This patch adds -A parameter to aerc's :open
> command, which allows to open message parts with custom programs. E.g. to
> open a HTML part in a non-default browser, use :open -A Safari. Or even
> :open -A MacVim to open it in an editor instead of a browser.
>
> DISCLAIMER: again, I'm not a programmer, and this probably can be done in
> a way better manner (if someone needs this functionality at all :)

That's a very strange approach and doesn't really scale well.
You really don't need to do platform specific stuff here.

Much better would be to just pass any non recognized arguments to the open call
prior to the filename.

With that, the UX would look like:
`:open -a App` for you and `:open --funkyFlagBecauseICan` on my special snowflake™
patched system running a custom xdg-open wrapper, all without any of the build time
complexity and code duplication you introduced.

Cheers,
Reto
JD
Details
Message ID
<C5ZLXP64FMPC.2Q0YDF9C4OLSV@Penelope.local>
In-Reply-To
<20200927134946.pu3haz3slljtj7tl@feather.localdomain> (view parent)
DKIM signature
pass
Download raw message
On Sun Sep 27, 2020 at 15:49 +0200, Reto <reto@labrat.space> wrote:

> Much better would be to just pass
> any non recognized arguments to the
> open call prior to the filename.

Yeah, that makes sense, thanks for a hint.

However, about these possibile "non recognized arguments": this way, we'll have a case with no arguments at all (the usual `:open`) and a case with whatever is going after `:open` to be passed to open/xdg-open -- so, when to show help for :open then? Should we also introduce something like `:open --help`? No other commands seem to support that...

> With that, the UX would look like:
> `:open -a App` for you and `:open
> --funkyFlagBecauseICan` on my
> special snowflake™ patched system
> running a custom xdg-open wrapper,
> all without any of the build time
> complexity and code duplication you
> introduced.

But then on a system wtih the standard xdg-open passing additional parameters to it (by mystake, let's say) will result in exit status 1, am I wrong? Should we do anything in this case?






-- 
JD
Details
Message ID
<50D9C695-C911-4B7C-A521-DC88D2FCCDB9@labrat.space>
In-Reply-To
<C5ZLXP64FMPC.2Q0YDF9C4OLSV@Penelope.local> (view parent)
DKIM signature
pass
Download raw message
On 29 September 2020 07:35:20 CEST, JD <john1doe@ya.ru> wrote:

>However, about these possibile "non recognized arguments": this way,
>we'll have a case with no arguments at all (the usual `:open`) and a
>case with whatever is going after `:open` to be passed to open/xdg-open
>-- so, when to show help for :open then? Should we also introduce

What do you mean by help? The command completion?
 
>something like `:open --help`? No other commands seem to support
>that...

No, we'll simply document the behaviour and call it a day. `open [args...]` or some such.

>But then on a system wtih the standard xdg-open passing additional
>parameters to it (by mystake, let's say) will result in exit status 1,
>am I wrong? Should we do anything in this case?

Yes you are wrong, exit status 1 is not guaranteed. You could get any non zero exit status (some applications may associate meaning with the exit status).

Other than that, we'd do what we always do, bubble up the error to the user and let them handle it, after all it's their fault ;)

Cheers,
Reto
JD
Details
Message ID
<C5ZPT6N3D1S0.TA0R2IGWLAN2@Penelope.local>
In-Reply-To
<50D9C695-C911-4B7C-A521-DC88D2FCCDB9@labrat.space> (view parent)
DKIM signature
pass
Download raw message
On Tue Sep 29, 2020 at 08:15 +0200, Reto <reto@labrat.space> wrote:

> What do you mean by help? The
> command completion?

No, I mean that little informational hint it prints out now if used inappropriately, `return errors.New("Usage: open")`. Now, if we either use :open without any arguments, or pass all the provided arguments to the system opener, there will be no opportunity to provide this help message (only if requested specifically, like with `-h` / `--help`).

> Other than that, we'd do what we
> always do, bubble up the error to
> the user and let them handle it,
> after all it's their fault ;)

For example, `xdg-open` supports parameters such as `--version`. Now, if in aerc, we do something like `:open --version`, it will report `Opened` but for a user, nothing will happen. Of course, this is  some sort of a farcorner-fetched corner case but still, should we do something here?






-- 
JD
Details
Message ID
<4F15483C-BB34-4463-BF8C-F9460D159945@labrat.space>
In-Reply-To
<C5ZPT6N3D1S0.TA0R2IGWLAN2@Penelope.local> (view parent)
DKIM signature
pass
Download raw message
On 29 September 2020 10:37:31 CEST, JD <john1doe@ya.ru> wrote:
>On Tue Sep 29, 2020 at 08:15 +0200, Reto 
>For example, `xdg-open` supports parameters such as `--version`. Now,
>if in aerc, we do something like `:open --version`, it will report
>`Opened` but for a user, nothing will happen. Of course, this is  some
>sort of a farcorner-fetched corner case but still, should we do
>something here?


Nah, :open is a convenience function, nothing more. For fancy stuff use :term.

If people do funny things, it's on them.
JD
Details
Message ID
<C5ZS3ZQ4JI7S.36HWW6OV1WC05@Penelope.local>
In-Reply-To
<4F15483C-BB34-4463-BF8C-F9460D159945@labrat.space> (view parent)
DKIM signature
pass
Download raw message
On Tue Sep 29, 2020 at 11:42 +0200, Reto <reto@labrat.space> wrote:

> If people do funny things, it's on them.

OK, sent PATCH v2 then :)

(I tried `git am` it before sending, was applied OK for me.)






-- 
JD
Reply to thread Export thread (mbox)