[Lldb-commits] [PATCH] D81001: [lldb] Display autosuggestion part in gray if there is one possible suggestion

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D81001#2190646 <https://reviews.llvm.org/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?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81001/new/

https://reviews.llvm.org/D81001