[lldb] Display autosuggestion part in gray if there is one possible suggestion

[gedatsu217]

Summary

I implemented autosuggestion if there is one possible suggestion.
I set the keybinds for every character. When a character is typed, Editline::TypedCharacter is called.
Then, autosuggestion part is displayed in gray, and you can actually input by typing C-k.
Editline::Autosuggest is a function for finding completion, and it is like Editline::TabCommand now, but I will add more features to it.

Testing does not work well in my environment, so I can't confirm that it goes well, sorry. I am dealing with it now.

[labath]

Thanks for the patch. It's a very interesting feature, but I'm not sure it is actually a good idea. Some tab completions can be very expensive (not to mention they can crash lldb), and so running them after every keypress sounds problematic to me. Editors usually have very complex logic to implement these things efficiently and unobtrusively. It usually involves running the completions on a separate thread with various delays to avoid stalling a user who types fast and avoid burning the cpu needlessly.

I don't see anything like that in this patch, and I'm not sure the complexity that would entail is worth avoiding pressing tab a couple of times. If we really do want this feature, I think it should be controlled by a setting, and probably the setting should be off by default.

I'm not sure what's your main motivation for this, but they way I could see this potentially being useful is if we had a notion of "lightweight" completions. These would things that don't do any extraordinary amount of work (like parsing gigabytes of debug info) -- just basic completions for lldb commands and their arguments.

Regarding the actual patch, I have two quick comments:

• please avoid unrelated formatting changes -- run clang-format only over the lines you've changed (git clang-format HEAD^ or something similar)
• don't work with stdout directly -- you should be using the "stdout" notions local to the Editline class (e.g.., m_output_file)

[teemperor]

@labath Just to get you into the loop: This is for a GSoC project that is about implementing the autosuggestions similar to what fish shell is providing. It's not really about actually invoking any completion logic (even though that might be an option in the future), but more about the 'history and context' based search fish is doing. This patch is just WIP and I asked @gedatsu217 to upload it that that I can give some feedback and make sure he's on the right track.

@gedatsu217 I forgot to say that if you upload reviews for early review that marking them as WIP and maybe not add the LLDB tag (as this will put all LLDB folks in CC). Just adding me/Jonas is fine for those patches.

[labath]

Thanks for the explanation Raphael. This makes more sense now, though I am still not very clear on the distinction between "completions" and "suggestions". The fish tutorial mentions command history -- that's something that's not typically done as a part of tab completion (though maybe it could be?). However, the rest -- file paths and command arguments -- that's clearly in scope for tab completion too and our tab handlers already do that. So, the idea of having "lightweight" completions which would also get run as a part of "suggestions" does not seem completely off track. Unless I am completely off track, that is...

[teemperor]

In D81001#2068925, @labath wrote:

Thanks for the explanation Raphael. This makes more sense now, though I am still not very clear on the distinction between "completions" and "suggestions". The fish tutorial mentions command history -- that's something that's not typically done as a part of tab completion (though maybe it could be?). However, the rest -- file paths and command arguments -- that's clearly in scope for tab completion too and our tab handlers already do that. So, the idea of having "lightweight" completions which would also get run as a part of "suggestions" does not seem completely off track. Unless I am completely off track, that is...

It's more like a smarter and more user-friendly Ctrl+R search. Having it do something similar to tab completion is a stretch goal at the end. But at the moment the plan is to do just history based suggestions, then see if history + context is possible (i.e., make the suggestions paired to a specific target that you only get a suggestion for b ASTContext.cpp:434 if you debug LLDB but not if you debug another random program) and then the lightweight completions if there is time left.

[teemperor]

So a few notes here in addition to Pavel's feedback:

• This patch should come with some basic backend for suggestions, but I don't think the tab completions are the right start (see Pavel's comments about why this doesn't work). The plain command history is a safe start and also what fish shell is doing, so I think that's what the minimal viable version of this patch should look like. See m_command_history in the CommandInterpreter for where the command history is stored (that history is maybe limited to the current session from what I understand, but it's enough for getting some functionality in here).
• This should be behind a setting (which will be off by default for now), so you should create a LLDB setting for that. lldb/source/Core/CoreProperties.td is the place to declare the setting and you need to add some code to pass that setting around (you can take the "use-colors" setting as an example for how to expose that setting via the Debugger class).
• Regarding the test, I think you'll have to test the interactive part via a pexpect test (which will start LLDB in a fake terminal and you can send input and read output from that terminal). See TestMultilineNavigation.py for a pexpect example test.

lldb/source/Host/common/Editline.cpp

1279

You can make indent_chars a llvm::StringRef and then you can just iterate over the characters via for (char c : indent_chars) ....

[gedatsu217]

In short, should I implement autosuggestion using m_command_history at first?

[gedatsu217]

I implemented autosuggestion based on command history first according to your advice.
But I did not understand what you said completely. So I want to know it in detail.

This should be behind a setting (which will be off by default for now), so you should create a LLDB setting for that.

↑I added some codes in CoreProperties.td, but I do not understand what this file means. Is it not enough?

Regarding the test, I think you'll have to test the interactive part via a pexpect test (which will start LLDB in a fake terminal and you can send input and read output from that terminal). See TestMultilineNavigation.py for a pexpect example test.

↑This means that I should make test cases like TestMultineNavigation.py?

Regarding the bug of python in my environment that I told you via email, I have not solved it. I do not know the causes, so I am dealing with it.

[gedatsu217]

ping

[teemperor]

Sorry for the delay.

In D81001#2074145, @gedatsu217 wrote:

In short, should I implement autosuggestion using m_command_history at first?

Yes, that's the plan. It's enough for a first version.

In D81001#2077240, @gedatsu217 wrote:

I implemented autosuggestion based on command history first according to your advice.
But I did not understand what you said completely. So I want to know it in detail.

This should be behind a setting (which will be off by default for now), so you should create a LLDB setting for that.

↑I added some codes in CoreProperties.td, but I do not understand what this file means. Is it not enough?

CoreProperties.td just used to generate the file CoreProperties.inc (this generation is done by a LLVM tool called "tablegen"). If you change CoreProperties.td, you will change CoreProperties.inc which is included Debugger.cpp. The *.inc files just contain variable declarations that represent the settings. So you still need to write a function that reads that setting from the variables declares in the *.inc file. You can just take the Debugger::GetUseColor method as a template and change the ePropertyUseColor value there to ePropertyUseAutosuggestion for your new method (e.g. Debugger::GetUseAutosuggestions).

Regarding the test, I think you'll have to test the interactive part via a pexpect test (which will start LLDB in a fake terminal and you can send input and read output from that terminal). See TestMultilineNavigation.py for a pexpect example test.

↑This means that I should make test cases like TestMultineNavigation.py?

Regarding the bug of python in my environment that I told you via email, I have not solved it. I do not know the causes, so I am dealing with it.

lldb/include/lldb/Host/Editline.h

321

This function and the one below should be documented like the rest.

lldb/source/Core/CoreProperties.td

134

I think ShowAutosuggestions/show-autosuggestions is the right way to put it.

lldb/source/Host/common/Editline.cpp

1054

typed seems to be always a single character but the type is std::string? It also seems redundant as ch should contain the same character?

1064

.empty() instead of .length() == 0?

1071

Just to clarify Pavel's point: This should be using m_output_file and not stdout. Same for the printf above.

1250

Why ^k (I assume this should be ^f for Ctrl+f (which is the keybinding fish is using).

1257

I think this code can just disappear if you remove the typed parameter from TypedCharacter.

lldb/source/Interpreter/CommandInterpreter.cpp

1881

This could just return a std::string. Even better, it could return a llvm::Optional<std::string> to better mark when no suggestion could be found (and you can return return llvm::None to return an empty llvm::Optional value to indicate no return value)

[gedatsu217]

lldb/source/Interpreter/CommandInterpreter.cpp

1881

Does this mean that this function should return llvm::Optional<std::string> instead of void? I do not know what the intent is.
I personally think either way makes sense.

[gedatsu217]

I corrected codes according to your advice.

[gedatsu217]

lldb/source/Host/common/Editline.cpp

1064

yes. I corrected it because empty() is easir to understand.

1250

I set it at random. I do not know fish is using ^f.
I corrected.

[teemperor]

I think this is ready to get a review from the rest. I'll add the other LLDB folks to the review.

lldb/include/lldb/Host/Editline.h

375

m_current_autosuggestion (completions are the thing we do when the user presses tab).

lldb/include/lldb/Interpreter/CommandInterpreter.h

354

Some documentation maybe:

/// Returns the auto-suggestion string that should be added to the given command line.

lldb/source/Core/CoreProperties.td

136

This should be off by default (until the feature is mature somewhen in the future enough to be on by default)/

137

Usually these settings start like If true, LLDB will show suggestions on possible commands the user might want to type. or something like that.

lldb/source/Core/Debugger.cpp

349

You declared the function, but you don't use it anywhere. You should move all the code you added behind if (GetShowAutosuggestion()) so that it is only active if the user activated the setting (by doing settings set show-autosuggestion true).

lldb/source/Host/common/Editline.cpp

1278

.-/()[]{};\"'\\!@#$%^&*_ are missing here. Maybe we should just iterate over all ASCII characters and add all printables except newline or something like that to the alphabet.

lldb/source/Interpreter/CommandInterpreter.cpp

1881

It just makes the API hard to use incorrectly.

E.g., without knowing how HandleSuggestion is implemented, which of the following calls is correct or incorrect?

std::string line = GetLine();
std::string result;
interpreter->HandleSuggestion(line, result); // is this correct?
interpreter->HandleSuggestion(result, line); // or is this correct?

Versus:

std::string line = GetLine();
std::string result = interpreter->HandleSuggestion(line); // can't call this function in the wrong way

The llvm::Optional would make it more obvious what the function returns in case there is no suggestion.

I think a name like "GetAutoSuggestionForCommand" or something like that would make it more obvious what this is doing.

1887

auto -> llvm::StringRef (it's short enough of a name).

[labath]

lldb/source/Host/common/Editline.cpp

1055–1056

the second line is redundant. std::string typed(1, ch) is enough. (Though I might also consider const char typed[] = {ch, 0};)

1072–1073

maybe fputs(typed.c_str(), m_output_file)

1074

Is the flush still necessary after the switch to m_output_file?

[gedatsu217]

lldb/source/Core/Debugger.cpp

349

Sorry, would you tell me more about it?
I understood that I did not use this function, but I do not know how to validate it.

[gedatsu217]

lldb/source/Host/common/Editline.cpp

1278

If I add -, \, and ^, an error occurs because these characters are probably used in other forms like -a (ll. 1283), \n (ll.1253), and ^p (ll.1257). Do you know good ways to avoid it?

[gedatsu217]

I updated my codes according to your advice:

Add if(GetUseAutosuggestion()) in IOHandlerSuggestion in IOHandler.cpp. (Is this not enough?)
Add more keybinds for TypedCharacter.
Return llvm::Optional<std::string> in HandleSuggestion in CommandInterpreter.cpp.

and several minor change...

[teemperor]

lldb/source/Core/Debugger.cpp

349

This function just returns the current setting that the user has set for the show-autosuggestion setting. So, you want to take that variable and pass it to the Editline code and then only activate all the autosuggestion code when this setting is on (the bool you get here is true if the setting is active).

The IOHandlerEditline takes a debugger variable in the constructor. There you can do debugger.GetUseAutosuggestion() to get the bool if we should use autosuggestions. You need to pass that to the Editline constructor and then put all the code in Editline behind some if (m_use_autosuggestions) or something like that.

You can test it by just doing settings set show-autosuggestion true (which should activate this) and settings set show-autosuggestion true (which should deactivate this). It's fine if LLDB requires a restart to do activate this setting (that's to my knowledge a limitation of the current way the IOHandlers work).

lldb/source/Host/common/Editline.cpp

1278

Actually that's a good point. This feature should never be active in the multiline editor (that's used for LLDB's multiline editor, e.g. when you just type expr and then press enter). So in that case we should never show autosuggestions, so you can just disable all of this if multiline is true. But this also means that the error is not coming from those characters (I guess you tested it in the normal LLDB console and not when you are in the multiline editor).

I think the actual problem is that some of the characters in that string need to be escape (such as -). Not sure how to escape them though, but I assume a backslash or so should do the trick? I would just try adding them one by one and see which one actually breaks editline (and then try adding it with a \\ before).

[JDevlieghere]

I tried out the patch and I have a few observations:

• For me the faint modifier doesn't do anything. Of course it might just be my shell/theme, but we should check whether this modifier is widely supported. Additionally, this modified should be configurable.
• There's no way to confirm the autosuggestion. In fish you can use right-arrow or a key to move to the end of the line (END, CTRL-E).
• The interaction between the autocompletion and autosuggestion is rather confusing. Fish solves this by showing the autocompletion under the current line. I don't we need to take it that far, but at least the the situation described below should work.

Text between square brackets is the autosuggestion:

(lldb) set[tings set show-autosuggestion true] # Press TAB
(lldb) settings

I would expect the suggestion to still show up after.

From a performance perspective, would it be possible to not register the callback at all when the functionality is disabled? I don't actually know if it's worth it, but I imagine that having a callback on every keystroke could be costly. The trade-off is dealing with the user changing the setting from the existing IOHandler, similar to we have to broadcast an event when the user changes the prompt. I'm not saying that's what we should do, just putting the idea out there.

lldb/source/Core/IOHandler.cpp

445

static_cast<IOHandlerEditline *>(baton)

lldb/source/Host/common/Editline.cpp

1275

You can just use a lexical block:

{ 
  ...
}

[gedatsu217]

Implementation all ascii characters for TypedCharacter.
Making m_use_autosuggestion in Editline.cpp. (I did not know a way to pass the bool value of IOHandlerEditline to Editline constructor, so I made a function, Editline::UseAutosuggestion, in Editline class. If the autosuggestion is valid, this function is called from IOHandlerEditline constructor. Is this implementation good?)

You can test it by just doing settings set show-autosuggestion true (which should activate this) and settings set show-autosuggestion true (which should deactivate this). It's fine if LLDB requires a restart to do activate this setting (that's to my knowledge a limitation of the current way the IOHandlers work).

"restart" means that excuting "quit" in LLDB and executing "bin/lldb" again in Terminal? I made m_use_autosuggestion in Editline.cpp and it goes well when show-autosuggestion is DefaultTrue. However, when show-autosuggestion is DefaultFalse and executing "settings set show-autosuggestion true", autosuggestion does not work well. Do you know what causes it?

In addition,

There's no way to confirm the autosuggestion. In fish you can use right-arrow or a key to move to the end of the line (END, CTRL-E).

If you press CTRL-F, you probably confirm the autosuggestion.

The interaction between the autocompletion and autosuggestion is rather confusing. Fish solves this by showing the autocompletion under the current line. I don't we need to take it that far, but at least the the situation described below should work.

I added some codes in TabCommand in Editline.cpp so that autosuggestion still show up after the completion when there is a possible completion.

[teemperor]

This is already looking quite reasonable, I think we're getting closer to something we can merge.

In D81001#2096702, @gedatsu217 wrote:

Implementation all ascii characters for TypedCharacter.
Making m_use_autosuggestion in Editline.cpp. (I did not know a way to pass the bool value of IOHandlerEditline to Editline constructor, so I made a function, Editline::UseAutosuggestion, in Editline class. If the autosuggestion is valid, this function is called from IOHandlerEditline constructor. Is this implementation good?)

You could just have just passed it to the constructor call a few lines above your change, but this seems fine (and maybe even better as the constructor already has a lot of arguments).

You can test it by just doing settings set show-autosuggestion true (which should activate this) and settings set show-autosuggestion true (which should deactivate this). It's fine if LLDB requires a restart to do activate this setting (that's to my knowledge a limitation of the current way the IOHandlers work).

"restart" means that excuting "quit" in LLDB and executing "bin/lldb" again in Terminal? I made m_use_autosuggestion in Editline.cpp and it goes well when show-autosuggestion is DefaultTrue. However, when show-autosuggestion is DefaultFalse and executing "settings set show-autosuggestion true", autosuggestion does not work well. Do you know what causes it?

The Editline instance isn't recreated when you change the settings, so you need to restart LLDB for this to work. However, settings aren't automatically saved, so I think the right way to do this by putting the settings set show-autosuggestion true into the ~/.lldbinit file (which will be automatically executed before LLDB's command line frontend starts). It probably requires a bunch of work to get that working without a restart, so I think this can be fixed as a follow-up.

On a more general note: I'm not sure why we need m_current_autosuggestion. There is a bunch of code that tries to keep that variable up-to-date with what is typed, but unless I'm missing something this is just the text the user has entered so far? If yes, then you can also just get the current user input from Editline (see the first few lines of the Editline::TabCommand function for how to do that).

lldb/source/Core/IOHandler.cpp

279

If this was a normal setter you could just do m_editline_up->SetShowAutosuggestion(debugger.GetShowAutosuggestions());.

lldb/source/Host/common/Editline.cpp

1010

If you make it size_t then you also don't need all the casting below.

1455

This shouldn't return a bool and have a normal setter name (SetShowAutosuggestion). Also the better name for this setting should be ShowAutosuggestion and not UseAutosuggestion (the same goes for all other variables/functions in the rest of this patch).

[gedatsu217]

Change the name and return of the function( bool UseAutosuggestion() -> void SetShowAutosuggestion (bool) ) (ll. 1447 in Editline.cpp and ll.194 in Editline.h).
int -> size_t (ll. 1009 in Editline.cpp).
Fix for normal setter (ll. 269 in IOHandler.cpp).

The Editline instance isn't recreated when you change the settings, so you need to restart LLDB for this to work. However, settings aren't automatically saved, so I think the right way to do this by putting the settings set show-autosuggestion true into the ~/.lldbinit file (which will be automatically executed before LLDB's command line frontend starts). It probably requires a bunch of work to get that working without a restart, so I think this can be fixed as a follow-up.

There is no ~/.lldbinit in my environment (I do not why). Instead, there is ~/.lldb directory (but there are just command history file there.).

On a more general note: I'm not sure why we need m_current_autosuggestion. There is a bunch of code that tries to keep that variable up-to-date with what is typed, but unless I'm missing something this is just the text the user has entered so far? If yes, then you can also just get the current user input from Editline (see the first few lines of the Editline::TabCommand function for how to do that).

I think "m_current_autosuggestion" is needed. That is because it keeps the characters for the suggestion, not just user input. For example, when "b" is typed, "reakpoint" is saved in m_current_autosuggestion (when "breakpoint" is typed before).

When a character is typed, Editline::TypedCharacter is called and m_current_autosuggestion is renewed every time. On the other hand, Editline::ApplyCompleteCommand, which execute suggestion actually, is not called unless C^f is typed. Therefore I think the suggestion parts should be saved until it is called. Indeed, I can get current user input by the first few lines of the Editline::TabCommand function, but it cannot save the suggestion parts probably.

However, I noticed that "to_add" in Editline::TypedCharacter is unnecessary, so removeed it.

[labath]

lldb/source/Interpreter/CommandInterpreter.cpp

1887–1890

if (entry.consume_front(line)) {
  result = entry.str();
  return result; // Why is this returning the result both as a proper return value and a by-ref argument?
}

[gedatsu217]

lldb/source/Interpreter/CommandInterpreter.cpp

1887–1890

That is because using the llvm::Optional as a return value instead of void would make it more obvious what the function returns in case there is no suggestion.

We've discussed this issue before, so please see the comments above.

[teemperor]

In D81001#2107102, @gedatsu217 wrote:

Change the name and return of the function( bool UseAutosuggestion() -> void SetShowAutosuggestion (bool) ) (ll. 1447 in Editline.cpp and ll.194 in Editline.h).
int -> size_t (ll. 1009 in Editline.cpp).
Fix for normal setter (ll. 269 in IOHandler.cpp).

The Editline instance isn't recreated when you change the settings, so you need to restart LLDB for this to work. However, settings aren't automatically saved, so I think the right way to do this by putting the settings set show-autosuggestion true into the ~/.lldbinit file (which will be automatically executed before LLDB's command line frontend starts). It probably requires a bunch of work to get that working without a restart, so I think this can be fixed as a follow-up.

There is no ~/.lldbinit in my environment (I do not why). Instead, there is ~/.lldb directory (but there are just command history file there.).

You need to create that file :) It's just a text file with LLDB commands in each line (and you just put the settings command in there to change the setting before LLDB starts). Doing lldb -o "settings set show-autosuggestion true" should also work.

On a more general note: I'm not sure why we need m_current_autosuggestion. There is a bunch of code that tries to keep that variable up-to-date with what is typed, but unless I'm missing something this is just the text the user has entered so far? If yes, then you can also just get the current user input from Editline (see the first few lines of the Editline::TabCommand function for how to do that).

I think "m_current_autosuggestion" is needed. That is because it keeps the characters for the suggestion, not just user input. For example, when "b" is typed, "reakpoint" is saved in m_current_autosuggestion (when "breakpoint" is typed before).

When a character is typed, Editline::TypedCharacter is called and m_current_autosuggestion is renewed every time. On the other hand, Editline::ApplyCompleteCommand, which execute suggestion actually, is not called unless C^f is typed. Therefore I think the suggestion parts should be saved until it is called. Indeed, I can get current user input by the first few lines of the Editline::TabCommand function, but it cannot save the suggestion parts probably.

However, I noticed that "to_add" in Editline::TypedCharacter is unnecessary, so removeed it.

My worry is that if we rely on always keeping this variable up-to-date during the life-time of Editline that this might lead to weird bugs due to the variable getting out-of-sync (e.g., someone changes some input logic but forgets to update the autosuggestion code). I tried moving some parts in this patch around in this commit and it does seem to work in the same way as this patch, but without the need to keep m_current_autosuggestion up-to-date. What do you think?

lldb/source/Host/common/Editline.cpp

1459

This can all be m_use_autosuggestion = autosuggestion;.

lldb/source/Interpreter/CommandInterpreter.cpp

1887–1890

Pavel's point is that the function is returning >both<. You can just remove the result parameter and do result = ... GetAutoSuggestionForCommand(line); when you call it is his point.

[labath]

lldb/source/Interpreter/CommandInterpreter.cpp

1887–1890

Yes that was one of my points. Thank you, Raphael.

My other point was that using consume_front is better than a startswith+substr combo.

[gedatsu217]

I understood what you said. Sorry, I misunderstood it. Indeed, m_current_autosuggestion is not good for the future.
I will get the current user input every time calling ApplyCompleteCommand instead of using m_current_autosuggestion.

By the way, when I use llvm::Optional<std::string> as a return, how do I receive it? I want to receive the result in IOHandlerDelegate::IOHandlerSuggestion. If I write "result = ...GetAutoSuggestionForCommand(line)", an error, "no viable overloaded '='", occurs.

[gedatsu217]

I revised the code according to your advice.

In addition, llvm::Optional<std::string> was originally returned in CommandInterpreter::GetAutoSuggestionForCommand, but it seems to useless that converting llvm::Optional to std::string in the function, so I changed return value.

By the way, when I use llvm::Optional<std::string> as a return, how do I receive it? I want to receive the result in IOHandlerDelegate::IOHandlerSuggestion. If I write "result = ...GetAutoSuggestionForCommand(line)", an error, "no viable overloaded '='", occurs.

This problem was solved. (I forgot about llvm::None, sorry.)

[teemperor]

Just a quick comment while I look into how we can test this code.

lldb/source/Host/common/Editline.cpp

1015

If I understand the only effect this whole code has is to return CC_REFRESH instead of CC_REDISPLAY? If yes, then I think you can just replace the break below with return CC_REFRESH; which will be much simpler.

[gedatsu217]

lldb/source/Host/common/Editline.cpp

1015

If yes, then I think you can just replace the break below with return CC_REFRESH; which will be much simpler.

Isn't it "return CC_REDISPLAY", not "CC_REFRESH"? I want to return CC_REFRESH only when "to_add" is in "to_add_autosuggestion" (e.g. to_add = b, to_add_autosuggestion = "breakpoint").

That is because CC_REDISPLAY deletes the gray-colored autosuggested part, while CC_REFRESH leaves it.

[JDevlieghere]

lldb/include/lldb/Host/Editline.h

321

It's not really clear to me what this method does based on the name or the description above. It seems similar to TabCommand, so maybe AutosuggestCommand would be a better name and more consistent?

375–377

Strings are empty by default, so you can omit the assignment: = "".

376

Why do we need this? Can we check that the m_suggestion_callback is null instead?

lldb/include/lldb/Interpreter/CommandInterpreter.h

356

Although I believe the current implementation is correct (because the returned string is backed by m_command_history) I think this interface looks rather suspicious by returning a StringRef. Do we envision this returning new string in the future? If so we might consider having it return a llvm::Optional<std::string>.

lldb/source/Core/CoreProperties.td

137

If true, LLDB will show suggestions to complete the command the user typed.

lldb/source/Core/IOHandler.cpp

269

Why should we set the callback if the Autosuggestions are off? Why not do:

if(debugger.GetUseAutosuggestion()))
  m_editline_up->SetSuggestionCallback(SuggestionCallback, this);

Both have the same downside we already discussed, that the setting cannot be changed for the current EditLine instance.

[gedatsu217]

I fixed the code according to your advice.

[teemperor]

Sorry this is taking so long, but I think beside one minor change this only needs some testing and then it's ready.

So the way I would like to see this tested would be with a pexpect test. Pexpect just launches a virtual terminal where LLDB runs inside and you can send input and read output. There is a good example test here that you can copy lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py. I think it's a good enough test if you just type any lldb command like help frame in the test, press enter and then check that if you type like hel you get the autosuggestion displayed in the terminal output. And then you press Ctrl+F in the terminal (you have to find the escape sequence for that when sending it to pexpect) and should see the complete autosuggestion.

Be aware that there is chance that the Pexpect terminal behaves differently than a real terminal and you see wrong results there. In that case we need to come up with another solution, but let's first try pexpect. Also you should know that pexpect tests don't know if LLDB is actually still producing output or idling, so if get timeouts while reading the output that just means we didn't see the right output.

lldb/source/Host/common/Editline.cpp

1008

This will crash with disabled suggestions (m_suggestion_callback can be null if the feature is disabled).

1015

I see. What about just retuning always CC_REFRESH here? That should work as we only add text to the end with a normal completion (which is IIRC that's what CC_REFRESH is for, but

case CompletionMode::Normal: {
  std::string to_add = completion.GetCompletion();
  to_add = to_add.substr(request.GetCursorArgumentPrefix().size());
  std::string to_add_autosuggestion = "";
  to_add.push_back(' ');
  el_insertstr(m_editline, to_add.c_str());
  return CC_REFRESH;
}

That seems to work for me (and it avoids the crash I pointed out above).

Also my main point here is that this is quite a large change just to change the return value (and the other tab completions below aren't covered and would need similar changes if we do this change).

[gedatsu217]

lldb/source/Host/common/Editline.cpp

1015

Where is "to_add_autosuggestion" used in the above example?

[teemperor]

lldb/source/Host/common/Editline.cpp

1015

Yeah, you could also remove that:

case CompletionMode::Normal: {
  std::string to_add = completion.GetCompletion();
  to_add = to_add.substr(request.GetCursorArgumentPrefix().size());
  to_add.push_back(' ');
  el_insertstr(m_editline, to_add.c_str());
  return CC_REFRESH;
}

(So, changing break; -> return CC_REFRESH; seems to be enough in this situation). CC_REFRESH should be fine for both normal completion and when we add the autosuggestion string afterwards if I understand the meaning of it correctly.

[gedatsu217]

lldb/source/Host/common/Editline.cpp

1015

Now I get it. Thanks.

[gedatsu217]

Remove extra parts in Editline::TabCommand in Editline.cpp.

Add a test for the autosuggestion.

[teemperor]

This now looks good to me. I'll give the others a chance to take a look too but beside that I think this is ready to go IMHO.

[labath]

The test looks wrong to me. If the feature is supposed to be off by default, then how can it test anything without first turning the feature on? Either the feature isn't really off, or the test is broken. I would expect it to be the latter, because the test sequence seems incorrect to me. I would expect to see some newlines in the strings being sent, otherwise all of this will just get concatenated into a single command.

Also, this only seems to test the ^F functionality. It does not check that the command is printed in gray _before_ the user hits ^F.

lldb/source/Core/IOHandler.cpp

204

Why does this switch Optional<string> result to a by-ref string argument. We have both code which uses an empty string to signify failure, and code which does that with None. Both have their advantages and disadvantages, and I don't have any strong objections to either one, but I certainly don't see an advantage in using both in the same patch.

lldb/source/Host/common/Editline.cpp

1226

These don't have anything to do with indenting. Looks like a leftover from the copy paste from line 1255

[teemperor]

The test looks wrong to me. If the feature is supposed to be off by default, then how can it test anything without first turning the feature on? Either the feature isn't really off, or the test is broken. I would expect it to be the latter, because the test sequence seems incorrect to me. I would expect to see some newlines in the strings being sent, otherwise all of this will just get concatenated into a single command.
Also, this only seems to test the ^F functionality. It does not check that the command is printed in gray _before_ the user hits ^F.

Yeah, and the test obviously doesn't pass.

Alright, let's try to fix this.

@gedatsu217 Not sure if you uploaded the wrong diff, but that test doesn't pass. It would be a nice test if if worked though. But as we anyway need another iteration, let's also extend the test while we're at it.

First, you need to get the test to actually enable autosuggestions, so you need to launch via self.launch(extra_args=["-o", "settings set show-autosuggestion true"]) (the extra_args are just passed to LLDB's command line).

Also as Pavel said, we should also test that we display the autosuggestion in faint colour. Not sure if there is a good way to test that the text is directly behind the cursor, but we can test that it's there. And I guess we should test that we actually pick the most recent command for the autosuggestion (this is currently wrong and we pick the least recent command).

I extend your test a bit to something like this (which is currently failing as it always completes help frame but not the help apropos).

self.launch(extra_args=["-o", "settings set show-autosuggestion true"]) 
# Common input codes and escape sequences.                                                                        
ctrl_f = "\x06"                                                         
faint_color = "\x1b[2m"                                                 
reset = "\x1b[0m"                                                       
                                                                        
frame_output_needle = "Syntax: frame <subcommand>"                      
# Run 'help frame' once to put it into the command history.             
self.expect("help frame", substrs=[frame_output_needle])                
                                                                        
# Check that LLDB shows the autosuggestion in gray behind the text.     
self.child.send("hel")                                                  
self.child.expect_exact(faint_color + "p frame" + reset)                
                                                                        
# Apply the autosuggestion and press enter. This should print the       
# the 'help frame' output if everything went correctly.                 
self.child.send(ctrl_f + "\n")                                          
self.child.expect_exact(frame_output_needle)                            
                                                                        
# Try autosuggestion using tab and ^f.                                  
# \t makes "help" and ^f makes "help frame". If everything went         
# correct we should see the 'help frame' output again.                  
self.child.send("hel\t" + ctrl_f + "\n")                                
self.child.expect_exact(frame_output_needle)                            
                                                                        
# Try another command.                                                  
apropos_output_needle = "Syntax: apropos <search-word>"                 
# Run 'help frame' once to put it into the command history.             
self.expect("help apropos", substrs=[apropos_output_needle])            
                                                                        
# 'hel' should have an autosuggestion for 'help apropos' now.           
self.child.send("hel")                                                  
# FIXME: This is showing 'frame' instead.                               
self.child.expect_exact(faint_color + "p apropos" + reset)                
                                                                        
# Run the command and expect the 'help apropos' output.                 
self.child.send(ctrl_f + "\n")                                          
self.child.expect_exact(apropos_output_needle)

A few more test cases that come to my mind and you should add:

• help help frame should not have an autosuggestion to help frame. You can just try to get the autosuggestion for help help frame and check for the error for an invalid command.
• Pressing Ctrl+F in an empty prompt should do nothing (you can just check for the (lldb) prompt I would say).
• Pressing Ctrl+F directly after Ctrl+F again should do nothing.
• Having help frame and help frame var in the command history, help fr should still complete to the most recent one.
• Entering a1234, then 5 backspaces, then hel and then Ctrl+F should bring up the autosuggestion for help frame.
• Entering help x, then 1 backspace, then Ctrl+F should bring up the autosuggestion for help frame.

[gedatsu217]

help help frame should not have an autosuggestion to help frame. You can just try to get the autosuggestion for help help frame and check for the error for an invalid command.

Sorry, I do not know what this means. Now, if I execute "help help frame" once, it is suggested. Is this wrong?

lldb/source/Core/IOHandler.cpp

204

Sorry, I do not what you mean. Would you explain it more in detail?

(If None is returned, it is not assigned to "result". So, only is string assigned to "result". Does this answer your question?)

[labath]

In D81001#2158645, @teemperor wrote:

Not sure if there is a good way to test that the text is directly behind the cursor, but we can test that it's there.

I guess that would be done by expecting the appropriate cursor movement command.

self.child.expect_exact(faint_color + "p frame" + reset)

+ "^[[move-cursor-left"+strlen("p frame") (I didn't look up the exact encoding of that)

lldb/source/Core/IOHandler.cpp

204

Ok, let me try to phrase that differently:

• if GetAutoSuggestionForCommand fails, it returns None
• if IOHandlerDelegate::IOHandlerSuggestion fails, it sets a by-ref string argument to "" (or rather, leaves it as empty).

The two behaviors are different, and it's not clear to me what justifies that difference. It would be much cleaner and clearer if both were using the same convention. Changing conventions part way like this is confusing and forces one to add conversion code. If IOHandlerSuggestion was using the same convention, then it's implementation would just be return io_handler.GetDebugger().GetCommandInterpreter().GetAutoSuggestionForCommand(line)).

[gedatsu217]

lldb/source/Core/IOHandler.cpp

204

I understood it.
Even if I change return of IOHandlerSuggestion from void to llvm::Optional<std::string>, I think I have to convert None to empty string in another function eventually, because the final perpose of these function is to make string for autosuggestion.

Therefore, if I change return of CommandInterperter::GetAutoSuggestionForCommand from llvm::Optional<std::string> to just std::string, I think above problem will be solved. What do you think?

[gedatsu217]

In addition to it, I tried the below code, but it did not go well. ("\x1b[nD" moves the cursor n steps to the left.)

self.child.send("hel")
self.child.expect_exact(faint_color + "p frame" + reset + "\x1b[" + str(len("p frame")) + "D")

In the first place, the below code does not go well.

self.child.send("help frame")
self.child.expect_exact("help frame" + "\x1b[0D")

"\x1b[0D" means that the cursor does not move. So, I suspect "\x1b[nD" does not function in pexpect. What do you think?

[labath]

In D81001#2162743, @gedatsu217 wrote:

In addition to it, I tried the below code, but it did not go well. ("\x1b[nD" moves the cursor n steps to the left.)

self.child.send("hel")
self.child.expect_exact(faint_color + "p frame" + reset + "\x1b[" + str(len("p frame")) + "D")

In the first place, the below code does not go well.

self.child.send("help frame")
self.child.expect_exact("help frame" + "\x1b[0D")

"\x1b[0D" means that the cursor does not move. So, I suspect "\x1b[nD" does not function in pexpect. What do you think?

It definitely "functions", but it's possible that lldb/editline output a slightly different sequence with the same effect. For example it might output "\x1b[mG", which would move the cursor to the absolute column m. Other ways of achieving the same thing are possible, but they are more convoluted so I don't expect we are using them. Bottom line: You'll need to check what's the sequence that actually gets output.

lldb/source/Core/IOHandler.cpp

204

Are you sure about that? If I follow the code correctly this eventually gets called in Editline::ApplyAutosuggestCommand

std::string to_add = "";
m_suggestion_callback(line, to_add, m_suggestion_callback_baton);

if (to_add.empty())
  return CC_REFRESH;

el_insertstr(m_editline, to_add.c_str());
return CC_REDISPLAY;

That could be written as:

if (Optional<string> to_add = m_suggestion_callback(line, to_add, m_suggestion_callback_baton)) {
  el_insertstr(m_editline, to_add->c_str());
  return CC_REDISPLAY;
}
return CC_REFRESH;

which is actually shorter and makes the failure case more explicit.

The call in Editline::TypedCharacter could be modified similarly...

[gedatsu217]

Add the test content.

Change return from void to llvm::Optional<std::string> in several functions to make the code cleaner.

Revise the details.

[labath]

Some quibbles about the test. Otherwise, this looks good to me.

lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py

34

[1G means move the cursor to the first column, which is not what we have wanted to test here. My guess is that this 1G is the start of the sequence that libedit uses to redraw the prompt (or the entire command line). If that's the case (and we are serious about testing the cursor position), then I guess you'll need to expect that entire sequence. Or possibly, split this into two expect_exact calls so that the second call just checks for the presence of the final sequence which actually puts the cursor in the right place.

53

This won't check anything because an empty string is "everywhere". I thing the best we can do right now is type some more text after the ^F, and then check that _that_ text is executed properly (i.e., that it didn't get appended to some other command).

[gedatsu217]

I checked what's the sequence that actually gets output, and it was like below.

h\x1b[2melp frame\x1b[0m\x1b[1Ghe\x1b[2mlp frame\x1b[0m\x1b[1Gel\x1b[2mp frame\x1b[0m\x1b[1Gl

Is it not good to check these characters in expect_exact?

[labath]

In D81001#2170570, @gedatsu217 wrote:

I checked what's the sequence that actually gets output, and it was like below.

h\x1b[2melp frame\x1b[0m\x1b[1Ghe\x1b[2mlp frame\x1b[0m\x1b[1Gel\x1b[2mp frame\x1b[0m\x1b[1Gl

Is it not good to check these characters in expect_exact?

The problem with that is that it might make the test too strict and susceptible to changes in libedit behavior (either due to different versions or different timings). There are many ways to achieve the same thing after all.
So, I would stick to the tail of the sequence which is printed after l is "pressed".

That said, are you sure this is the right sequence? \x1b[1Gl seems like it should print the l at column one, which does not sound right...

The sequence I got in this test was l\x1b[2mp frame\x1b[0m\x1b[1G\r\x1b[9Cl, which seems more believable. If I am decoding that correctly, it does:

• l - l is "pressed" and printed
• \x1b[2mp frame\x1b[0m - lldb prints "p frame" in faint
• \x1b[1G - libedit moves cursor to column 1
• \r - moves cursor to column 1 again (not sure why, probably to work around some terminal bugs)
• \x1b[9C - moves cursor 9 characters forward (strlen("(lldb) he"))
• l - redraws l

If that sequence works for you, then this is what I'd suggest putting in the test.

[gedatsu217]

That said, are you sure this is the right sequence? \x1b[1Gl seems like it should print the l at column one, which does not sound right...

I thought it did not add up too, but pexpect probably actually outputs these characters.

The sequence I got in this test was l\x1b[2mp frame\x1b[0m\x1b[1G\r\x1b[9Cl, which seems more believable.

It did not go well... However, "l" + faint_color + "p frame" + reset + "\x1b[1G" + "l" passed the test.
This test ensure that the cursor is behind "l", but I do not understand what role "\x1b[1G" plays...

What do you think about this test?

[labath]

In D81001#2171999, @gedatsu217 wrote:

That said, are you sure this is the right sequence? \x1b[1Gl seems like it should print the l at column one, which does not sound right...

I thought it did not add up too, but pexpect probably actually outputs these characters.

The sequence I got in this test was l\x1b[2mp frame\x1b[0m\x1b[1G\r\x1b[9Cl, which seems more believable.

It did not go well... However, "l" + faint_color + "p frame" + reset + "\x1b[1G" + "l" passed the test.
This test ensure that the cursor is behind "l", but I do not understand what role "\x1b[1G" plays...

What do you think about this test?

I think it won't pass on my machine because that's not the sequence I am getting. :(

I can see how different libedit versions could produce different sequences, but I do not understand how can the one you're seeing be "right".

Are you testing this on a mac? Lemme try what I get there...

[gedatsu217]

Yes, I'm testing it on Mac.

By the way, I checked its output on the error message. When the test failed, the characters, buffer (last 100 chars) : ~~~, are displayed as an error message. I thought it was pexpect's output, possibly it is not that output?

[labath]

In D81001#2172035, @labath wrote:

In D81001#2171999, @gedatsu217 wrote:

That said, are you sure this is the right sequence? \x1b[1Gl seems like it should print the l at column one, which does not sound right...

I thought it did not add up too, but pexpect probably actually outputs these characters.

The sequence I got in this test was l\x1b[2mp frame\x1b[0m\x1b[1G\r\x1b[9Cl, which seems more believable.

It did not go well... However, "l" + faint_color + "p frame" + reset + "\x1b[1G" + "l" passed the test.
This test ensure that the cursor is behind "l", but I do not understand what role "\x1b[1G" plays...

What do you think about this test?

I think it won't pass on my machine because that's not the sequence I am getting. :(

I can see how different libedit versions could produce different sequences, but I do not understand how can the one you're seeing be "right".

Are you testing this on a mac? Lemme try what I get there...

Ok, so I was able to confirm what you are seeing on a mac. I don't fully understand what is happening but the problem is related to the use-color setting. Our pexpect harness starts lldb with colors disabled, and this seems to be causing lldb (or libedit) to go berserk. Indeed if you start lldb without colors manually (lldb --no-use-color), you will see that this feature (on a mac) misbehaves -- the cursor gets moved to the first column just like we expected based on the ansi sequences. On linux everything seems to be working fine even without colors.

Now the question is what to do about it? Did you actually intend for your feature to work with colors disabled? If yes, then we'll need to figure out why things don't work on a mac. If no, then we'll need to customize our test harness to enable launching lldb with colors for the autosuggestion tests. And then we'll need to find the right sequence to expect...

[gedatsu217]

I do not intend for this feature to work with colors disabled.

I found that pexpect output the below sequence, and this passed the test.

self.child.expect_exact("\x1b[" + str(len("(lldb) he") + 1) + "G" + "l" + "\x1b[2m" + "p frame" + "\x1b[0m\x1b[1G" + "l" + "\x1b[1G\x1b[2m" + "(lldb) " + "\x1b[22m\x1b[" + str(len("(lldb) hel") + 1) + "G")

Probably, "(lldb)" is redisplayed every time a character is typed. On the other hand, the character is placed in the designated position.

However, there are two strange points.

1. When a character is typed, it is placed in the designated position, but later, it is placed again in column one and overwritten by "(lldb)".
2. About "\x1b[22m". I think this is equal to "\x1b[0m", but the test failed when I replace "\x1b[22m" with "\x1b[0m".

Do you think this is a valid test?

[labath]

In D81001#2172658, @gedatsu217 wrote:

I do not intend for this feature to work with colors disabled.

Right in that case, we should probably disable the feature if colors are disabled.

I found that pexpect output the below sequence, and this passed the test.

self.child.expect_exact("\x1b[" + str(len("(lldb) he") + 1) + "G" + "l" + "\x1b[2m" + "p frame" + "\x1b[0m\x1b[1G" + "l" + "\x1b[1G\x1b[2m" + "(lldb) " + "\x1b[22m\x1b[" + str(len("(lldb) hel") + 1) + "G")

Probably, "(lldb)" is redisplayed every time a character is typed. On the other hand, the character is placed in the designated position.

However, there are two strange points.

1. When a character is typed, it is placed in the designated position, but later, it is placed again in column one and overwritten by "(lldb)".

Yes, that is true. This is a problem and the prompt drawing is only covering it up.

That said, now that I understand this code better, I believe this is actually a bug on our part that we can fix. In your TypedCharacter function you call MoveCursor(CursorLocation::BlockEnd, CursorLocation::EditingPrompt); after printing the suggested text. This is the thing which moves the cursor to the beginning of the line, and judging by the editline behavior (printing the character at the start of line), this is not what it expects. It seems like the right solution here would be to move the cursor to be just before the character that was added. CursorLocation::EditingCursor moves it just *after* it, so it seems we may need a new constant (BeforeEditingCursor) to perform the desired move.

Alternatively, it may also be possible to move to CursorLocation::EditingCursor with a combination of return CC_NORM. That seemed to work fine in my experiments except for introducing some artefacts when backspacing. It's possible this is another bug that could also be fixed, but I did not look into it.

2. About "\x1b[22m". I think this is equal to "\x1b[0m", but the test failed when I replace "\x1b[22m" with "\x1b[0m".

The test checks for string equivalence, not functional equivalence. That string is coming from Editline::GetCharacter (ANSI_UNFAINT). That said, since both of these functions are doing the same thing (restoring normal formatting after displaying some custom stuff) it would make sense for them to do it the same way. 0m seems to be more explicit so maybe we should standardize on that? Could you create a patch to change the definition of ANSI_UNFAINT ? (might be worth taking a quick look at git history if there is no good reason for why it uses the color code that it uses)

Also, given the presence of ANSI_(UN)FAINT, I'm not sure what to make of the usage of FormatAnsiTerminalCodes in this patch. This file is already intimately familiar with ansi escape sequences needed move cursors and stuff, so I'm not sure that going through that function actually buys us anything.

Do you think this is a valid test?

With the above caveats, yes. There's also one additional aspect -- even after the above is addressed, we may still need to allow for some variance in the sequence to allow for different libedit versions -- as our test have shown, these can differ sometime.

[gedatsu217]

Could you create a patch to change the definition of ANSI_UNFAINT ? (might be worth taking a quick look at git history if there is no good reason for why it uses the color code that it uses)

Sure. Should I create another patch? (In short, should I create it separately from the current patch?)

[labath]

In D81001#2176076, @gedatsu217 wrote:

Could you create a patch to change the definition of ANSI_UNFAINT ? (might be worth taking a quick look at git history if there is no good reason for why it uses the color code that it uses)

Sure. Should I create another patch? (In short, should I create it separately from the current patch?)

Yes, please.

[gedatsu217]

Revise Editline::ApplyAutosuggestCommand. (Change the return value.)

Revise IOHandlerEditline::IOHandlerEditline (Disable to use autosuggestion if debugger.GetUseColor() is false.)

Fix the test.

[labath]

Thanks.

When trying this out I did notice one subtle bug related to backspacing. When backspacing over the first character, the command line does not get redrawn, which means the autosuggested text remains on screen. The "deleted" character also remains, and is printed in normal colour, though this is very hard to see (on my terminal) as it is now right under the cursor. Pressing backspace the second time makes the faint text and both deleted characters disappear. I'll try to illustrate it using crude ascii art. Initially the command line is (F = faint, ^ = cursor):

(lldb) help frame
FFFFFF    FFFFFFF
          ^

After the first backspace, the line becomes:

(lldb) help frame
FFFFFF    FFFFFFF
         ^

After the second one:

(lldb) h
FFFFFF    
        ^

I didn't find this effect too disturbing, so I don't think it should be a blocker for this, but it something to keep an eye on, as it may cause problems down the road.

Anyway, LGTM from me.

lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py

72

Since you have variables for the faint and reset escape codes, maybe you could have a function for the cursor movement? def cursor_horizontal_abs(n): return "\x1b[" + str(n) + "G" ?

[gedatsu217]

Editline::TypedCharacter is called when delete is pressed. (This probably fix the above bug.)

Revise the test.

[gedatsu217]

Sorry, I found the bug. Please do not check this yet.

[gedatsu217]

For example, I execute "help frame variable" and save it as command history. Then, when I type "hel", "helhelp [me variable]" (gray characters are in []) is displayed, probably because of cursor position or CC_NORM.

I am trying to solve this problem, but I have not come up with a solution. If you know how to solve this problem, would you help me?

[labath]

In D81001#2179690, @gedatsu217 wrote:

For example, I execute "help frame variable" and save it as command history. Then, when I type "hel", "helhelp [me variable]" (gray characters are in []) is displayed, probably because of cursor position or CC_NORM.

I am trying to solve this problem, but I have not come up with a solution. If you know how to solve this problem, would you help me?

This works for me (linux, libedit version 20191211.3.1). I don't have any immediate suggestions, but I'd start with looking at the escape sequences produced by libedit and lldb when this happens. That might give a clue as to who is getting confused and why. In particular it may be useful to compare the sequences when this does happen vs. when it does not happen (like why didn't this happen when you just type h or he? Or why doesn't this happen with a different command, etc.).

[gedatsu217]

Fix the cursor position. (Editline::TypedCharacter)

[gedatsu217]

Sorry, I mistakenly uploaded a different file. This is the correct file.

[teemperor]

I'm not hitting the same bug here, but I do hit a different bug. If I have a autosuggestion and then type another character that causes a shorter autosuggestion to display, then the old text of the autosuggestion is not deleted from the terminal.

For example:

> ./build/bin/lldb -o "settings set show-autosuggestion true"
(lldb) settings set show-autosuggestion true
(lldb) help frame var
[...]
(lldb) help frame info
[...]
(lldb) help frame v[aro]

• is the grey text.

"help frame " correctly shows "help frame var info" which is the last command, but typing the "v" will cause that only "help frame var" is left as a valid autosuggestion. However the "o" from "info" is not clear, but only the "inf" part ovewritten with "var".

Tbh, I'm not sure what's the best fix for this. I don't think we can clear all following characters in the line with some control character (correct me if I'm wrong). We could adding spaces into the autosuggestion string to fill the rest of the line though (this way the old characters in the buffer are getting ovewritten).

[gedatsu217]

Although I thought of how to clear all the following characters, I did not come up with it. So, I will try to fill the space. Should I add how many spaces? If I add too many spaces, sequences will be two lines, but if I add a few spaces, I can't handle some of the cases.

[teemperor]

In D81001#2188888, @gedatsu217 wrote:

Although I thought of how to clear all the following characters, I did not come up with it. So, I will try to fill the space. Should I add how many spaces? If I add too many spaces, sequences will be two lines, but if I add a few spaces, I can't handle some of the cases.

It should be possible to keep track of how long the previous autosuggestion was and then print the difference between the current autosuggestion length and the previous one in spaces to clear the buffer. So, something like spaces_to_print = line.size() + to_add.size - m_previous_autosuggestion_size (and if that's positive we add that many spaces).

Also could you rebase the diff here when you have time (e.g., regenerate the diff from you changes on top of the latest master commit)? It no longer applies at the moment without having to manually resolve a conflict.

lldb/include/lldb/Host/Editline.h

363

Unrelated removed line.

[gedatsu217]

Also could you rebase the diff here when you have time (e.g., regenerate the diff from you changes on top of the latest master commit)? It no longer applies at the moment without having to manually resolve a conflict.

Does this mean that I should generate diff from the current latest master commit of [https://github.com/llvm/llvm-project]? or from my latest commit (local)? Sorry, I am not used to using git:(

[teemperor]

In D81001#2189008, @gedatsu217 wrote:

Also could you rebase the diff here when you have time (e.g., regenerate the diff from you changes on top of the latest master commit)? It no longer applies at the moment without having to manually resolve a conflict.

Does this mean that I should generate diff from the current latest master commit of [https://github.com/llvm/llvm-project]? or from my latest commit (local)? Sorry, I am not used to using git:(

No worries! From the latest master commit from GitHub.com is what I mean. Someone changed one of the lines your patch changes between your checkout and master and now there is a conflict when applying thes patch. If you git pull on your master branch and then make a new diff, then everyone can apply this patch again without having to manually resolve this conflict over and over again.

[gedatsu217]

Add spaces when a character is typed.

[teemperor]

Added some comments on the new code. Also we should be able to see these spaces in the pexpect output, so I guess we should be able to test this too? To make this special case less fragile to test, you can make a new subtest in the Test case by just defining a new function:

@skipIfAsan
@skipIfEditlineSupportMissing
def test_hidden_autosuggestion(self):
@skipIfAsan
@skipIfEditlineSupportMissing
def test_autosuggestion(self):
    self.launch(extra_args=["-o", "settings set show-autosuggestion true", "-o", "settings set use-color true"])
    self.expect("help frame v")
    self.expect("help frame info")
    [type 'help frame v' and check for the three space after the grey part to cover up the "nfo" text]

So, I *believe* that everything that we want to have in this patch has been addressed? There are quite a few comments in this thread so it's hard to say if there are open issues. I played around with the current patch for a bit and I couldn't find anymore issues, so I think this is in pretty good shape in any case.

Let's see if @labath or @JDevlieghere have any more comments, otherwise I would say this can go in and the rest can be fixed as follow up commits.

lldb/include/lldb/Host/Editline.h

377

This probably should be std::size_t type (a negative previous suggestion size seems weird). Also add a comment that this is the length of the previous autosuggestion + user input in bytes.

lldb/source/Host/common/Editline.cpp

1074

Whoever sees this in the future will really wonder why we print a bunch of spaces here, so I think a comment like "Print spaces to hide any remains of a previous longer autosuggestion".

1510

I don't think the value of m_previous_autosuggestion_size should only grow (which is what this if is doing), as this way we just keep printing a longer and longer space buffer over time. Just printing enough to clear the buffer of the previous completion is enough.

Also you can just do this calculation directly after you calculated int spaces_to_print above. This way this code is closer together which hopefully makes this easier to understand.

[gedatsu217]

@skipIfAsan
@skipIfEditlineSupportMissing
def test_hidden_autosuggestion(self):
@skipIfAsan
@skipIfEditlineSupportMissing
def test_autosuggestion(self):
    self.launch(extra_args=["-o", "settings set show-autosuggestion true", "-o", "settings set use-color true"])
    self.expect("help frame v")
    self.expect("help frame info")
    [type 'help frame v' and check for the three space after the grey part to cover up the "nfo" text]

Sorry, would you tell me about this code in more detail? Does this mean that I should make test_hidden_autosuggestion and test if there are spaces there? What is the difference between test_hidden_autosuggestion and test_autosuggestion?

I don't think the value of m_previous_autosuggestion_size should only grow (which is what this if is doing), as this way we just keep printing a longer and longer space buffer over time. Just printing enough to clear the buffer of the previous completion is enough.

If I keep the number of characters of the only previous command, I think there is a problem. For example, If I type "help frame var" → "help frame info" → "help frame v", the remains are hidden. However, If I type "help frame var" → "help frame info" → "help" → "help frame v", the number of characters of "help frame var" is more than that of "help", so "help frame v[aro]" is displayed. What do you think?

Also you can just do this calculation directly after you calculated int spaces_to_print above. This way this code is closer together which hopefully makes this easier to understand.

For example, if the user executes a command directly after using tab-completion, Editline::TypedCharacter is not called, so spaces_to_print is not calculated. That is because I can calculate this here.

By the way, I found a bug again. The gray characters remain when only one character is completed by tab-completion.
For instance, when I type "b" and press tab-key after I execute "breakpoint", b [eakpoint] is displayed. [eakpoint] should not be displayed. This problem will be probably solved by my previous diff (https://reviews.llvm.org/D81001?id=276468). But this diff changes Editline::TabCommand significantly. Would you tell me a good way if any?

[teemperor]

In D81001#2190646, @gedatsu217 wrote:

@skipIfAsan
@skipIfEditlineSupportMissing
def test_hidden_autosuggestion(self):
@skipIfAsan
@skipIfEditlineSupportMissing
def test_autosuggestion(self):
    self.launch(extra_args=["-o", "settings set show-autosuggestion true", "-o", "settings set use-color true"])
    self.expect("help frame v")
    self.expect("help frame info")
    [type 'help frame v' and check for the three space after the grey part to cover up the "nfo" text]

Sorry, would you tell me about this code in more detail? Does this mean that I should make test_hidden_autosuggestion and test if there are spaces there? What is the difference between test_hidden_autosuggestion and test_autosuggestion?

Every Test*.py file can have multiple test_ methods that are each their own separate test. First the test suite would run test_autosuggestion and then would run test_hidden_autosuggestion, but each is its own isolated test. My point was that the test with the spaces relies on having exactly two commands in that order in the command history to work, so you probably should make a new subtest for this so that the test doesn't break if someone extends the test and runs another command. So, if you *would* add the test for the spaces to the existing test, then if someone would add a command like help frame va to the history it would break.

I don't think the value of m_previous_autosuggestion_size should only grow (which is what this if is doing), as this way we just keep printing a longer and longer space buffer over time. Just printing enough to clear the buffer of the previous completion is enough.

If I keep the number of characters of the only previous command, I think there is a problem. For example, If I type "help frame var" → "help frame info" → "help frame v", the remains are hidden. However, If I type "help frame var" → "help frame info" → "help" → "help frame v", the number of characters of "help frame var" is more than that of "help", so "help frame v[aro]" is displayed. What do you think?

Not sure if I understand your example correctly, but as soon as you type "help frame ", you should have an autosuggestion of "help frame info" and then typing the "v" should clear the "nfo" part. The "help" autosuggestion should not be involved at all in any logic after you typed "help "?

Also you can just do this calculation directly after you calculated int spaces_to_print above. This way this code is closer together which hopefully makes this easier to understand.

For example, if the user executes a command directly after using tab-completion, Editline::TypedCharacter is not called, so spaces_to_print is not calculated. That is because I can calculate this here.

Can you give me an example input where this breaks? I'm not sure how the tab completion would influence until what column we would need to overwrite the line buffer (that should still be the same).

By the way, I found a bug again. The gray characters remain when only one character is completed by tab-completion.
For instance, when I type "b" and press tab-key after I execute "breakpoint", b [eakpoint] is displayed. [eakpoint] should not be displayed. This problem will be probably solved by my previous diff (https://reviews.llvm.org/D81001?id=276468). But this diff changes Editline::TabCommand significantly. Would you tell me a good way if any?

That probably comes from the fact that "b" is a command and we just insert a space when you tab complete it. And it seems the space we insert into the el_insertstr function doesn't seem to overwrite the existing buffer:

switch (completion.GetMode()) {                                             
case CompletionMode::Normal: {                                              
  std::string to_add = completion.GetCompletion();                          
  to_add = to_add.substr(request.GetCursorArgumentPrefix().size());         
  if (request.GetParsedArg().IsQuoted())                                    
    to_add.push_back(request.GetParsedArg().GetQuoteChar());                
  to_add.push_back(' '); <- here we add the space. Changing this to another character seems to correctly overwrite the buffer.                                                   
  el_insertstr(m_editline, to_add.c_str()); <- This space doesn't seem to overwrite the buffer?                                 
  return CC_REFRESH;                                                        
}

Did you try if your previous code fixes this issue?

[gedatsu217]

I don't think the value of m_previous_autosuggestion_size should only grow (which is what this if is doing), as this way we just keep printing a longer and longer space buffer over time. Just printing enough to clear the buffer of the previous completion is enough.

If I keep the number of characters of the only previous command, I think there is a problem. For example, If I type "help frame var" → "help frame info" → "help frame v", the remains are hidden. However, If I type "help frame var" → "help frame info" → "help" → "help frame v", the number of characters of "help frame var" is more than that of "help", so "help frame v[aro]" is displayed. What do you think?

Not sure if I understand your example correctly, but as soon as you type "help frame ", you should have an autosuggestion of "help frame info" and then typing the "v" should clear the "nfo" part. The "help" autosuggestion should not be involved at all in any logic after you typed "help "?

What I mean is that if I should keep the length of characters of the only previous command, following thing occurs.

1. execution "help frame var" and m_previous_autosuggestion_size = len("help frame var") = 14
2. execution "help frame info" and m_previous_autosuggestion_size = len("help frame info") = 15
3. execution "help" and m_previous_autosuggestion_size = len("help") = 4
4. Typing "help frame v". Then, spaces_to_print = m_previous_autosuggestion_size - line.size() - to_add.getValue().length() = 4 - 12 - 2 < 0. So, spaces are not added. (In short, "help frame v[aro]" is displayed.)

(Maybe, I do not understand what you say. )

Also you can just do this calculation directly after you calculated int spaces_to_print above. This way this code is closer together which hopefully makes this easier to understand.

For example, if the user executes a command directly after using tab-completion, Editline::TypedCharacter is not called, so spaces_to_print is not calculated. That is because I can calculate this here.

Can you give me an example input where this breaks? I'm not sure how the tab completion would influence until what column we would need to overwrite the line buffer (that should still be the same).

If m_previous_autosuggestion_size is calculated in Editline::TypedCharacter,

1. Execution "help frame info", m_previous_autosuggestion_size = len("help frame info") = 15
2. Typing "help frame va" and pressing tab-key. "help frame variable" is executed. m_previous_autosuggestion_size = len("help frame va") = 13. (because m_previous_autosuggestion_size is not calculated in Editline::TabCommand.)
3. Typing "help frame i". Then, spaces_to_print = m_previous_autosuggestion_size - line.size() - to_add.getValue().length() = 13 - 12 - 3 < 0. So, spaces are not added. "help frame i[nfoable] is displayed.

Did you try if your previous code fixes this issue?

Not yet. I will try it.

[gedatsu217]

@teemperor I understand what you say just now. Indeed, your method is more efficient than mine. I'm fixing the code now.

[gedatsu217]

Add test.

Delete gray character if autosuggestion has the only one character. (e.g. "b") (Editline::TabCommand)

Simplify the code.

[teemperor]

So the way the issue with the single space is now solved is by doing CC_REDISPLAY when we're only adding the single space? Isn't that also deleting the whole autosuggestion?

In D81001#2190864, @gedatsu217 wrote:

I don't think the value of m_previous_autosuggestion_size should only grow (which is what this if is doing), as this way we just keep printing a longer and longer space buffer over time. Just printing enough to clear the buffer of the previous completion is enough.

If I keep the number of characters of the only previous command, I think there is a problem. For example, If I type "help frame var" → "help frame info" → "help frame v", the remains are hidden. However, If I type "help frame var" → "help frame info" → "help" → "help frame v", the number of characters of "help frame var" is more than that of "help", so "help frame v[aro]" is displayed. What do you think?

Not sure if I understand your example correctly, but as soon as you type "help frame ", you should have an autosuggestion of "help frame info" and then typing the "v" should clear the "nfo" part. The "help" autosuggestion should not be involved at all in any logic after you typed "help "?

What I mean is that if I should keep the length of characters of the only previous command, following thing occurs.

1. execution "help frame var" and m_previous_autosuggestion_size = len("help frame var") = 14
2. execution "help frame info" and m_previous_autosuggestion_size = len("help frame info") = 15
3. execution "help" and m_previous_autosuggestion_size = len("help") = 4
4. Typing "help frame v". Then, spaces_to_print = m_previous_autosuggestion_size - line.size() - to_add.getValue().length() = 4 - 12 - 2 < 0. So, spaces are not added. (In short, "help frame v[aro]" is displayed.)

(Maybe, I do not understand what you say. )

I think you got it :). My point was that m_previous_autosuggestion_size is updated everytime you type a character from what I can see, so you don't see the 4 from step 3. It's more like this from what I can tell:

1. same as your step 1.
2. same as your step 2.
3. same asyour step 3: execution "help" and m_previous_autosuggestion_size = len("help") = 4

3.5 [new step]: Typing "help frame ". m_previous_autosuggestion_size = len("help frame info") = 15. This is because we update the size in TypedCharacter which gets triggered for every character.

4. Typing "help frame v". Then, spaces_to_print = m_previous_autosuggestion_size - line.size() - to_add.getValue().length() = 15 - 12 - 2 = 1. (which is exactly the space needed to overwrite the 'o').

lldb/source/Host/common/Editline.cpp

1009

I think this can be shortened to just:

  to_add.push_back(' ');
  el_insertstr(m_editline, to_add.c_str());
  // Clear autosuggestion from line buffer if we only added a space.
  if (to_add == " ")
    return CC_REDISPLAY;
  return CC_REFRESH;
}

1079

The (int) cast isn't necessary.

[gedatsu217]

So the way the issue with the single space is now solved is by doing CC_REDISPLAY when we're only adding the single space? Isn't that also deleting the whole autosuggestion?

Yes. CC_REDISPLAY can delete all the gray characters left.

lldb/source/Host/common/Editline.cpp

1079

I found that lldb crashed. Sorry, I should have checked more when I uploaded this.

I use spaces_to_print as size_t, but this sometimes becomes less than zero. So, I have to use this as int.

[teemperor]

In D81001#2199205, @gedatsu217 wrote:

So the way the issue with the single space is now solved is by doing CC_REDISPLAY when we're only adding the single space? Isn't that also deleting the whole autosuggestion?

Yes. CC_REDISPLAY can delete all the gray characters left.

So, if I would type "b" and then press tab, the autosuggestion would briefly disappear until I type the next character?

lldb/source/Host/common/Editline.cpp

1079

Yeah I see that spaces_to_print can overflow. How about this code instead that avoids all the casting and overflowing and so on:

size_t new_autosuggestion_size = line.size() + to_add->length();
// If the previous command line + autosuggestion was longer than the
// current autosuggestion, make sure that the autosuggestion is overwriting
// the old output by enough adding trailing spaces.
if (new_autosuggestion_size < m_previous_autosuggestion_size) {
  size_t spaces_to_print = m_previous_autosuggestion_size - new_autosuggestion_size;
  std::string spaces = std::string(spaces_to_print, ' ');
  fputs(spaces.c_str(), m_output_file);
}
m_previous_autosuggestion_size = new_autosuggestion_size;

[gedatsu217]

So, if I would type "b" and then press tab, the autosuggestion would briefly disappear until I type the next character?

Yes. Indeed, it may not be good. I'll think of other ways.

[teemperor]

In D81001#2199450, @gedatsu217 wrote:

So, if I would type "b" and then press tab, the autosuggestion would briefly disappear until I type the next character?

Yes. Indeed, it may not be good. I'll think of other ways.

This seems minor enough that I wouldn't block the whole patch over it, so this can be addresses in a follow-up patch.

Are there any other problems with the current state of the patch that haven't been resolved? (I went over the comments, but it's hard to say what has and hasn't been fixed until now).

[gedatsu217]

Are there any other problems with the current state of the patch that haven't been resolved? (I went over the comments, but it's hard to say what has and hasn't been fixed until now).

Yes, I have solved all the problems.

[gedatsu217]

Simplify the code.

[teemperor]

Alright, let's see if the others see any other issues, otherwise I think this is finally good to go.

lldb/source/Host/common/Editline.cpp

1011–1014

Without a comment this code is very mysterious to whoever will read it.

[gedatsu217]

add comments.

revise a test(add self.quit()).

[JDevlieghere]

I applied the patch locally and gave this another shot. I'm still missing all the common ways I accept auto-suggestions in fish:

• C-e
• Right arrow
• End

but I'm fine with fixing that in another patch assuming that wouldn't require a huge redesign. BTW your original description says you can accept suggestions with C-k, but it's actually C-f.

My terminal isn't honoring the faint escape code, it would be nice to make this configurable like most of the other color options.

As I mentioned in my earlier comment you can't enable auto-suggestions in the current command interpreter. You have to launch with -o settings set show-autosuggestion true or adding it to your `.lldbinit. I think that's fine but I'd be nice if we could fix that.

All these things can be fixed in follow-up patches. I already have a hard time following the discussion here so I'm fine with landing this and improving things incrementally.

lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py

106

No newline at end of file

[teemperor]

Also signing this off as I think this is in a good enough state to land. The remaining issues can all be follow-up patches.

Also you might want to change your editor to remove trailing spaces from lines. The test file has a lot of them (I'll just fix that when merging for this patch).

[labath]

Slightly late to the party, but I want to point out that there _is_ an escape code for clearing a line:

CSI n K 	--	Erase in Line --	Erases part of the line. If n is 0 (or missing), clear from cursor to the end of the line. If n is 1, clear from cursor to beginning of the line. If n is 2, clear entire line. Cursor position does not change.

It sounds like it should be sufficient to slap this to the end of the autosuggested text, before the cursor is moved in any way.

That said I'm not sure what exactly will this do in case the suggested text spans multiple lines (because it's longer than the terminal width). It's possible inserting spaces will handle that better, but it's also possible it will not -- I can imagine that creating a new line in an autosuggestion will hopelessly confuse lldb/editline. In either case, I think it'd be good to verify how we handle commands which span multiple lines.

But that can come as a separate patch.

lldb/include/lldb/Core/Debugger.h

public:
   bool GetUseExternalEditor() const;
 
   bool SetUseExternalEditor(bool use_external_editor_p);
 
   bool GetUseColor() const;
 
   bool SetUseColor(bool use_color);
 
+  bool GetUseAutosuggestion() const;
+
   bool GetUseSourceCache() const;
 
   bool SetUseSourceCache(bool use_source_cache);
 
   bool GetHighlightSource() const;
 
   lldb::StopShowColumn GetStopShowColumn() const;
 

lldb/include/lldb/Core/IOHandler.h

IOHandlerDelegate(Completion completion = Completion::None)
       : m_completion(completion) {}
 
   virtual ~IOHandlerDelegate() = default;
 
   virtual void IOHandlerActivated(IOHandler &io_handler, bool interactive) {}
 
   virtual void IOHandlerDeactivated(IOHandler &io_handler) {}
 
+  virtual llvm::Optional<std::string> IOHandlerSuggestion(IOHandler &io_handler,
+                                                          llvm::StringRef line);
+
   virtual void IOHandlerComplete(IOHandler &io_handler,
                                  CompletionRequest &request);
 
   virtual const char *IOHandlerGetFixIndentationCharacters() { return nullptr; }
 
   /// Called when a new line is created or one of an identified set of
   /// indentation characters is typed.
   ///

 private:
 #if LLDB_ENABLE_LIBEDIT
   static bool IsInputCompleteCallback(Editline *editline, StringList &lines,
                                       void *baton);
 
   static int FixIndentationCallback(Editline *editline, const StringList &lines,
                                     int cursor_position, void *baton);
 
+  static llvm::Optional<std::string> SuggestionCallback(llvm::StringRef line,
+                                                        void *baton);
+
   static void AutoCompleteCallback(CompletionRequest &request, void *baton);
 #endif
 
 protected:
 #if LLDB_ENABLE_LIBEDIT
   std::unique_ptr<Editline> m_editline_up;
 #endif
   IOHandlerDelegate &m_delegate;

lldb/include/lldb/Host/Editline.h

 
 typedef bool (*IsInputCompleteCallbackType)(Editline *editline,
                                             StringList &lines, void *baton);
 
 typedef int (*FixIndentationCallbackType)(Editline *editline,
                                           const StringList &lines,
                                           int cursor_position, void *baton);
 
+typedef llvm::Optional<std::string> (*SuggestionCallbackType)(
+    llvm::StringRef line, void *baton);
+
 typedef void (*CompleteCallbackType)(CompletionRequest &request, void *baton);
 
 /// Status used to decide when and how to start editing another line in
 /// multi-line sessions
 enum class EditorStatus {
 
   /// The default state proceeds to edit the current line
   Editing,
public:
   uint32_t GetCurrentLine();
 
   /// Interrupt the current edit as if ^C was pressed
   bool Interrupt();
 
   /// Cancel this edit and oblitarate all trace of it
   bool Cancel();
 
+  /// Register a callback for autosuggestion.
+  void SetSuggestionCallback(SuggestionCallbackType callback, void *baton);
+
   /// Register a callback for the tab key
   void SetAutoCompleteCallback(CompleteCallbackType callback, void *baton);
 
   /// Register a callback for testing whether multi-line input is complete
   void SetIsInputCompleteCallback(IsInputCompleteCallbackType callback,
                                   void *baton);
 
   /// Register a callback for determining the appropriate indentation for a line
private:
 
   /// Buffer end command used when Esc > is typed in multi-line emacs mode.
   unsigned char BufferEndCommand(int ch);
 
   /// Context-sensitive tab insertion or code completion command used when the
   /// tab key is typed.
   unsigned char TabCommand(int ch);
 
+  /// Apply autosuggestion part in gray as editline.
+  unsigned char ApplyAutosuggestCommand(int ch);
+
+  /// Command used when a character is typed.
+  unsigned char TypedCharacter(int ch);
+
   /// Respond to normal character insertion by fixing line indentation
   unsigned char FixIndentationCommand(int ch);
 
   /// Revert line command used when moving between lines.
   unsigned char RevertLineCommand(int ch);
 
   /// Ensures that the current EditLine instance is properly configured for
   /// single or multi-line editing.
#endif
   ConnectionFileDescriptor m_input_connection;
   IsInputCompleteCallbackType m_is_input_complete_callback = nullptr;
   void *m_is_input_complete_callback_baton = nullptr;
   FixIndentationCallbackType m_fix_indentation_callback = nullptr;
   void *m_fix_indentation_callback_baton = nullptr;
   const char *m_fix_indentation_callback_chars = nullptr;
   CompleteCallbackType m_completion_callback = nullptr;
   void *m_completion_callback_baton = nullptr;
-
+  SuggestionCallbackType m_suggestion_callback = nullptr;
+  void *m_suggestion_callback_baton = nullptr;
+  std::size_t m_previous_autosuggestion_size = 0;
   std::mutex m_output_mutex;
 };
 }
 
 #endif // #if defined(__cplusplus)
 #endif // LLDB_HOST_EDITLINE_H

lldb/include/lldb/Interpreter/CommandInterpreter.h

public:
   ///    and failed with some explanation if we aborted executing the commands
   ///    at some point.
   void HandleCommandsFromFile(FileSpec &file, ExecutionContext *context,
                               CommandInterpreterRunOptions &options,
                               CommandReturnObject &result);
 
   CommandObject *GetCommandObjectForCommand(llvm::StringRef &command_line);
 
+  /// Returns the auto-suggestion string that should be added to the given
+  /// command line.
+  llvm::Optional<std::string> GetAutoSuggestionForCommand(llvm::StringRef line);
+
   // This handles command line completion.
   void HandleCompletion(CompletionRequest &request);
 
   // This version just returns matches, and doesn't compute the substring. It
   // is here so the Help command can call it for the first argument.
   void HandleCompletionMatches(CompletionRequest &request);
 
   int GetCommandNamesMatchingPartialString(const char *cmd_cstr,

lldb/source/Core/CoreProperties.td

let Definition = "debugger" in {
   def EscapeNonPrintables: Property<"escape-non-printables", "Boolean">,
     Global,
     DefaultTrue,
     Desc<"If true, LLDB will automatically escape non-printable and escape characters when formatting strings.">;
   def FrameFormatUnique: Property<"frame-format-unique", "FormatEntity">,
     Global,
     DefaultStringValue<"frame #${frame.index}: ${ansi.fg.yellow}${frame.pc}${ansi.normal}{ ${module.file.basename}{`${function.name-without-args}{${frame.no-debug}${function.pc-offset}}}}{ at ${ansi.fg.cyan}${line.file.basename}${ansi.normal}:${ansi.fg.yellow}${line.number}${ansi.normal}{:${ansi.fg.yellow}${line.column}${ansi.normal}}}{${function.is-optimized} [opt]}{${frame.is-artificial} [artificial]}\\\\n">,
     Desc<"The default frame format string to use when displaying stack frameinformation for threads from thread backtrace unique.">;
+  def ShowAutosuggestion: Property<"show-autosuggestion", "Boolean">,
+    Global,
+    DefaultFalse,
+    Desc<"If true, LLDB will show suggestions to complete the command the user typed.">;
 }

lldb/source/Core/Debugger.cpp

 
 bool Debugger::SetUseColor(bool b) {
   const uint32_t idx = ePropertyUseColor;
   bool ret = m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
   SetPrompt(GetPrompt());
   return ret;
 }
 
+bool Debugger::GetUseAutosuggestion() const {
+  const uint32_t idx = ePropertyShowAutosuggestion;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+      nullptr, idx, g_debugger_properties[idx].default_uint_value != 0);
+}
+
 bool Debugger::GetUseSourceCache() const {
   const uint32_t idx = ePropertyUseSourceCache;
   return m_collection_sp->GetPropertyAtIndexAsBoolean(
       nullptr, idx, g_debugger_properties[idx].default_uint_value != 0);
 }
 
 bool Debugger::SetUseSourceCache(bool b) {
   const uint32_t idx = ePropertyUseSourceCache;

lldb/source/Core/IOHandler.cpp

if (line == "yes" || line == "YES" || line == "Yes") {
     m_user_response = true;
     io_handler.SetIsDone(true);
   } else if (line == "no" || line == "NO" || line == "No") {
     m_user_response = false;
     io_handler.SetIsDone(true);
   }
 }
 
+llvm::Optional<std::string>
+IOHandlerDelegate::IOHandlerSuggestion(IOHandler &io_handler,
+                                       llvm::StringRef line) {
+  return io_handler.GetDebugger()
+      .GetCommandInterpreter()
+      .GetAutoSuggestionForCommand(line);
+}
+
 void IOHandlerDelegate::IOHandlerComplete(IOHandler &io_handler,
                                           CompletionRequest &request) {
   switch (m_completion) {
   case Completion::None:
     break;
   case Completion::LLDBCommand:
     io_handler.GetDebugger().GetCommandInterpreter().HandleCompletion(request);
     break;
use_editline = GetInputFILE() && GetOutputFILE() && GetErrorFILE() &&
                  m_input_sp && m_input_sp->GetIsRealTerminal();
 
   if (use_editline) {
     m_editline_up = std::make_unique<Editline>(editline_name, GetInputFILE(),
                                                GetOutputFILE(), GetErrorFILE(),
                                                m_color_prompts);
     m_editline_up->SetIsInputCompleteCallback(IsInputCompleteCallback, this);
     m_editline_up->SetAutoCompleteCallback(AutoCompleteCallback, this);
+    if (debugger.GetUseAutosuggestion() && debugger.GetUseColor())
+      m_editline_up->SetSuggestionCallback(SuggestionCallback, this);
     // See if the delegate supports fixing indentation
     const char *indent_chars = delegate.IOHandlerGetFixIndentationCharacters();
     if (indent_chars) {
       // The delegate does support indentation, hook it up so when any
       // indentation character is typed, the delegate gets a chance to fix it
       m_editline_up->SetFixIndentationCallback(FixIndentationCallback, this,
                                                indent_chars);
     }
   }
 #endif
   SetBaseLineNumber(m_base_line_number);
   SetPrompt(prompt);
   SetContinuationPrompt(continuation_prompt);
 }
 
 IOHandlerEditline::~IOHandlerEditline() {
 #if LLDB_ENABLE_LIBEDIT
int IOHandlerEditline::FixIndentationCallback(Editline *editline,
                                               const StringList &lines,
                                               int cursor_position,
                                               void *baton) {
   IOHandlerEditline *editline_reader = (IOHandlerEditline *)baton;
   return editline_reader->m_delegate.IOHandlerFixIndentation(
       *editline_reader, lines, cursor_position);
 }
 
+llvm::Optional<std::string>
+IOHandlerEditline::SuggestionCallback(llvm::StringRef line, void *baton) {
+  IOHandlerEditline *editline_reader = static_cast<IOHandlerEditline *>(baton);
+  if (editline_reader)
+    return editline_reader->m_delegate.IOHandlerSuggestion(*editline_reader,
+                                                           line);
+
+  return llvm::None;
+}
+
 void IOHandlerEditline::AutoCompleteCallback(CompletionRequest &request,
                                              void *baton) {
   IOHandlerEditline *editline_reader = (IOHandlerEditline *)baton;
   if (editline_reader)
     editline_reader->m_delegate.IOHandlerComplete(*editline_reader, request);
 }
 #endif
 

lldb/source/Host/common/Editline.cpp

unsigned char Editline::TabCommand(int ch) {
 
   if (results.size() == 1) {
     CompletionResult::Completion completion = results.front();
     switch (completion.GetMode()) {
     case CompletionMode::Normal: {
       std::string to_add = completion.GetCompletion();
       to_add = to_add.substr(request.GetCursorArgumentPrefix().size());
       if (request.GetParsedArg().IsQuoted())
         to_add.push_back(request.GetParsedArg().GetQuoteChar());
       to_add.push_back(' ');
       el_insertstr(m_editline, to_add.c_str());
-      break;
+      // Clear all the autosuggestion parts if the only single space can be completed.
+      if (to_add == " ")
+        return CC_REDISPLAY;
+      return CC_REFRESH;
     }
     case CompletionMode::Partial: {
       std::string to_add = completion.GetCompletion();
       to_add = to_add.substr(request.GetCursorArgumentPrefix().size());
       el_insertstr(m_editline, to_add.c_str());
       break;
     }
     case CompletionMode::RewriteLine: {
       el_deletestr(m_editline, line_info->cursor - line_info->buffer);
unsigned char Editline::TabCommand(int ch) {
 
   DisplayCompletions(m_editline, m_output_file, results);
 
   DisplayInput();
   MoveCursor(CursorLocation::BlockEnd, CursorLocation::EditingCursor);
   return CC_REDISPLAY;
 }
 
+unsigned char Editline::ApplyAutosuggestCommand(int ch) {
+  const LineInfo *line_info = el_line(m_editline);
+  llvm::StringRef line(line_info->buffer,
+                       line_info->lastchar - line_info->buffer);
+
+  if (llvm::Optional<std::string> to_add =
+          m_suggestion_callback(line, m_suggestion_callback_baton))
+    el_insertstr(m_editline, to_add->c_str());
+
+  return CC_REDISPLAY;
+}
+
+unsigned char Editline::TypedCharacter(int ch) {
+  std::string typed = std::string(1, ch);
+  el_insertstr(m_editline, typed.c_str());
+  const LineInfo *line_info = el_line(m_editline);
+  llvm::StringRef line(line_info->buffer,
+                       line_info->lastchar - line_info->buffer);
+
+  if (llvm::Optional<std::string> to_add =
+          m_suggestion_callback(line, m_suggestion_callback_baton)) {
+    std::string to_add_color = ANSI_FAINT + to_add.getValue() + ANSI_UNFAINT;
+    fputs(typed.c_str(), m_output_file);
+    fputs(to_add_color.c_str(), m_output_file);
+    size_t new_autosuggestion_size = line.size() + to_add->length();
+    // Print spaces to hide any remains of a previous longer autosuggestion.
+    if (new_autosuggestion_size < m_previous_autosuggestion_size) {
+      size_t spaces_to_print =
+          m_previous_autosuggestion_size - new_autosuggestion_size;
+      std::string spaces = std::string(spaces_to_print, ' ');
+      fputs(spaces.c_str(), m_output_file);
+    }
+    m_previous_autosuggestion_size = new_autosuggestion_size;
+
+    int editline_cursor_position =
+        (int)((line_info->cursor - line_info->buffer) + GetPromptWidth());
+    int editline_cursor_row = editline_cursor_position / m_terminal_width;
+    int toColumn =
+        editline_cursor_position - (editline_cursor_row * m_terminal_width);
+    fprintf(m_output_file, ANSI_SET_COLUMN_N, toColumn);
+    return CC_REFRESH;
+  }
+
+  return CC_REDISPLAY;
+}
+
 void Editline::ConfigureEditor(bool multiline) {
   if (m_editline && m_multiline_enabled == multiline)
     return;
   m_multiline_enabled = multiline;
 
   if (m_editline) {
     // Disable edit mode to stop the terminal from flushing all input during
     // the call to el_end() since we expect to have multiple editline instances
el_wset(m_editline, EL_ADDFN, EditLineConstString("lldb-complete"),
           EditLineConstString("Invoke completion"), complete_callback);
   el_wset(m_editline, EL_ADDFN, EditLineConstString("lldb_complete"),
           EditLineConstString("Invoke completion"), complete_callback);
 
   // General bindings we don't mind being overridden
   if (!multiline) {
     el_set(m_editline, EL_BIND, "^r", "em-inc-search-prev",
            NULL); // Cycle through backwards search, entering string
+
+    if (m_suggestion_callback) {
+      el_wset(m_editline, EL_ADDFN, EditLineConstString("lldb-apply-complete"),
+              EditLineConstString("Adopt autocompletion"),
+              (EditlineCommandCallbackType)([](EditLine *editline, int ch) {
+                return Editline::InstanceFor(editline)->ApplyAutosuggestCommand(
+                    ch);
+              }));
+
+      el_set(m_editline, EL_BIND, "^f", "lldb-apply-complete",
+             NULL); // Apply a part that is suggested automatically
+
+      el_wset(m_editline, EL_ADDFN, EditLineConstString("lldb-typed-character"),
+              EditLineConstString("Typed character"),
+              (EditlineCommandCallbackType)([](EditLine *editline, int ch) {
+                return Editline::InstanceFor(editline)->TypedCharacter(ch);
+              }));
+
+      char bind_key[2] = {0, 0};
+      llvm::StringRef ascii_chars =
+          "abcdefghijklmnopqrstuvwxzyABCDEFGHIJKLMNOPQRSTUVWXZY1234567890!\"#$%"
+          "&'()*+,./:;<=>?@[]_`{|}~ ";
+      for (char c : ascii_chars) {
+        bind_key[0] = c;
+        el_set(m_editline, EL_BIND, bind_key, "lldb-typed-character", NULL);
+      }
+      el_set(m_editline, EL_BIND, "\\-", "lldb-typed-character", NULL);
+      el_set(m_editline, EL_BIND, "\\^", "lldb-typed-character", NULL);
+      el_set(m_editline, EL_BIND, "\\\\", "lldb-typed-character", NULL);
     }
+  }
+
   el_set(m_editline, EL_BIND, "^w", "ed-delete-prev-word",
          NULL); // Delete previous word, behave like bash in emacs mode
   el_set(m_editline, EL_BIND, "\t", "lldb-complete",
          NULL); // Bind TAB to auto complete
 
   // Allow ctrl-left-arrow and ctrl-right-arrow for navigation, behave like
   // bash in emacs mode.
   el_set(m_editline, EL_BIND, ESCAPE "[1;5C", "em-next-word", NULL);
   el_set(m_editline, EL_BIND, ESCAPE "[1;5D", "ed-prev-word", NULL);
   el_set(m_editline, EL_BIND, ESCAPE "[5C", "em-next-word", NULL);
   el_set(m_editline, EL_BIND, ESCAPE "[5D", "ed-prev-word", NULL);
   el_set(m_editline, EL_BIND, ESCAPE ESCAPE "[C", "em-next-word", NULL);
   el_set(m_editline, EL_BIND, ESCAPE ESCAPE "[D", "ed-prev-word", NULL);
 
   // Allow user-specific customization prior to registering bindings we
   // absolutely require
   el_source(m_editline, nullptr);
 
   // Register an internal binding that external developers shouldn't use
   el_wset(m_editline, EL_ADDFN, EditLineConstString("lldb-revert-line"),
           EditLineConstString("Revert line to saved state"),
           (EditlineCommandCallbackType)([](EditLine *editline, int ch) {
             return Editline::InstanceFor(editline)->RevertLineCommand(ch);
           }));
 
   // Register keys that perform auto-indent correction
   if (m_fix_indentation_callback && m_fix_indentation_callback_chars) {
     char bind_key[2] = {0, 0};
     const char *indent_chars = m_fix_indentation_callback_chars;
     while (*indent_chars) {
       bind_key[0] = *indent_chars;
       el_set(m_editline, EL_BIND, bind_key, "lldb-fix-indentation", NULL);
       ++indent_chars;
     }
   }
 
   // Multi-line editor bindings
   if (multiline) {
     el_set(m_editline, EL_BIND, "\n", "lldb-end-or-add-line", NULL);
     el_set(m_editline, EL_BIND, "\r", "lldb-end-or-add-line", NULL);
     el_set(m_editline, EL_BIND, ESCAPE "\n", "lldb-break-line", NULL);
     el_set(m_editline, EL_BIND, ESCAPE "\r", "lldb-break-line", NULL);
     el_set(m_editline, EL_BIND, "^p", "lldb-previous-line", NULL);
     el_set(m_editline, EL_BIND, "^n", "lldb-next-line", NULL);
     el_set(m_editline, EL_BIND, "^?", "lldb-delete-previous-char", NULL);
     el_set(m_editline, EL_BIND, "^d", "lldb-delete-next-char", NULL);
     el_set(m_editline, EL_BIND, ESCAPE "[3~", "lldb-delete-next-char", NULL);
     el_set(m_editline, EL_BIND, ESCAPE "[\\^", "lldb-revert-line", NULL);
 
if (m_editor_status == EditorStatus::Editing) {
     MoveCursor(CursorLocation::EditingCursor, CursorLocation::BlockStart);
     fprintf(m_output_file, ANSI_CLEAR_BELOW);
     result = m_input_connection.InterruptRead();
   }
   m_editor_status = EditorStatus::Interrupted;
   return result;
 }
 
+void Editline::SetSuggestionCallback(SuggestionCallbackType callback,
+                                     void *baton) {
+  m_suggestion_callback = callback;
+  m_suggestion_callback_baton = baton;
+}
+
 void Editline::SetAutoCompleteCallback(CompleteCallbackType callback,
                                        void *baton) {
   m_completion_callback = callback;
   m_completion_callback_baton = baton;
 }
 
 void Editline::SetIsInputCompleteCallback(IsInputCompleteCallbackType callback,
                                           void *baton) {
   m_is_input_complete_callback = callback;
   m_is_input_complete_callback_baton = baton;
 }
 
 bool Editline::SetFixIndentationCallback(FixIndentationCallbackType callback,
if (input == nullptr) {
       m_editor_status = EditorStatus::EndOfInput;
     } else {
       m_history_sp->Enter(input);
 #if LLDB_EDITLINE_USE_WCHAR
       line = m_utf8conv.to_bytes(SplitLines(input)[0]);
 #else
       line = SplitLines(input)[0];
 #endif
       m_editor_status = EditorStatus::Complete;
     }
   }
   return m_editor_status != EditorStatus::EndOfInput;
 }
 
 bool Editline::GetLines(int first_line_number, StringList &lines,
                         bool &interrupted) {
   ConfigureEditor(true);

lldb/source/Interpreter/CommandInterpreter.cpp

if (first_arg.front() == CommandHistory::g_repeat_char) {
                               CompletionMode::RewriteLine);
       return;
     }
   }
 
   HandleCompletionMatches(request);
 }
 
+llvm::Optional<std::string>
+CommandInterpreter::GetAutoSuggestionForCommand(llvm::StringRef line) {
+  if (line.empty())
+    return llvm::None;
+  const size_t s = m_command_history.GetSize();
+  for (int i = s - 1; i >= 0; --i) {
+    llvm::StringRef entry = m_command_history.GetStringAtIndex(i);
+    if (entry.consume_front(line))
+      return entry.str();
+  }
+  return llvm::None;
+}
+
 CommandInterpreter::~CommandInterpreter() {}
 
 void CommandInterpreter::UpdatePrompt(llvm::StringRef new_prompt) {
   EventSP prompt_change_event_sp(
       new Event(eBroadcastBitResetPrompt, new EventDataBytes(new_prompt)));
   ;
   BroadcastEvent(prompt_change_event_sp);
   if (m_command_io_handler_sp)

lldb/test/API/iohandler/autosuggestion/TestAutosuggestion.py

+"""
+Tests autosuggestion using pexpect.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+def cursor_horizontal_abs(s):
+    return "\x1b[" + str(len(s) + 1) + "G"
+
+
+
+class TestCase(PExpectTest):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    # PExpect uses many timeouts internally and doesn't play well
+    # under ASAN on a loaded machine..
+    @skipIfAsan
+    @skipIfEditlineSupportMissing
+    def test_autosuggestion_add_spaces(self):
+        self.launch(extra_args=["-o", "settings set show-autosuggestion true", "-o", "settings set use-color true"])
+
+        # Common input codes and escape sequences.
+        faint_color = "\x1b[2m"
+        reset = "\x1b[0m"
+
+        # Check if spaces are added to hide the previous gray characters.
+        self.expect("help frame var")
+        self.expect("help frame info")
+        self.child.send("help frame v")
+        self.child.expect_exact(cursor_horizontal_abs("(lldb) help frame ") + "v" + faint_color + "ar" + reset + " ")
+
+        self.quit()
+
+    @skipIfAsan
+    @skipIfEditlineSupportMissing
+    def test_autosuggestion(self):
+        self.launch(extra_args=["-o", "settings set show-autosuggestion true", "-o", "settings set use-color true"])
+
+        # Common input codes and escape sequences.
+        ctrl_f = "\x06"
+        faint_color = "\x1b[2m"
+        reset = "\x1b[0m"
+        delete = chr(127)
+
+        frame_output_needle = "Syntax: frame <subcommand>"
+        # Run 'help frame' once to put it into the command history.
+        self.expect("help frame", substrs=[frame_output_needle])
+
+        # Check that LLDB shows the autosuggestion in gray behind the text.
+        self.child.send("hel")
+        self.child.expect_exact(cursor_horizontal_abs("(lldb) he") + "l" + faint_color + "p frame" + reset)
+
+        # Apply the autosuggestion and press enter. This should print the
+        # 'help frame' output if everything went correctly.
+        self.child.send(ctrl_f + "\n")
+        self.child.expect_exact(frame_output_needle)
+
+        # Check that pressing Ctrl+F directly after Ctrl+F again does nothing.
+        self.child.send("hel" + ctrl_f + ctrl_f + "\n")
+        self.child.expect_exact(frame_output_needle)
+
+        # Try autosuggestion using tab and ^f.
+        # \t makes "help" and ^f makes "help frame". If everything went
+        # correct we should see the 'help frame' output again.
+        self.child.send("hel\t" + ctrl_f + "\n")
+        self.child.expect_exact(frame_output_needle)
+
+        # Check that autosuggestion works after delete.
+        self.child.send("a1234" + 5 * delete + "hel" + ctrl_f + "\n")
+        self.child.expect_exact(frame_output_needle)
+
+        # Check that autosuggestion works after delete.
+        self.child.send("help x" + delete + ctrl_f + "\n")
+        self.child.expect_exact(frame_output_needle)
+
+        # Check that autosuggestion complete to the most recent one.
+        self.child.send("help frame variable\n")
+        self.child.send("help fr")
+        self.child.expect_exact(faint_color + "ame variable" + reset)
+        self.child.send("\n")
+
+        # Try another command.
+        apropos_output_needle = "Syntax: apropos <search-word>"
+        # Run 'help frame' once to put it into the command history.
+        self.expect("help apropos", substrs=[apropos_output_needle])
+
+        # Check that 'hel' should have an autosuggestion for 'help apropos' now.
+        self.child.send("hel")
+        self.child.expect_exact(cursor_horizontal_abs("(lldb) he") + "l" + faint_color + "p apropos" + reset)
+
+        # Run the command and expect the 'help apropos' output.
+        self.child.send(ctrl_f + "\n")
+        self.child.expect_exact(apropos_output_needle)
+
+        # Check that pressing Ctrl+F in an empty prompt does nothing.
+        breakpoint_output_needle = "Syntax: breakpoint <subcommand>"
+        self.child.send(ctrl_f + "help breakpoint" +"\n")
+        self.child.expect_exact(breakpoint_output_needle)
+
+
+        self.quit()