Desktop: Register file type on Linux and Windows#4125
Open
timon-schelling wants to merge 5 commits intomasterfrom
Open
Desktop: Register file type on Linux and Windows#4125timon-schelling wants to merge 5 commits intomasterfrom
timon-schelling wants to merge 5 commits intomasterfrom
Conversation
timon-schelling
changed the title
Member
Author
|
!build desktop (Run ID 25513940875) |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request implements file associations for Graphite on both Linux and Windows platforms. On Linux, it updates the desktop entry and adds a MIME type definition. For Windows, it introduces a registry registration mechanism to associate .graphite and several image formats with the application. Feedback was provided regarding an inconsistency between the supported file extensions on Windows and the MIME types declared for Linux, suggesting an expansion of the Windows list for better platform parity.
|
|
|
timon-schelling
force-pushed
the
desktop-register-file-type-on-linux-and-win
branch
from
6dd6e84 to
8ca0dc9
Compare
timon-schelling
force-pushed
the
desktop-socket-mvp
branch
from
d33085b to
1a3573c
Compare
Member
Author
|
!build desktop (Run ID 25562585190) |
timon-schelling
force-pushed
the
desktop-register-file-type-on-linux-and-win
branch
from
0073f88 to
9029c6f
Compare
timon-schelling
force-pushed
the
desktop-socket-mvp
branch
from
1a3573c to
f7d4982
Compare
Member
Author
|
!build desktop (Run ID 25564141284) |
|
|
|
|
|
|
timon-schelling
force-pushed
the
desktop-register-file-type-on-linux-and-win
branch
from
9029c6f to
0ae71a8
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 7 files
Confidence score: 3/5
- There is a concrete stability risk in
desktop/platform/win/src/file_associations.rs: unwrappingenv::current_exe()can panic and crash during file association registration. - Given the issue is medium severity (6/10) with high confidence (10/10) and directly user-impacting on Windows flows, this introduces some merge risk rather than a purely cosmetic concern.
- Pay close attention to
desktop/platform/win/src/file_associations.rs- replace the unwrap with explicitResulthandling and graceful return/logging on failure.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="desktop/platform/win/src/file_associations.rs">
<violation number="1" location="desktop/platform/win/src/file_associations.rs:16">
P2: Avoid unwrapping `env::current_exe()` here; this can panic and crash the app during file association registration. Handle the `Result` and return/log on failure instead.
(Based on your team's feedback about avoiding panics from unwrap when resolving the executable path.) [FEEDBACK_USED]</violation>
</file>
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
| const SUPPORTED_EXTENSIONS: &[&str] = &[FILE_EXTENSION, ".svg", ".png", ".jpg", ".jpeg"]; | ||
|
|
||
| pub fn write() { | ||
| if let Err(e) = FileAssociationWriter::new(&env::current_exe().unwrap()) |
Contributor
There was a problem hiding this comment.
P2: Avoid unwrapping env::current_exe() here; this can panic and crash the app during file association registration. Handle the Result and return/log on failure instead.
(Based on your team's feedback about avoiding panics from unwrap when resolving the executable path.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At desktop/platform/win/src/file_associations.rs, line 16:
<comment>Avoid unwrapping `env::current_exe()` here; this can panic and crash the app during file association registration. Handle the `Result` and return/log on failure instead.
(Based on your team's feedback about avoiding panics from unwrap when resolving the executable path.) </comment>
<file context>
@@ -0,0 +1,100 @@
+const SUPPORTED_EXTENSIONS: &[&str] = &[FILE_EXTENSION, ".svg", ".png", ".jpg", ".jpeg"];
+
+pub fn write() {
+ if let Err(e) = FileAssociationWriter::new(&env::current_exe().unwrap())
+ .document_type(PROG_ID, DOCUMENT_FRIENDLY_NAME)
+ .application(EXECUTABLE_NAME, APP_FRIENDLY_NAME, SUPPORTED_EXTENSIONS)
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
depends on #4123
doing this on win feels horrible