[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 inline comments.


================
Comment at: lldb/source/Host/common/Editline.cpp:1017
+          el_insertstr(m_editline, to_add.c_str());
+          return CC_REFRESH;
+        }
----------------
gedatsu217 wrote:
> teemperor wrote:
> > gedatsu217 wrote:
> > > teemperor wrote:
> > > > 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.
> > > > 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.
> > > 
> > > 
> > 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 
> > 
> > ```
> > lang=c++
> >     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).
> Where is "to_add_autosuggestion" used in the above example?
Yeah, you could also remove that:

```
lang=c++
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.


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

https://reviews.llvm.org/D81001