# Refactoring completion menus

Methods that have some amount of code duplication:

- `update_values`
- This doesn't seem like something the menu should concern itself with tbh
- `replace_in_buffer`
- This also doesn't feel like one of the menu's responsibilities
- The description menu's implementation is slightly different from the others, because it also looks at examples, but it's not a big deal, we can account for that
- `can_partially_complete` - Columnar menu and IDE menu both have the same code

## Things we could do to reduce code duplication

### Moving the description_menu into Reedline or a separate menu crate

stormasm suggested this in [#706](https://github.com/nushell/reedline/issues/706). It would make sharing logic between the description menu and other menus easier, as well as updating menu logic. `NuHelpCompleter` would have to remain in the Nushell repo, though.

> Not sure the best location of this code but options would include...
> - Moving the description menu over to reedline from nushell
> - Having a separate menu crate that lives in the reedline repo ?
> - Having a separate menu crate that lives inside the nushell organization
so that both reedline and nushell have access to them along with other
projects that want a terminal based menu system.

### Helper functions

Reedline already has a [`menu_functions`](https://github.com/nushell/reedline/blob/main/src/menu/menu_functions.rs) module with some helper functions. We can put more functionality there (other Reedline users will also be able to use them, not just Nushell). This won't require changing the API in any way.

This option may suit `can_partially_complete` in particular well. Originally, only the columnar menu had an actual implementation for it, but now that the IDE menu uses the same logic, it can be extracted to a helper function.

### Moving functionality out of `Menu`

If it weren't for breaking the API, this might not be a bad idea for `replace_in_buffer`. The description menu does get an example if applicable, but if we make `get_value` a method on `Menu` and `replace_in_buffer` uses that, the description menu could return a modified `Suggestion` that has the example text in the `value` field.

maxomatic458 also [suggested](https://github.com/nushell/reedline/issues/706#issuecomment-1908866461) extracting `name`, `active`, and other fields into a separate struct:
> so i guess we could do something like this then

struct MenuCommon {
    name: String,
    active: bool,
    // a bunch of fields that the menus share
}

// and then include this in the menus
struct Menu {
    common: MenuCommon,
    some_specific_thing_for_this_menu: ...
}

> this would remove a good bit of code duplication, but then we would need two builders (one for the MenuCommon, and one for the menu itself) to build a menu

As for `update_values`, the list of suggestions could be maintained completely separately from the menu itself. However, this would be a bit difficult to implement since only the menu knows which value should be selected when you hit down or tab or whatever. A similar approach that wouldn't necessarily require breaking the API would be with a helper struct.

### Helper struct to hold suggestions

Like the helper functions option, but this would give less power to the menus. The helper struct could have a method to update suggestions, as well as a `can_partially_complete`-like method.

The menu could hold the struct as one of its members (instead of having a `values: Vec<Suggestion>` like it currently does), and we could either
- change the `get_values()` method to return this struct (breaking change D:), or
- we could keep the API exactly the same. Reedline users could choose to use this helper struct to make life easier if they're fine with losing some power.

We could also hold the struct separately and pass it to any methods on `Menu` that need it. Breaking API change, though.

### ???