Codecov Report

Merging #9020 (5ee938f) into main (39cdf56) will increase coverage by 0.03%.
The diff coverage is 92.63%.

@@            Coverage Diff             @@
##             main    #9020      +/-   ##
==========================================
+ Coverage   68.97%   69.00%   +0.03%     
==========================================
  Files         641      641              
  Lines      102440   102497      +57     
==========================================
+ Hits        70656    70727      +71     
+ Misses      31784    31770      -14     

I think this is a cool PR, thanks!

I'm wondering if we can follow @Sygmei's suggestion in Discord and create a separate group-by block so we don't lose that powerful type of grouping? Here's an example of how to use it.

ls | group-by { get name | path parse | get extension } 
╭──────┬─────────────────╮
│ md   │ [table 3 rows]  │
│ lock │ [table 1 row]   │
│ toml │ [table 3 rows]  │
│      │ [table 12 rows] │
│ txt  │ [table 1 row]   │
│ sh   │ [table 4 rows]  │
│ cmd  │ [table 1 row]   │
│ nu   │ [table 4 rows]  │
│ rs   │ [table 1 row]   │
│ yml  │ [table 1 row]   │
│ json │ [table 1 row]   │
│ ps1  │ [table 1 row]   │
╰──────┴─────────────────╯

@rgwood I think you can make the two features co-exist by using the following SyntaxShape

            .optional(
                "grouper",
                SyntaxShape::OneOf(vec![
                    SyntaxShape::CellPath,
                    SyntaxShape::Block,
                    SyntaxShape::Closure(None),
                    SyntaxShape::Closure(Some(vec![SyntaxShape::Any])),
                ]),

I started working on this PR as well, I didn't know someone else was willing to take on this issue
What we need to do is probably merge your new code + old code
The old way of doing things (Grouper::ByBlock / Grouper::ByColumn) was kinda clunky imho, I like your way of doing things more, we just need to allow Blocks back and evaluate for each value (we can just copy paste the old way of doing things for blocks and keep your new system for CellPaths)

Good points, thanks. I might not have time/energy to implement that for a couple weeks.

@Sygmei would you like to take this issue+code and run with it? If not no worries, I'll get back to this eventually.

Sure, I'll give it a try !

@Sygmei Any luck? I may be able to pick this up again soon if you don't have time to work on it.

Go ahead ! In the end I had a pretty busy schedule as well :(

OK, this is ready to go! I've re-added the block/closure functionality and added an example.

It's getting a bit close to the next release; I think this one can wait until after the release.

let's go!