Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support discord uri scheme #813

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Conversation

Covkie
Copy link
Collaborator

@Covkie Covkie commented Aug 13, 2024

closes: #769

Could someone on mac confirm this works as well? (I followed the electron docs so im like pretty sure it works but 🥴)

src/main/mainWindow.ts Outdated Show resolved Hide resolved
attempt to cover malformed or incomplete uris

Use RegExp to avoid escape character hell
@ryawaa
Copy link
Contributor

ryawaa commented Sep 11, 2024

Confirmed Working on macOS Sonoma 14.6 via open discord://
(tho it reloads the client, could be from invalid url)

Covkie and others added 3 commits September 11, 2024 17:34
also consolidated restore function
Discord handles incorrect schemes pretty well but `discord:///settings` is a case where it doesn't, so this may be needed to ensure user experience
src/main/index.ts Outdated Show resolved Hide resolved
src/main/ipc.ts Outdated Show resolved Hide resolved
src/main/mainWindow.ts Outdated Show resolved Hide resolved
src/main/mainWindow.ts Outdated Show resolved Hide resolved
src/main/mainWindow.ts Outdated Show resolved Hide resolved
src/main/ipc.ts Outdated Show resolved Hide resolved
src/preload/index.ts Outdated Show resolved Hide resolved
src/main/mainWindow.ts Outdated Show resolved Hide resolved
@Arcitec
Copy link

Arcitec commented Nov 29, 2024

Looks clean for the most part.

What's up with uriFiredDarwin? The logic does not make sense (never calling loadUrl if the value is true, and apparently calling it twice if the value is false):

    app.on("open-url", (_, url) => {
        if (uriFiredDarwin) restoreVesktop();
        else loadUrl(url);

...cut...

    if (!uriFiredDarwin) loadUrl(uri);

and there's no mention of needing to do that in the official Electron docs:

https://www.electronjs.org/docs/latest/tutorial/launch-app-from-url-in-another-app

Even if it's actually necessary, the current code with that variable is fragile and unclear (no clear flow). Can it be removed as per the Electron docs, or rewritten in a way that flows better? Or adding code comments to explain the reasoning behind the weird flow. Does it fix a platform-specific bug? Then please mention that in a comment. :)

Covkie and others added 2 commits November 30, 2024 00:53
I actually tested it inside a vm this time instead of guessing :p
@Vendicated
Copy link
Member

did anyone test this with flatpak?

@Arcitec
Copy link

Arcitec commented Dec 3, 2024

I've only had a really quick look at the new commit but the code definitely makes a lot more sense now. :) Now I finally understand its purpose.

So it seems like when Vesktop is being called as URL Open handler, on Mac it triggers an "open-url" event, and on Linux/Windows it instead triggers a "app executed with arg discord://theurl".

The new code makes sense and seems robust. Nice work. :)

4464752

Edit: The commit message is pretty funny too. ;)

@Covkie
Copy link
Collaborator Author

Covkie commented Dec 6, 2024

did anyone test this with flatpak?

It works but https://github.com/flathub/dev.vencord.Vesktop/blob/b55f7ec286f070143b3c9f9048cf043817aebba2/dev.vencord.Vesktop.yml#L35

-       - desktop-file-edit --set-key="Exec" --set-value="startvesktop" --set-icon=$FLATPAK_ID squashfs-root/vesktop.desktop
+       - desktop-file-edit --set-key="Exec" --set-value="startvesktop %U" --set-icon=$FLATPAK_ID squashfs-root/vesktop.desktop

Will have to be done. Other than that I'm not familiar with the rest of the flatpak spec to know if any metadata is needed for flathub regarding consuming a MimeType.

@4A8
Copy link

4A8 commented Dec 16, 2024

does this not work with args? for example jumping to a message or opening invites
if not then it's possible to achieve the same thing using registry keys on windows and some commands on linux (and the args are not working for me, it simply switches to the vesktop window
)

@Covkie
Copy link
Collaborator Author

Covkie commented Dec 16, 2024

This is NOT for an existing instance. At most it focuses the window like youre experiencing. This is for launching Vesktop from a closed state (not to tray!!!). Discord desktop has this identical behaviour.

arRPC handles existing instances via websockets. I'm assuming you have arrpc disabled since vesktop is not handling invites like you implied. I guess because of that case existing instance support can be implemented?

"args" do work btw

@4A8
Copy link

4A8 commented Dec 16, 2024

there are deeplinks that allow you to jump to a message that work on the normal discord app, but since arguments don't seem to be supported here they wont work (at least for me, even with arRPC enabled)
i think they look something like this: discord://-/channels/<server-id>/<channel-id>/<message-id>

@Covkie
Copy link
Collaborator Author

Covkie commented Dec 18, 2024

@4A8 can you explain in detail what about this PR isn't working as expected and how youre testing it. Executing vesktop https://discord.com/channels/1015060230222131221/1024351821873037462/1284611505505042463 works just fine for me and opens vesktop on the channel with its message highlighted.

if this doesn't work youre doing something wrong with building this branch. im not gonna really be able to help you there.

@4A8
Copy link

4A8 commented Dec 18, 2024

basically, doing exactly what you just did simply opens the vesktop window and does not highlight anything, i even tried with a link of my own

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Implement discord:// scheme / protocol
6 participants