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

Cmd+click on a file with a line from iTerm2 should open to that line #22990

Open
1 task done
hgezim opened this issue Jan 10, 2025 · 9 comments
Open
1 task done

Cmd+click on a file with a line from iTerm2 should open to that line #22990

hgezim opened this issue Jan 10, 2025 · 9 comments
Labels
admin read Pending admin review enhancement [core label] triage Maintainer needs to classify the issue

Comments

@hgezim
Copy link

hgezim commented Jan 10, 2025

Check for existing issues

  • Completed

Describe the feature

It currently opens the file but not to the line.

Zed Version and System Specs

Zed: v0.168.2 (Zed)
OS: macOS 14.6.1
Memory: 16 GiB
Architecture: aarch64

If applicable, add mockups / screenshots to help present your vision of the feature

No response

@hgezim hgezim added admin read Pending admin review enhancement [core label] triage Maintainer needs to classify the issue labels Jan 10, 2025
@SomeoneToIgnore SomeoneToIgnore added the needs info / awaiting response Issue that needs more information from the user label Jan 10, 2025
@SomeoneToIgnore
Copy link
Contributor

We do have such functionality overall, here's a recent addition to that, for example: #22559

So, can you provide more details: which kind of link that is, where it's open, etc.

@hgezim
Copy link
Author

hgezim commented Jan 10, 2025

CleanShot 2025-01-10 at 16 35 20@2x

Format: lib/phapi_web/components/profile_menu_component.ex:103:16

@SomeoneToIgnore SomeoneToIgnore removed the needs info / awaiting response Issue that needs more information from the user label Jan 10, 2025
@SomeoneToIgnore
Copy link
Contributor

regular.project.mov

Just to note, it works just fine in regular Zed project: clicks do open the file at the correct location.

So, without extra details it's not quite clear what's broken.

@CharlesChen0823
Copy link
Contributor

this is because the path is lib/phapi_web/components/profile_menu_component.ex:103:16:, one more : in the path end, which not support open.

@SomeoneToIgnore
Copy link
Contributor

Great point about the trailing : but I think

which not support open.

is wrong, unless I'm doing something odd: I've created a dummy file with the exact same path and pasted lib/phapi_web/components/profile_menu_component.ex:103:16: into a file that I cat inside the terminal: works for me just fine.

odd.mov

@SomeoneToIgnore SomeoneToIgnore added the needs info / awaiting response Issue that needs more information from the user label Jan 12, 2025
@CharlesChen0823
Copy link
Contributor

sorry, it's my previous operator error. i cannot reproduce that

@bennetbo
Copy link
Contributor

bennetbo commented Jan 13, 2025

When using the integrated terminal this works as expected, but not when clicking on links inside a standalone iTerm2 application (i think this might be what the issue is about).

Screen.Recording.2025-01-13.at.11.27.22.mov

Not sure why this is not working for zed, need to investigate this more. In the meantime you can follow this workaround posted here, which makes zed work as expected.

@SomeoneToIgnore SomeoneToIgnore removed the needs info / awaiting response Issue that needs more information from the user label Jan 13, 2025
@SomeoneToIgnore
Copy link
Contributor

Great point that it's iTerm2 external click -> Zed opening, I've totally missed that.

Then, it's handled by the OS which expects an application (which zed CLI is not) to open any link clicked:
image

That is handled in

extern "C" fn open_urls(this: &mut Object, _: Sel, _: id, urls: id) {
let urls = unsafe {
(0..urls.count())
.filter_map(|i| {
let url = urls.objectAtIndex(i);
match CStr::from_ptr(url.absoluteString().UTF8String() as *mut c_char).to_str() {
Ok(string) => Some(string.to_string()),
Err(err) => {
log::error!("error converting path to string: {}", err);
None
}
}
})
.collect::<Vec<_>>()
};

If I add

log::error!("$$$?: absoluteString: {string:?}");

change inside filter_map after retrieving that property, I see

$$$?: absoluteString: "file:///Users/someonetoignore/work/zed/zed/crates/editor/src/inlay_hint_cache.rs"

in my logs when clicking thread 'inlay_hint_cache::tests::test_large_buffer_inlay_requests_split' panicked at crates/editor/src/inlay_hint_cache.rs:2333:17: in iTerm2.

Not sure what to try after that:
https://developer.apple.com/documentation/appkit/nsapplicationdelegate/application(_:open:)?language=objc
mentions it's NSURL we get:
https://developer.apple.com/documentation/foundation/nsurl?language=objc

but all my attempts to read other data from this URL ended up with nothing that resembles the suffix: port is 0, query() call segraults, etc.

Interesting to note, that VSCode as a default app for *.rs opening, manages to handle this right.

@hgezim
Copy link
Author

hgezim commented Jan 14, 2025

Workaround works great:

Image

/usr/local/bin/zed \1:\2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
admin read Pending admin review enhancement [core label] triage Maintainer needs to classify the issue
Projects
None yet
Development

No branches or pull requests

4 participants