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

Allow copying paths from the File Tab context menu #15260

Merged

Conversation

HarshNarayanJha
Copy link
Contributor

@HarshNarayanJha HarshNarayanJha commented Jul 26, 2024

Pretty bare-bones implementation of copying paths from tab context menu. Right now, both actions do the same thing (i.e. copy relative path), as I need to find a way to get the absolute path Figured out!

Release Notes:

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Jul 26, 2024
crates/workspace/src/pane.rs Outdated Show resolved Hide resolved
crates/workspace/src/pane.rs Outdated Show resolved Hide resolved
@williamdes
Copy link
Contributor

Can you also post a screenshot please?

@HarshNarayanJha
Copy link
Contributor Author

Sure!

20240726_16h01m19s_grim

Read only if you want to answer

Btw, why is the target folder 16GB after compiling? And even if I change a line in pane.rs, upon running again using cargo run, why it again compiles some of the crates which I haven't touched? (It always starts compiling from workspace crate, 1145/1193, and then takes a lot of time and CPU, and heats up my laptop).
Any way to change this behaviour?

I know rust projects are heavy, but this is too much for my laptop. I only started learning rust a few months ago and didn't expect this much from big projects 😂. Likewise, I plan to move zed repo to my external ssd to save space.

Another thing, I am using stable zed to edit the files, which is a little unusable since cargo checks every crate upon a single edit, which slows down the zed, and my laptop. I need to disable checking while writing code. Thank god I didn't open it up in VSC

@williamdes
Copy link
Contributor

williamdes commented Jul 26, 2024

Btw, why is the target folder 16GB after compiling? And even if I change a line in pane.rs, upon running again using cargo run, why it again compiles some of the crates which I haven't touched? (It always starts compiling from workspace crate, 1145/1193, and then takes a lot of time and CPU, and heats up my laptop).
Any way to change this behaviour?

Yeah, quite a pain
cargo clean is your friend
@SomeoneToIgnore do you know why Zed re builds all crates each time?

I know rust projects are heavy, but this is too much for my laptop. I only started learning rust a few months ago and didn't expect this much from big projects 😂. Likewise, I plan to move zed repo to my external ssd to save space.

haha, same

Another thing, I am using stable zed to edit the files, which is a little unusable since cargo checks every crate upon a single edit, which slows down the zed, and my laptop. I need to disable checking while writing code. Thank god I didn't open it up in VSC

Hmm, I think I have found an option in the settings.json for the rust language that disables this.
Having tons of projects in my forked folder it tries to compile all of them. This is a pain.
Will send you the option here when I am back
Edit: I did not find the setting

@SomeoneToIgnore
Copy link
Contributor

These questions indeed belong to some #new-rustaceans generic channel, but let's do that once

Btw, why is the target folder 16GB after compiling?

This means you've compiled it too few times, you need to try a few more times, with different toolchain versions preferrably

du -h target/
...snip
189G	target

cargo and rustc need various intermediate data for incremental compilation and build creation.
In addition to that, default debug builds contain a lot of "extra" things like debug information, not-fully-optimized-and-reduced code (that could cargo --release or a differently configured linker do at cost of much more CPU usage), and Zed uses a lot of dependencies, exposing such issues more than inside an average rust project.

Whenever something in the build chain diverges (a crate version, a rustc version, etc.) — whatever's affected by that change has to be recompiled, bringing in more artefacts, debug info, incremental info, etc.
As cargo does not know the semantics of such changes, it preserves old versions (what if you just switch a branch and now switching back, wanting faster recompilation?) of old files, sometimes too aggressively by my taste.

cargo clean or, better, https://github.com/holmgr/cargo-sweep might help here

And even if I change a line in pane.rs, upon running again using cargo run, why it again compiles some of the crates which I haven't touched?

pane.rs is a part of a quite foundational crate, workspace, and now every crate that depends on workspace, transitively or not, has to recheck whether it used (transitively or not) certain modules that you've changed or not — and if you did, it has to recompile.
And, of course, the final binary has to be relinked nonetheless.

and then takes a lot of time and CPU, and heats up my laptop).
Any way to change this behaviour?

Realistically — using a different machine, either a remote one (Zed has some remote capabilities enough to work and it improves every month).

Apart from that, you could install a different linker if you're on Linux (Zed CI uses mold) and tinker with cargo profiles more, adding another near

zed/Cargo.toml

Line 506 in 20213a7

[profile.dev]
with something, that improves your situation (no good suggestions, docs and cargo --timings to profile your local build).

After that — only some work on Zed's dependencies to reduce them, but not sure what are bottlenecks on your machine.

which is a little unusable since cargo checks every crate upon a single edit

There are two things: your default settings save files very often, presumably after editor's focus is lost (which might be good by itself) — that triggers the default feature of rust-analyzer, "cargo check on save", which produces most of its diagnostics currently.

So, either you could move into state of manual saves only, or tweak check on save (up to disabling it fully): https://zed.dev/docs/languages/rust#large-projects-and-performance

… `worktree.absolutize()`

add a new `paths` variable to include both relative and absolute paths in `pane.rs`, and use those paths in the context menu option.
@HarshNarayanJha
Copy link
Contributor Author

Thanks very much for the help, Someone! rust-analzer options have been very helpful, I can now use intellisense (though a bit outdated), but it works!

Now it's ready to merge. I may also create the PR to unify Copy{Path,RelativePath} into workspace::, if possible.

Copy link
Contributor

@SomeoneToIgnore SomeoneToIgnore left a comment

Choose a reason for hiding this comment

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

I really liked the version from https://github.com/HarshNarayanJha/zed/blob/212605984629f8c9cc7019577bd14f497e4a85d1/crates/workspace/src/pane.rs more, now it's a bunch of questionable code added which is harder to read.

crates/workspace/src/pane.rs Outdated Show resolved Hide resolved
crates/workspace/src/pane.rs Outdated Show resolved Hide resolved
crates/workspace/src/pane.rs Outdated Show resolved Hide resolved
crates/workspace/src/pane.rs Outdated Show resolved Hide resolved
also re-create copy_relative_path into it's own function, while the menu entry itself handles copying absolute path (by the value from Pane::get_absolute_path)
@HarshNarayanJha
Copy link
Contributor Author

Hope this helps!

Copy link
Contributor

@SomeoneToIgnore SomeoneToIgnore left a comment

Choose a reason for hiding this comment

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

Thank you.
I've pushed some style fixes as it seemed faster to do it myself than commenting.

Now it looks fine to merge.

@SomeoneToIgnore SomeoneToIgnore merged commit b05d532 into zed-industries:main Jul 27, 2024
9 checks passed
kevmo314 pushed a commit to kevmo314/zed that referenced this pull request Jul 29, 2024
…ed-industries#15260)


Release Notes:

- Added "Copy Path" and "Copy Relative Path" items into tab context menu ([zed-industries#13970](zed-industries#13970))

---------

Co-authored-by: Kirill Bulatov <[email protected]>
@HarshNarayanJha
Copy link
Contributor Author

Why is this PR not in the latest release?

@SomeoneToIgnore
Copy link
Contributor

Hello again.
#15148 (comment)

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.

3 participants