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

Add copy_path_with_file_number shortcut #25029

Closed
wants to merge 2 commits into from

Conversation

pawurb
Copy link

@pawurb pawurb commented Feb 17, 2025

Hi. It's my first time contributing so sorry in advance if I'm mixing something up.

This PR adds shortcuts enabling copying file path (absolute and relative) with line number appended, in the following format:

crates/editor/src/editor.rs:13094

I use this feature constantly when working in VS code to run only a specific test case with Rspec (other testing frameworks also support this behaviour).

I've also noticed that there is editor::CopyPermalinkToLine shortcut, so a local path with line number should probably also be supported.

Release Notes:

  • Added CopyPathWithLineNumber and CopyRelativePathWithLineNumber shortcuts

This comment was marked as resolved.

@pawurb

This comment was marked as resolved.

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Feb 17, 2025

This comment was marked as resolved.

@notpeter
Copy link
Member

Where is the ctrl-k n / cmd-k n shortcut coming from?
Is that duplicating another editor or just a suggestion?

@pawurb
Copy link
Author

pawurb commented Feb 18, 2025

@notpeter suggestion

@Angelk90
Copy link
Contributor

@pawurb : Since copy_path_with_line_number and copy_relative_path_with_line_number are two functions with similar logic, it's not possible to create a single generic function by simply passing a parameter to determine whether the path is relative or absolute.

Example, I don't know if it works:

pub enum PathType {
    Absolute,
    Relative,
}


pub fn copy_path_with_line_number(
        &mut self,
        path_type: PathType,
        _: &zed_actions::workspace::CopyPathWithLineNumber,
        _window: &mut Window,
        cx: &mut Context<Self>,
    ) {
        let path_target = match path_type {
            PathType::Absolute => self.target_file_abs_path(cx),
            PathType::Relative => self.target_file_path(cx),
        };

        if let Some(path) = path_target {
            if let Some(path) = path.to_str() {
                let selection = self.selections.newest::<Point>(cx).start.row + 1;
                let path_with_line_number = format!("{}:{}", path, selection);
                cx.write_to_clipboard(ClipboardItem::new_string(path_with_line_number));
            }
        }
    }


self.copy_path_with_line_number(PathType::Absolute, action, window, cx); //Absolute
self.copy_path_with_line_number(PathType::Relative, action, window, cx); //Relative

@pawurb
Copy link
Author

pawurb commented Feb 18, 2025

@Angelk90 I don't think we can change the method params. I've followed the convention from copy_path and copy_relative_path where there's repetition. It should be possible to extract the core logic to a subfunction, but in that case we should probably also refactor the original functions.

@Angelk90
Copy link
Contributor

@pawurb :
When I have repeated code that does similar things but has some differences, I like to create a function that abstracts this logic.
That way, if I need to make changes in the future, I can just update one function instead of having to do it in multiple places in the code.
In this case, I would do the same for copy_path and copy_relative_path.

@pawurb
Copy link
Author

pawurb commented Feb 20, 2025

I've refactored calls to remove repetition.

@maxdeviant
Copy link
Member

Thank you for the PR!

There is already an action that does this, editor::CopyFileLocation.

It produces output like what you are expecting:

crates/extension_host/src/extension_host.rs:1218

@maxdeviant maxdeviant closed this Feb 22, 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.

4 participants