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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to