Skip to content
This repository has been archived by the owner on Dec 16, 2021. It is now read-only.

Add hotkey to open message's image attachments in feh #256

Merged
merged 2 commits into from
Mar 3, 2020
Merged

Add hotkey to open message's image attachments in feh #256

merged 2 commits into from
Mar 3, 2020

Conversation

joshiemoore
Copy link
Contributor

feh is a lightweight and powerful image viewer. This PR adds a hotkey which opens all of a message's image attachments in feh. The hotkey is mapped to 'o' by default.

Select a message that has attached image(s) and press 'o', then the image(s) will be opened for viewing in feh. This negates the need to copy/paste the attachment links into a web browser. feh will simply ignore any file types it is passed that it doesn't support (such as mp4, etc).

This feature requires feh to be installed. If feh is not installed and the user attempts to use this feature, a graceful error dialog will be displayed.

Note that this feature only handles actual image attachments - it will not process image links that are pasted directly into the chat bar and sent. Those types of links will still need to be manually copy+pasted into a browser.

@Bios-Marcel
Copy link
Owner

Bios-Marcel commented Feb 29, 2020

How about we make it a configurable command instead of hardcoding feh? So windowsusers could make use of it for example. Or linuxusers can use their own tool of choice, if they dislike feh maybe.

@Bios-Marcel
Copy link
Owner

Related #241

@joshiemoore
Copy link
Contributor Author

I have pulled the image viewer selection out into the user config instead of having it be hard-coded.

@Bios-Marcel
Copy link
Owner

So ... your PR got me thinking a bit. Wouldn't it be better to have a solution that works for all kind of files in a system-independent matter?

I thought about smth like this:

		if shortcuts.ViewSelectedMessageImages.Equals(event) {
			downloadedFiles := make(chan string)
			go func() {
				for _, file := range message.Attachments {
					cacheDir, osError := os.UserCacheDir()
					if osError != nil {
						log.Println(osError)
						return
					}

					pathInCache := filepath.Join(cacheDir, filepath.Base(file.URL))
					_, statError := os.Stat(pathInCache)
					if os.IsNotExist(statError) {
						response, httpError := http.Get(file.URL)
						if httpError != nil {
							log.Println(httpError)
							return
						}
						defer response.Body.Close()

						handle, osError := os.Create(pathInCache)
						if osError != nil {
							log.Println(osError)
							return
						}

						_, copyError := io.Copy(handle, response.Body)
						if copyError != nil {
							log.Println(copyError)
							return
						}
					} else if statError != nil {
						log.Println(statError)
						continue
					}

					log.Printf("Path :%s\n", pathInCache)
					downloadedFiles <- pathInCache
				}

				close(downloadedFiles)
			}()

			go func() {
				for file := range downloadedFiles {
					err := open.Run(file)
					if err != nil {
						log.Println(err)
					}
				}
			}()

			return nil
		}

This will download the file, cache it and then ask the system to display the file. Obviously this will pose a security risk, therefore the mimetype could be checked to see whether the file is an image or video and not an executable. In case of file types that are potentially dangerous more often, a dialog could be shown. I don't know, but I'd prefer this way of doing it guess. Here, if you want to test this out, apply this patch:

diff --git a/go.mod b/go.mod
index 9863fb2..2dd16fe 100644
--- a/go.mod
+++ b/go.mod
@@ -21,6 +21,7 @@ require (
 	github.com/nu7hatch/gouuid v0.0.0-20131221200532-179d4d0c4d8d // indirect
 	github.com/pkg/errors v0.8.1
 	github.com/robertkrimen/otto v0.0.0-20180617131154-15f95af6e78d
+	github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966
 	github.com/tadvi/systray v0.0.0-20190226123456-11a2b8fa57af // indirect
 	golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4 // indirect
 	gopkg.in/sourcemap.v1 v1.0.5 // indirect
diff --git a/go.sum b/go.sum
index 3a92a50..2d200eb 100644
--- a/go.sum
+++ b/go.sum
@@ -95,6 +95,8 @@ github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
 github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
 github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
 github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
+github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966 h1:JIAuq3EEf9cgbU6AtGPK4CTG3Zf6CKMNqf0MHTggAUA=
+github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966/go.mod h1:sUM3LWHvSMaG192sy56D9F7CNvL7jUJVXoqM1QKLnog=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
diff --git a/ui/window.go b/ui/window.go
index cd4cbd0..d8955ea 100644
--- a/ui/window.go
+++ b/ui/window.go
@@ -3,14 +3,18 @@ package ui
 import (
 	"bytes"
 	"fmt"
+	"io"
 	"log"
+	"net/http"
+	"os"
+	"path/filepath"
 	"strings"
 	"time"
 	"unicode"
-	"os/exec"
 
 	"github.com/mattn/go-runewidth"
 	"github.com/mdp/qrterminal/v3"
+	"github.com/skratchdot/open-golang/open"
 
 	"github.com/Bios-Marcel/cordless/util/fuzzy"
 	"github.com/Bios-Marcel/cordless/util/text"
@@ -387,17 +391,56 @@ func NewWindow(doRestart chan bool, app *tview.Application, session *discordgo.S
 		}
 
 		if shortcuts.ViewSelectedMessageImages.Equals(event) {
-			links := make([]string, 0, len(message.Attachments))
-			for _, file := range message.Attachments {
-				links = append(links, file.URL)
-			}
-			if len(links) > 0 {
-				cmd := exec.Command(config.Current.ImageViewer, links...)
-				err := cmd.Start()
-				if err != nil {
-					window.ShowErrorDialog(err.Error())
+			downloadedFiles := make(chan string)
+			go func() {
+				for _, file := range message.Attachments {
+					cacheDir, osError := os.UserCacheDir()
+					if osError != nil {
+						log.Println(osError)
+						return
+					}
+
+					pathInCache := filepath.Join(cacheDir, filepath.Base(file.URL))
+					_, statError := os.Stat(pathInCache)
+					if os.IsNotExist(statError) {
+						response, httpError := http.Get(file.URL)
+						if httpError != nil {
+							log.Println(httpError)
+							return
+						}
+						defer response.Body.Close()
+
+						handle, osError := os.Create(pathInCache)
+						if osError != nil {
+							log.Println(osError)
+							return
+						}
+
+						_, copyError := io.Copy(handle, response.Body)
+						if copyError != nil {
+							log.Println(copyError)
+							return
+						}
+					} else if statError != nil {
+						log.Println(statError)
+						continue
+					}
+
+					log.Printf("Path :%s\n", pathInCache)
+					downloadedFiles <- pathInCache
 				}
-			}
+
+				close(downloadedFiles)
+			}()
+
+			go func() {
+				for file := range downloadedFiles {
+					err := open.Run(file)
+					if err != nil {
+						log.Println(err)
+					}
+				}
+			}()
 
 			return nil
 		}
@@ -1140,7 +1183,7 @@ func NewWindow(doRestart chan bool, app *tview.Application, session *discordgo.S
 }
 
 func getWelcomeText() string {
-	return fmt.Sprintf(splashText + `
+	return fmt.Sprintf(splashText+`
 
 Welcome to version %s of Cordless. Below you can see the most
 important changes of the last two versions officially released.

@joshiemoore
Copy link
Contributor Author

I'm not comfortable with that. While it's very generalized, and I'm sure it will work well, I wouldn't want to be downloading and running arbitrary files that people attach to Discord messages in that way. I'd rather pass image links directly to an image viewer, and video links directly to a video player.

Of course this is your package so the decision is yours, but the original way has been working quite well for me so I'll stick with it. Thanks for your time

@Bios-Marcel
Copy link
Owner

Yes, I see that as a valid argument. I've also mentioned that in my previous comment. Maybe this could be done only for certain mime-types. And other files would display a dialog instead: Do you really want to open X. I'd like to find a solution that satisfies everyones needs. That way we can avoid having forks that split apart from the original project :D

Maybe we can have another solution where you can hardcode certain applications for certain filetypes. So you can have some kind of handler. There'd be the default handler and specific handlers. And if you will, you can simply disable the default handler completly. This is more of a random idea. But anyway, I'd like to have a satisfying solution that requires no configuration or installation of extra-tools and isn't dangerous.

@Bios-Marcel
Copy link
Owner

I am merging this for now 😄

@Bios-Marcel Bios-Marcel merged commit 3b764ac into Bios-Marcel:master Mar 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants