-
Notifications
You must be signed in to change notification settings - Fork 158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
get correct cursor pos when menu indicator contains newline #708
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #708 +/- ##
==========================================
- Coverage 49.69% 47.59% -2.11%
==========================================
Files 46 47 +1
Lines 8433 9312 +879
==========================================
+ Hits 4191 4432 +241
- Misses 4242 4880 +638
|
it would be great if we had a test for this to ensure no breakage in the future. /cc @stormasm |
I am not sure this is working correctly as I tested it by modifying your ide_completions example with the following code fn add_newline_keybinding(keybindings: &mut Keybindings) {
keybindings.add_binding(
KeyModifiers::ALT,
KeyCode::Enter,
ReedlineEvent::Edit(vec![EditCommand::InsertNewline]),
);
} and then this code as well... let mut keybindings = default_emacs_keybindings();
add_menu_keybindings(&mut keybindings);
add_newline_keybinding(&mut keybindings);
let edit_mode = Box::new(Emacs::new(keybindings)); similar to the way it works in the demo example... and then tried typing one letter basically just the "t" and then insert one new line and then hit tab... See if it works properly on your machine / I am on a Mac After the third new line is entered I am not seeing test anymore I just see "this is the reedline crate" |
im not to sure what you mean by that? |
So check out the demo example Type an "h" at the prompt and you will see another "h" show up on the screen many lines down probably where the menu ends... This is a separate issue than the one I mentioned earlier... |
@stormasm so i tried that but i dont think i was able to replicate it. Did i do something wrong? (did you also try the latest commit?) if you referred to the cut off at the bottom, then thats intended (you can scroll throught the options that dont fit on the screen) |
this might be somewhat intentional, i think thats just a multiline history suggestion. but you cant hit tab to complete it, so i guess thats bugged, and it probably shouldnt even show up if you are not in multiline mode |
This is your example -> ide_completions.rs modified to show whats going on... https://github.com/stormasm/nutmp/blob/main/reedline/ide_completions.rs Note that after the third input line the first line "test" goes away --- this does not seem correct to me... pr708-0119.mov |
Here is a slightly longer video --- you will note that after the third InsertNewLine that "test" for some reason disappears... pr708-01-19-take2.mov |
as soon as i enter a newline i get "NO RECORDS FOUND" until i enter text in the current line, i start the example i also tried this on the columnar menu and it does the same can you check if this happens for other menus as well? |
ok i was able to replicate it, by using the unix linebreaks |
@stormasm reedline/src/menu/columnar_menu.rs Line 546 in 31eaeeb
causes this to only happen on unix (only replace "\n", not "\r\n") changing the line to let trimmed_buffer = editor.get_buffer()
.replace("\r\n", " ")
.replace("\n", " "); will lead to it to work on windows as well. but there is still the issue that some of the suggestion dissapear after a few lines (like your clip above) i think this might be something for a seperate PR (as this isn't caused by the ide menu, and happens in the other menus as well) should i open up a seperate issue for this? |
Ah, line endings. we've seen that time and time again in reedline and nushell, since we're cross platform. nice catch. Let's see what @stormasm says but imo, I think I'd put this change let trimmed_buffer = editor.get_buffer()
.replace("\r\n", " ")
.replace("\n", " "); in a separate PR in case we have to revert it (or this current PR) for some odd reason. |
just squeezed this in here as well in preperation for full support to nushell |
I will defer to @fdncred for the call on what to do here as I don't know what the best way to go is... |
@maxomatic458 can you open another pr for that line ending change? We'll land that real quick. |
does this need to be rebased to accommodate the line ending pr? |
@fdncred i don't think so |
Ok. Let's give it a try. |
fixes a bug from #696