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

pane: Hide "Copy Relative Path" and "Reveal In Project Panel" actions for files outside of the projects #23349

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

Conversation

Poldraunic
Copy link
Contributor

Split off from #21000.

"Copy Relative Path" action had a check for existence of relative path but it always passed because WorktreeStore::find_worktree() function returned empty path for these kinds of files. It feels correct to make changes there, but I don't know what else could be impacted.

"Reveal In Project Panel" had no check whatsoever and so I made one.

Release Notes:

  • N/A

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jan 19, 2025
Check for this action is already in place but it always passes because resolved relative path is an empty Path.
@Poldraunic Poldraunic force-pushed the pane-treat-empty-path-as-if-it-were-none branch from 16a4038 to a6aeb3a Compare January 30, 2025 12:08
@@ -2466,7 +2466,9 @@ impl Pane {
.read(cx)
.item_for_entry(entry, cx)
.and_then(|item| item.project_path(cx))
.map(|project_path| project_path.path);
.map(|project_path| project_path.path)
.map_or(None, |path| if path.exists() { Some(path) } else { None });
Copy link
Contributor

@SomeoneToIgnore SomeoneToIgnore Jan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.exists()

  1. We prefer to avoid using std::fs-related methods and use Fs instead,

    async fn is_file(&self, path: &Path) -> bool;
    in this case.

  2. This is a rendering code, potentially called 120 times/second, calling whatever FS-related things here is madness.
    Is there another way we can check that file is outside of projects?

  3. We can be on remote, where the client will have no FS at all.

@SomeoneToIgnore SomeoneToIgnore self-assigned this Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants