feat(commands): sort command #1288
Conversation
|
I think it would be better to sort selections instead of lines since it gives more flexibility since the contents of the selection would be compared and reordered. It also reuses the concept of multiple cursors and selections which we want to do wherever possible. |
|
Note that this is also possible to do by using the pipe command to pipe each selection to |
|
👋 hi, first of all, apologies for very ad-hoc PR, should have discussed it before I even opened something. I wanted to learn more about the internal workings so I tried creating something that I am currently missing from vim as it seemed like a good start. Secondly, thanks a lot for the suggestions. I wasn't sure if my approach makes sense but your suggestions are definitely right, I will try to adjust the PR. Few questions I was wondering about:
Side note: thanks a lot for the awesome work on helix, it's something I was looking for for a long time and I enjoy it very much so far. |
|
My first reaction to this was that it is unnecessary, since you can just pipe to sort; however, on some more thought, I'm intrigued by the idea of sorting ranges. This would e.g. allow sorting the members of a list literal that don't even necessarily have to be on multiple lines. |
|
|
||
| fn sort_impl( | ||
| cx: &mut compositor::Context, | ||
| _args: &[Cow<str>], |
There was a problem hiding this comment.
Keeping this here as I assume there could be args for case-insensitivity, number sorting, etc. Maybe it might even be worth it to extract the whole implementation into some Sorter.
There was a problem hiding this comment.
Not a maintainer, but I don't see that being necessary unless there were many commands sharing this same function. A single option seems fine to leave as a bool.
|
So I think this might be actually pretty useful when working over selections. Little showcase: Screen.Recording.2021-12-22.at.0.11.13.mov |
|
|
||
| fn sort_impl( | ||
| cx: &mut compositor::Context, | ||
| _args: &[Cow<str>], |
There was a problem hiding this comment.
Not a maintainer, but I don't see that being necessary unless there were many commands sharing this same function. A single option seems fine to leave as a bool.
| name: "sort", | ||
| aliases: &[], | ||
| doc: "Sort lines in selection.", | ||
| fun: sort, |
There was a problem hiding this comment.
I'm not sure how much appetite @archseer has for doing GNU style command arguments, but if so, this could just be a single command that takes an -r flag to reverse?
There was a problem hiding this comment.
I don't have a set opinion here, I did consider it might be nice to have flag parsing for typed commands in the future.
|
Nice work, this will be super useful! |
| fragments.sort_by(match reverse { | ||
| true => |a: &Tendril, b: &Tendril| b.cmp(a), | ||
| false => |a: &Tendril, b: &Tendril| a.cmp(b), | ||
| }); |
There was a problem hiding this comment.
https://doc.rust-lang.org/std/cmp/struct.Reverse.html would make it clearer when you're reverse sorting (we already import it in the file)
There was a problem hiding this comment.
How do we use it though? Reverse is a struct, so if we try to use it in a match or if/else, it complains that the types are incompatible. It looks like it was meant to use with sort_by_key and friends where there is only one thing to reference.
Add basic implementation of sort command.
| }, | ||
| TypableCommand { | ||
| name: "sort", | ||
| aliases: &[], |
There was a problem hiding this comment.
Maybe we should do an alias sor which is similar to vim?
Add basic implementation of sort command.
This PR adds
sortandsort!(reverse sort) commands that sort selections. This can be used to not only sort data on separate lines (select -> split on new lineAlt-s->:sort) but also any arbitrary list (select -> select all matches, for example numbers\d+->:sort).Additional functionality can be added in the future such as removing duplicates, case insensitivity, etc.
edit: provide better description for the PR