Skip to content
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

Fixing hints when not editing at end of line #722

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jerbs
Copy link

@jerbs jerbs commented Aug 14, 2023

Currently, hints are only working correctly when editing at the end of the line.
Let me describe the current behaviour using a simple example:
The line contains "abcd" and the cursor is at position zero when typing "123". That text has the hint "1234567890_".

  1. rustyline displays: 123|abcd{456789_}
  2. After typing a 4: 1234|abcd{56789_}
  3. After cursor right: 1234a|bcd{56789_}
  4. After cursor right: 1234ab|cd{56789_}
  5. After cursor right: 1234abc|d{56789_}
  6. After cursor right: 1234abcd|{56789_}
  7. After cursor right: 1234abcd56789_|

The pipe character indicates the cursor position. The curly braces do contain the hint.

Multiple issues:

  • Hint is displayed at end of line, not directly right of cursor.
  • Hint stays active when moving the cursor.
  • Remaining part of hint is finally inserted at the end of the line.
  • No possibility to complete a hint when not at end of line.

This pull request fixes all four issues.

@gwenn
Copy link
Collaborator

gwenn commented Aug 15, 2023

Could you please provide your Hinter implementation ?
Or to say otherwise, are you sure that the issue is in rustyline and not in your hinter ?

@@ -128,7 +128,8 @@ impl<'out, 'prompt, H: Helper> State<'out, 'prompt, H> {
if self.layout.cursor == cursor {
return Ok(());
}
if self.highlight_char() {
if self.highlight_char() || self.has_hint() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why moving the cursor should invalidate the hint ?
At least, this should be configurable, not hardcoded.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my example in the initial description go the pull request the cursor is moved in steps (3) to (7). In all these positions it does not make sense to insert the remaining partial part of the hint. Try to use it with the app I uploaded https://github.com/jerbs/fsidx. It has a shell mode with hints and completions implemented for long options starting with "--". I don't see in which scenario the currently implemented behaviour is useful.

E(K::Right, M::NONE) if wrt.has_hint() && wrt.is_cursor_at_end() => Cmd::CompleteHint,
E(K::Right, M::ALT) if wrt.has_hint() => Cmd::CompleteHint,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer not to have this default binding in rustyline.
See ConditionalEventHandler to add your own custom binding.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I have seen that API. I can remove that commit from the pull request, if that's your preference. Just thought that it gives a better out of the box experience for everybody.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Alt+Right to get the hint completed also feels quite natural to me. With the rustyline default Alt+Right jumps to the end of the word. And that's exactly what happens when the hint is displayed right next to the cursor and Alt+Right is used.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #406 where Ctrl-E feels quite natural to someone else...

@@ -979,11 +979,21 @@ impl Renderer for PosixRenderer {
}
// display hint
if let Some(hint) = hint {
// position the cursor within the line
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So your PR works only on unix, right ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not have access to any Windows. I also don't know if Windows terminals have an insert mode? The resulting difference between Unix and Windows is that for Windows the hint is still displayed at the end of the line while it is displayed right next to the cursor on Unix. So, the difference is on presentation level only.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jerbs
Copy link
Author

jerbs commented Aug 15, 2023

Could you please provide your Hinter implementation ?
Or to say otherwise, are you sure that the issue is in rustyline and not in your hinter ?

See https://github.com/jerbs/fsidx for my hinter implementation.
The main branch uses to official rustyline. The hint branch uses the forked rustyline with my proposed fix.

@jerbs jerbs requested a review from gwenn August 15, 2023 08:58
@gwenn
Copy link
Collaborator

gwenn commented Aug 15, 2023

I will do some tests with other similar libraries but for me all of them use hint / auto-suggestion at the end of the line.
What you propose is more like auto-completion...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants