Add URL protocol support for trilium://#9248
Conversation
- Register trilium:// as default protocol client - Handle URL protocol in second-instance event - Add IPC listener in client to open notes by ID - Fixes TriliumNext#649 - Open/focus a note from command line / desktop URL handler
dosubot
Bot
added
the
size:M
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements support for the trilium:// URL protocol, enabling the application to open specific notes from external links in both new and existing instances. The changes involve registering the protocol in the main process and setting up IPC listeners in the renderer. Feedback focuses on improving the robustness of the implementation by addressing a brittle setTimeout used for window readiness, removing a redundant single-instance lock check, and refactoring fragile, duplicated URL parsing logic. Additionally, it is recommended to avoid using the global object for state management to ensure better type safety.
| setTimeout(() => { | ||
| mainWindow.webContents.send("open-note-by-id", global.noteIdToOpen); | ||
| delete global.noteIdToOpen; | ||
| }, 1000); // Wait a bit for window to be ready | ||
| } |
There was a problem hiding this comment.
Using a hardcoded setTimeout of 1000ms to wait for the renderer process is brittle and creates a race condition. On slower systems or during heavy initialization (like database migrations), the renderer might not be ready in time to receive the IPC message, causing the note to fail to open. A more robust solution would be to have the renderer signal its readiness via IPC once the listener is registered, or at least use the dom-ready event of the webContents.
| const urlArg = commandLine.find(arg => arg.startsWith("trilium://")); | ||
| if (urlArg) { | ||
| const noteId = urlArg.replace("trilium://", "").replace(/^\/\//, ""); |
There was a problem hiding this comment.
The URL parsing logic here is fragile and duplicated. Specifically, the second .replace(/^\/\//, "") will not work as intended because the first .replace("trilium://", "") already consumes the double slashes. This would leave a leading slash if the URL is formatted as trilium:///noteId. Additionally, protocol matching should ideally be case-insensitive to handle various OS behaviors.
| const urlArg = commandLine.find(arg => arg.startsWith("trilium://")); | |
| if (urlArg) { | |
| const noteId = urlArg.replace("trilium://", "").replace(/^\/\//, ""); | |
| const urlArg = commandLine.find(arg => /^trilium:\/\//i.test(arg)); | |
| if (urlArg) { | |
| const noteId = urlArg.replace(/^trilium:\/+/i, ""); |
| const gotTheLock = app.requestSingleInstanceLock(); | ||
| if (!gotTheLock) { | ||
| app.quit(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
This call to app.requestSingleInstanceLock() is redundant. The single instance lock is already requested and handled at the beginning of the main() function (lines 107-111). If the lock is not acquired, the application process exits before reaching the onReady function.
| const urlArg = process.argv.find(arg => arg.startsWith("trilium://")); | ||
| if (urlArg) { | ||
| const noteId = urlArg.replace("trilium://", "").replace(/^\/\//, ""); |
There was a problem hiding this comment.
This parsing logic is duplicated from the second-instance handler and shares the same issues regarding leading slashes and case sensitivity.
| const urlArg = process.argv.find(arg => arg.startsWith("trilium://")); | |
| if (urlArg) { | |
| const noteId = urlArg.replace("trilium://", "").replace(/^\/\//, ""); | |
| const urlArg = process.argv.find(arg => /^trilium:\/\//i.test(arg)); | |
| if (urlArg) { | |
| const noteId = urlArg.replace(/^trilium:\/+/i, ""); |
| const noteId = urlArg.replace("trilium://", "").replace(/^\/\//, ""); | ||
| console.log(`Initial launch with note ID: ${noteId}`); | ||
| // Store note ID to be used after window is created | ||
| global.noteIdToOpen = noteId; |
There was a problem hiding this comment.
Using the global object to store application state is generally discouraged in TypeScript as it lacks type safety and can lead to naming collisions. A module-level variable (e.g., let noteIdToOpen: string | null = null;) at the top of the file would be a cleaner and more type-safe approach.
|
Hi, Unfortunately I can't merge this due to design issues as highlighted by Gemini (use of timeouts being one notable example). We'd prefer to manage this core feature on our side. |
2 participants
Add URL protocol support for trilium://
Description
This PR adds URL protocol support for Trilium Notes, allowing users to open notes via
trilium://URLs from external applications.Changes Made
1. Desktop Main Process (
apps/desktop/src/main.ts)trilium://as a default protocol clientsecond-instanceeventopen-note-by-idto renderer process2. Client Renderer Process (
apps/client/src/index.ts)open-note-by-idmessagesUsage Examples
Register URL protocol (automatically done on app launch):
# On first launch, Trilium registers itself as handler for trilium://Open note from command line:
Use in external applications:
trilium://note123linksTechnical Details
trilium://{noteId}trilium://prefix and any leading slashesrequestSingleInstanceLockfor proper handlingTesting
Related Issue
Fixes #649 - Open/focus a note from command line / desktop URL handler
IssueHunt Summary
Referenced issues
This pull request has been submitted to: