-
Notifications
You must be signed in to change notification settings - Fork 604
feat: add "Open in Finder" menu option #7873
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
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@AndrewMast is attempting to deploy a commit to the GitButler Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! This looks good to me, there does however appear to be some formatting issues on the TS side.
Thank you greatly for the contribution!
92eb61e
to
1d2dd7b
Compare
shortcutService.on('open-in-finder', () => { | ||
const path = `file://${project.path}`; | ||
openExternalUrl(path); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be leveraging tauri's revealItemInDir
method with their official opener
plugin?
https://v2.tauri.app/reference/javascript/opener/#revealitemindir
shortcutService.on('open-in-finder', () => { | |
const path = `file://${project.path}`; | |
openExternalUrl(path); | |
}); | |
shortcutService.on('open-in-finder', async () => { | |
try { | |
// Use the Opener plugin to reveal the folder in the native file explorer | |
await invoke('plugin:opener|reveal_item_in_dir', { path: project.path }); | |
} catch (e) { | |
const message = ` | |
Failed to reveal directory in file explorer: | |
${project.path} | |
`; | |
showToast({ title: 'Explorer error', message, style: 'error' }); | |
// Rethrowing for sentry and posthog | |
throw e; | |
} | |
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for highlighting this!
It looks like the tuari opener function relies on the same open
crate that our implementation already does. We seem to have extra handling for AppImages that the tauri plugin doesn't have.
In the longer term we should upstream our extra logic and move over to the tauri plugin, but for now I think it's fine.
Sorry for the bother, I've just been trying this out locally and it appears that diff --git a/crates/gitbutler-tauri/src/open.rs b/crates/gitbutler-tauri/src/open.rs
index adb2bce66..0e19def90 100644
--- a/crates/gitbutler-tauri/src/open.rs
+++ b/crates/gitbutler-tauri/src/open.rs
@@ -16,6 +16,7 @@ pub(crate) fn open_that(path: &str) -> anyhow::Result<()> {
"zed",
"windsurf",
"cursor",
+ "file",
]
.contains(&target_url.scheme())
{ Again, many thanks for the contribution |
Ok then I’ll add |
1d2dd7b
to
7b4d051
Compare
@Caleb-T-Owens @matthewevans How does the last commit look? Did I take it too far with the new function? I feel like in the future, we could have an "Open in Finder" option in the context menu after the "Open in VSCode" and utilize this function: |
🧢 Changes
Added an
Open in Finder
menu option.☕️ Reasoning
Uses
file://
and theproject.path
(opening in Windows uses\\
so I didn't useproject.vscodePath
).I originally was going to add a function similar to
getEditorUri
, but that seemed like too much of a project change for too little code and for a first contribution. Might be best to create agetFinderUri
and write os-specific implementations if it is necessary.🎫 Affected issues
Fixes: #7853