Skip to content

Conversation

@viiru-
Copy link
Contributor

@viiru- viiru- commented Apr 22, 2025

Add a function for downloading media, and use it as the default action for m.audio, m.file and m.video.

Now that authenticated media is the expected situation opening these as links in an external browser no longer works, so try this instead.

"Open in the chosen application for this mime type" would be nice as well, but based on looking at the way Gnus does this it seems harder than expected. Perhaps there is something I don't understand here..

@alphapapa
Copy link
Owner

Thanks, that looks useful. I pushed a few changes on top of your branch. If you concur, I'll merge it.

@alphapapa alphapapa self-assigned this Apr 23, 2025
@alphapapa alphapapa added the enhancement New feature or request label Apr 23, 2025
@alphapapa alphapapa added this to the v0.17 milestone Apr 23, 2025
@alphapapa alphapapa force-pushed the support-downloading-media branch 2 times, most recently from babc9f4 to c72199e Compare April 23, 2025 01:12
@alphapapa
Copy link
Owner

Thanks for adding the bindings. Part of me feels like it should be an uppercase letter, in case we want to use lowercase-D for a more mundane command in the future, but I don't feel too strongly about it. What do you think?

@viiru-
Copy link
Contributor Author

viiru- commented May 3, 2025

Thanks for adding the bindings. Part of me feels like it should be an uppercase letter, in case we want to use lowercase-D for a more mundane command in the future, but I don't feel too strongly about it. What do you think?

Either works fine for me, all I need is the command somewhere so that I get access to the flies sent to me. I'll switch it to uppercase, for both the room command and the transient, I guess?

@viiru- viiru- force-pushed the support-downloading-media branch from a03c96a to 9df7f55 Compare May 3, 2025 07:32
@alphapapa alphapapa force-pushed the support-downloading-media branch from 9df7f55 to effc225 Compare May 10, 2025 03:24
@alphapapa alphapapa merged commit e52c5a7 into alphapapa:master May 10, 2025
0 of 4 checks passed
@alphapapa
Copy link
Owner

Thanks, @viiru-!

@phil-s
Copy link

phil-s commented May 10, 2025

The downloading fails for me for the video posted by amadaluzia in the Emacs room on 2025-04-16/17 (depending on your time zone).

((:id . "$ir0NiwBgEwMXIqsYvnbz0pADpE2gPTU43lgi5hQMN9s")
 (:sender . "@amadaluzia:tchncs.de")
 (:content (body . "16_Apr_2025-20:17:04.mp4")
	   (info (duration . 16751) (h . 1080)
		 (mimetype . "video/mp4") (size . 5227651)
		 (thumbnail_info (h . 337) (mimetype . "image/webp")
				 (size . 15236) (w . 600))
		 (thumbnail_url
		  . "mxc://tchncs.de/1dfc70b7f402d3af38c2f7c4b7bc5955bd1bf85b1912586167333683200")
		 (w . 1920))
	   (msgtype . "m.video")
	   (url
	    . "mxc://tchncs.de/6b7f989f5ca789c3d4169756eff8a59a2571a6401912586226855051264"))
 (:origin-server-ts . 1744831067529) (:type . "m.room.message")
 (:state-key) (:unsigned (membership . "join")) (:receipts) (:local))

I get ement-room-download-file: File already exists: "/path/to/Downloads/" when I use D, and if I click the mouse I get button-activate: Wrong number of arguments: #<subr ement-room-download-file>, 1

This is the only download URL I could recall seeing, so I haven't tested anything else.

@phil-s
Copy link

phil-s commented May 10, 2025

And re-testing with a fresh config, I get error: (void-variable eww-download-directory) from D, so I assume it's also missing a (require 'eww).

Loading eww manually got me back to the previous pair of errors.

alphapapa added a commit that referenced this pull request May 11, 2025
alphapapa added a commit that referenced this pull request May 11, 2025
@alphapapa
Copy link
Owner

Thanks for testing and reporting, @phil-s. The two commits I just pushed should solve those things, with one exception: pressing C-u before clicking a download link doesn't prompt for a filename the same way C-u D does. I don't know how to fix that right now, and I'm going to leave that for later. :)

@phil-s
Copy link

phil-s commented May 12, 2025

Confirming that the fixes work for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants