Repository: nushell/nushell
File: crates/nu-command/src/filters/group_by.rs (Rust)

Code before:

pub fn group_by(
    engine_state: &EngineState,
    stack: &mut Stack,
    call: &Call,
    input: PipelineData,
) -> Result<PipelineData, ShellError> {
    let head = call.head;
    let groupers: Vec<Value> = call.rest(engine_state, stack, 0)?;
    let to_table = call.has_flag(engine_state, stack, "to-table")?;
    let config = engine_state.get_config();

    let values: Vec<Value> = input.into_iter().collect();
    if values.is_empty() {
        return Ok(Value::record(Record::new(), head).into_pipeline_data());
    }

    let mut closure_idx = 0;

    // Error early on unsupported types by collecting
    let groupers = groupers
        .into_iter()
        .map(|val| match val {
            Value::CellPath { val, .. } => Ok(Grouper::CellPath { val }),
            Value::Closure {
                val, internal_span, ..
            } => {
                closure_idx += 1;
                Ok(Grouper::Closure {
                    val,
                    idx: closure_idx - 1,
                    span: internal_span,
                })
            }
            _ => Err(ShellError::TypeMismatch {
                err_message: "unsupported grouper type".to_string(),
                span: val.span(),
            }),
        })
        .collect::<Result<Vec<_>, ShellError>>()?;

    if to_table {
        let grouper_names = groupers
            .iter()
            .map(Grouper::to_column_name)
            .collect::<IndexSet<_>>();

        if grouper_names.len() != groupers.len() {
            return Err(ShellError::GenericError {
                error: "grouper arguments have duplicate names".into(),
                msg: "contains duplicates".into(),

Review comment (bug):
Typically, we don't allow internal_span to be used since the plan is to eventually remove them. I think the way we solved this previously, iirc, is to get the span for each grouper and use it. I could be saying that wrong.

Code after:
    call: &Call,
    input: PipelineData,
) -> Result<PipelineData, ShellError> {
    let head = call.head;
    let groupers: Vec<Spanned<Grouper>> = call.rest(engine_state, stack, 0)?;
    let to_table = call.has_flag(engine_state, stack, "to-table")?;
    let config = engine_state.get_config();

    let values: Vec<Value> = input.into_iter().collect();
    if values.is_empty() {
        return Ok(Value::record(Record::new(), head).into_pipeline_data());
    }

    let grouped = match &groupers[..] {
        [first, rest @ ..] => {
            let mut grouped = Grouped::new(first.as_ref(), values, config, engine_state, stack)?;
            for grouper in rest {
                grouped.subgroup(grouper.as_ref(), config, engine_state, stack)?;
            }
            grouped
        }
        [] => Grouped::empty(values, config),
    };

    let value = if to_table {
        let column_names = groupers_to_column_names(&groupers)?;
        grouped.into_table(&column_names, head)
    } else {
        grouped.into_record(head)
    };

    Ok(value.into_pipeline_data())
}

fn groupers_to_column_names(groupers: &[Spanned<Grouper>]) -> Result<Vec<String>, ShellError> {
    if groupers.is_empty() {
        return Ok(vec!["group".into(), "items".into()]);
    }

    let mut closure_idx: usize = 0;
    let grouper_names = groupers.iter().map(|grouper| {
        grouper.as_ref().map(|item| match item {
            Grouper::CellPath { val } => val.to_column_name(),
            Grouper::Closure { .. } => {
                closure_idx += 1;
                format!("closure_{}", closure_idx - 1)
            }
        })
    });

    let mut name_set: Vec<Spanned<String>> = Vec::with_capacity(grouper_names.len());