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 C-y to yank picker selection to clipboard #3786

Closed
wants to merge 1 commit into from

Conversation

kirawi
Copy link
Member

@kirawi kirawi commented Sep 10, 2022

I think this is useful for easily searching diagnostics online.

Copy link
Member

@CBenoit CBenoit left a comment

Choose a reason for hiding this comment

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

Looks good to me! Minor comment.

@@ -556,6 +556,22 @@ impl<T: Item + 'static> Component for Picker<T> {
ctrl!('t') => {
self.toggle_preview();
}
ctrl!('y') => {
if let Some(contents) = self.selection() {
let contents = contents.sort_text(&self.editor_data).to_string();
Copy link
Member

@CBenoit CBenoit Sep 11, 2022

Choose a reason for hiding this comment

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

(Oops, I forgot to add the comment. Here it is.)

Since the sort text is not always equal to the stringified label depending on the Item, maybe we specifically want String::from(contents.label(&self.editor_data)) so that it always copies what is actually rendered in the picker minus the style?

Also when you need a String by consuming a Cow<str>, you might want to use into_owned instead in order to steal the String if it is already allocated.

@kirawi kirawi added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. A-helix-term Area: Helix term improvements labels Sep 13, 2022
@crushingCodes
Copy link

i was just looking for this as i usually copy error messages often and its already a pr nice!

@CBenoit
Copy link
Member

CBenoit commented Mar 6, 2023

Hi @kirawi
Any chance you could complete this PR?

@Antonio-Bennett
Copy link

@kirawi Could this be pushed ahead again? This is a super helpful PR! Just switched over from neovim and this is definitely something I miss :)

@kirawi
Copy link
Member Author

kirawi commented Feb 9, 2024

I will finish this PR tomorrow or the day after.

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

I don't think we want to add this to the picker specifically. A command to yank the diagnostic under the cursor would be fine I think, as #9527 describes, and it would be roughly equivalent to this: just use the picker (or ]d/[d etc) to jump to a diagnostic and then have a command to yank it. Alternatively we could add a special register for the diagnostic(s) under the cursor and then it would be covered by #9552.

But as we move away from the menu::Item trait in picker and towards a table layout yanking will make less sense. We could concatenate all of the columns for the selected row but that seems hacky to me.

@kirawi kirawi closed this Mar 18, 2024
@kirawi kirawi deleted the picker-popup branch March 18, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants