JDevlieghere added a comment.

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.



================
Comment at: lldb/source/Core/IOHandler.cpp:444
+                                           std::string &result, void *baton) {
+  IOHandlerEditline *editline_reader = (IOHandlerEditline *)baton;
+  if (editline_reader)
----------------
`static_cast<IOHandlerEditline *>(baton)`


================
Comment at: lldb/source/Host/common/Editline.cpp:1239
 
+  if (true) {
+    char bind_key[2] = {0, 0};
----------------
You can just use a lexical block:
```
{ 
  ...
}
```


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