Add yank_joined command #7195

spectre256 · 2023-06-01T05:01:28Z

Resolves issue #6888 by adding a command to join all selections and yank them to the specified register. The typed command takes an argument as the separator to use when joining the selections.

This command works the same as yank_joined_to_clipboard except that it yanks to a register.

pascalkuthe

I am not sure if introducing a new command is a right thing tondo here. I think we could make the register configurable instead (so the yank_joined command yanks to clipbaors by default but you can specify a different register). I am not sure if that's already possible or if that requires #6985

CC @the-mikedavis

pascalkuthe · 2023-06-01T11:39:05Z

helix-term/src/commands.rs

+    let (view, doc) = current!(editor);
+    let text = doc.text().slice(..);
+
+    let values: Vec<String> = doc


The existing code does this too but this code ks allocating twice here which is a bit unnecessary/potentially slow for large selections so maybe we can fix that while we are working on this anyway.

Instead you can iterate the Selection::slices text.slice(range) and then push_str to the result. This will require special handeling for the first element but you can just use that to intalize the string: let mut res = slices.next,().unwrap().to_string()

Thanks for your feedback! I think I've fixed it.

Also, about introducing a new command: since the register is already configurable, I think it should be fine to leave as is. Once the special registers for the clipboard is added, it will be able to be used with the system clipboard. (and the yank_joined_to_clipboard command can be removed) The " register seems like a sane default to me. This also reflects the discussion in #6888.

Or did you mean the typed command? I could add an argument for which register to use?

Oh and one more thing: there's an experimental intersperse_with method for iterators on nightly that can be enabled with #![feature(iter_intersperse)]. Since it's experimental, I didn't use it, but it would simplify some of the code. Should I just add a note in the comments or something?

the-mikedavis · 2023-06-07T23:11:50Z

Ideally join+yank to the clipboards would be implemented in terms of this command. I would prefer to add this and eventually remove the yank_to_*_clipboard commands. I think this command could be useful on its own too.

spectre256 · 2023-06-12T17:28:01Z

Hey everyone, is there anything else I could do to get this merged? No rush, just checking in :)

the-mikedavis · 2023-06-13T01:04:14Z

No we just need to find time to review the changes. I have a bunch more PRs in my queue to look at than usual since I was away for a while but I'd like to look at this sooner rather than later

helix-term/src/commands.rs

the-mikedavis

Just a minor thing, otherwise this looks good

the-mikedavis · 2023-06-13T22:36:53Z

helix-term/src/commands/typed.rs

+    if event != PromptEvent::Validate {
+        return Ok(());
+    }
+


I don't think we do this consistently in typable commands but we can give some feedback when the command gets more arguments than expected:

ensure!(args.len() <= 1, ":yank-join takes at most 1 argument");

Just updated it! Would it be helpful to make a pr to add this to the other commands as necessary?

Yeah I think it would be good to have some feedback especially on the new/save file operations like new, write, format, etc 👍. Discarding arguments silently seems potentially confusing to me

Resolves issue helix-editor#6888 by adding a command to join all selections and yank them to the specified register. The typed command takes an argument as the separator to use when joining the selections.

David-Else · 2023-10-12T16:13:51Z

I think I am missing something obvious, but how exactly do I yank to a chosen register. I make multiple selections and run this command using :yank-join and it yanks and joins to ".

Once the special registers for the clipboard is added, it will be able to be used with the system clipboard. (and the yank_joined_to_clipboard command can be removed)

This has happened now.

the-mikedavis · 2023-10-12T18:06:13Z

:yank-join takes a string to join the selections as its argument rather than a register to yank into. You can select a register ("*) and then execute yank_joined for example from the command picker

pascalkuthe reviewed Jun 1, 2023

View reviewed changes

spectre256 force-pushed the master branch from 8f06028 to 7c84d96 Compare June 1, 2023 18:32

spectre256 requested a review from pascalkuthe June 2, 2023 17:12

kirawi added A-helix-term Area: Helix term improvements S-waiting-on-review Status: Awaiting review from a maintainer. labels Jun 5, 2023

the-mikedavis self-requested a review June 7, 2023 23:11

the-mikedavis reviewed Jun 13, 2023

View reviewed changes

helix-term/src/commands.rs Outdated Show resolved Hide resolved

helix-term/src/commands.rs Outdated Show resolved Hide resolved

the-mikedavis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from a maintainer. labels Jun 13, 2023

spectre256 force-pushed the master branch 3 times, most recently from ae0a33a to d93728e Compare June 13, 2023 22:08

the-mikedavis reviewed Jun 13, 2023

View reviewed changes

Add yank_joined command

4a0ca8e

Resolves issue helix-editor#6888 by adding a command to join all selections and yank them to the specified register. The typed command takes an argument as the separator to use when joining the selections.

spectre256 force-pushed the master branch from d93728e to 4a0ca8e Compare June 14, 2023 00:29

spectre256 requested a review from the-mikedavis June 16, 2023 01:37

the-mikedavis approved these changes Jun 16, 2023

View reviewed changes

the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 16, 2023

pascalkuthe approved these changes Jun 16, 2023

View reviewed changes

pascalkuthe merged commit d8b7232 into helix-editor:master Jun 16, 2023
6 checks passed

gabydd mentioned this pull request Jun 23, 2023

Add yank_joined command #6888

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add yank_joined command #7195

Add yank_joined command #7195

spectre256 commented Jun 1, 2023

pascalkuthe left a comment

pascalkuthe Jun 1, 2023

spectre256 Jun 1, 2023 •

edited

spectre256 Jun 1, 2023

the-mikedavis commented Jun 7, 2023

spectre256 commented Jun 12, 2023

the-mikedavis commented Jun 13, 2023

the-mikedavis left a comment

the-mikedavis Jun 13, 2023

spectre256 Jun 14, 2023

the-mikedavis Jun 16, 2023

David-Else commented Oct 12, 2023

the-mikedavis commented Oct 12, 2023

Add yank_joined command #7195

Add yank_joined command #7195

Conversation

spectre256 commented Jun 1, 2023

pascalkuthe left a comment

Choose a reason for hiding this comment

pascalkuthe Jun 1, 2023

Choose a reason for hiding this comment

spectre256 Jun 1, 2023 • edited

Choose a reason for hiding this comment

spectre256 Jun 1, 2023

Choose a reason for hiding this comment

the-mikedavis commented Jun 7, 2023

spectre256 commented Jun 12, 2023

the-mikedavis commented Jun 13, 2023

the-mikedavis left a comment

Choose a reason for hiding this comment

the-mikedavis Jun 13, 2023

Choose a reason for hiding this comment

spectre256 Jun 14, 2023

Choose a reason for hiding this comment

the-mikedavis Jun 16, 2023

Choose a reason for hiding this comment

David-Else commented Oct 12, 2023

the-mikedavis commented Oct 12, 2023

spectre256 Jun 1, 2023 •

edited