get back the ranges of the strings from the completer used for generating completions #713
Conversation
This reverts commit 070d600.
|
This is another reason why consolidating all of our menu code in one place is going to make the testing and landing of PRs much simpler going forward... This change will probably impact https://github.com/nushell/nushell/blob/main/crates/nu-cli/src/menus/description_menu.rs Meaning once we land this PR nushell may no longer compile ---- having all of our menu code in one place is probably the way to go... |
I didnt really mess with the menus in this one, i added https://github.com/maxomatic458/reedline/blob/5f65d771873b3a15b862604746106bd2bfec4a84/src/completion/base.rs#L36-L50 and then used that in the ide menu nushell should compile fine but i agree in the future its probably better to first rework the menus before adding new ones |
src/menu/mod.rs
Outdated
There was a problem hiding this comment.
Nice addition for simplicity and consolidation as it cleans up the columnar and list menu 👍
OK, now I understand this better ! Thanks... The description_menu will still compile fine --- I can get in there after this PR lands and modify the description menu in nushell similar to the way you modified the columnar and list menu removing the unused code ? |
|
@stormasm yes, that should work just fine |
|
@maxomatic458 Great work on this PR as it simplifies the code further as well as makes the end user of your API have more options for tweaking the menus they implement... Can you please add in some more "scenarios" to ide_completions.rs example.... let commands = vec![
"test".into(),
"hello world".into(),
"hello world reedline".into(),
"this is the reedline crate".into(),
];just so I can test some other types of cases as well... I saw the test you added in too which is great... Also --- maybe we can add in the ability to have some new lines just so we can see how the code behaves if you throw some new lines in there as well ?? Besides the tests you added --- its kind of cool and helpful to be able to play with your code a bit more with an expanded example that gives the end user more options to understand and test further the cool concepts you have implemented... fn add_newline_keybinding(keybindings: &mut Keybindings) {
// This doesn't work for macOS
keybindings.add_binding(
KeyModifiers::ALT,
KeyCode::Enter,
ReedlineEvent::Edit(vec![EditCommand::InsertNewline]),
);
}similar to -> https://github.com/stormasm/nutmp/blob/main/reedline/ide_completions.rs |
|
Once your PR lands obviously we will have a lot more abilities to test it with the nushell code base... But for the folks who just use reedline as end users and are not using nushell at all this makes the ide_completions.rs example more robust... |
|
So here is the idea that I have --- let me know what you think... In your expanded example or new example outline and show the scenario to the end user of two cases:
correct_cursor_pos: false
correct_cursor_pos: trueThis way the end user can have a better mental model of whats going on... Thanks ! |
|
can i just add those variables in the example, like so let min_completion_width = 0;
let max_completion_width = 50;
let max_completion_height = u16::MAX;
let padding = 0;
let border = false;
let cursor_offset = 0;
let description_mode = DescriptionMode::PreferRight;
let min_description_width = 0;
let max_description_width = 50;
let description_offset = 1;
let correct_cursor_pos = false;
let commands = vec![
"test".into(),
"hello world".into(),
"hello world reedline".into(),
"this is the reedline crate".into(),
];
let completer = Box::new(DefaultCompleter::new_with_wordlen(commands, 2));
let mut ide_menu = IdeMenu::default()
.with_name("completion_menu")
.with_min_completion_width(min_completion_width)
.with_max_completion_width(max_completion_width)
.with_max_completion_height(max_completion_height)
.with_padding(padding)
.with_cursor_offset(cursor_offset)
.with_description_mode(description_mode)
.with_min_description_width(min_description_width)
.with_max_description_width(max_description_width)
.with_description_offset(description_offset)
.with_correct_cursor_pos(correct_cursor_pos);
...so someone could just change them? (ill also add some comments then) |
| @@ -680,12 +699,18 @@ impl Menu for IdeMenu { | |||
| // Also, by replacing the new line character with a space, the insert | |||
| // position is maintain in the line buffer. | |||
| let trimmed_buffer = editor.get_buffer().replace("\r\n", " ").replace('\n', " "); | |||
| completer.complete( | |||
| completer.complete_with_base_ranges( | |||
There was a problem hiding this comment.
The expanded example with the added ability to throw some new lines in there will test this part of the code better
sounds perfect, thank you ! |
|
@stormasm quick question /// Menu builder with the default border value
#[must_use]
pub fn with_default_border(mut self) -> Self {
self.default_details.border = Some(BorderSymbols::default());
self
}i have this function in the ide menu builder, do you think i should change it to this? /// Menu builder with the default border value
#[must_use]
pub fn with_default_border(mut self, default_border: bool) -> Self {
self.default_details.border = if default_border {
Some(BorderSymbols::default())
} else {
None
}
self
}would be a breaking change for nushell, but i would probably do a PR there anyways to load the new config setting for the ide menu |
Lets leave it alone for now and not make the change.... |
|
ok, then i think im done with the PR, |
|
@maxomatic458 thanks for making the requested changes in the examples... good job 😄 |
closes #711
adds the function
complete_with_base_rangesto the Completer trait, that returns the ranges, of the strings that the Suggestions were calculated from.the ide menu can now use of this, to adjust the position of the suggestions to the typed text matches the suggestion.
