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

Raphael Isemann via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Jun 15 12:37:50 PDT 2020


teemperor added inline comments.


================
Comment at: lldb/source/Core/Debugger.cpp:349
 
+bool Debugger::GetUseAutosuggestion() const {
+  const uint32_t idx = ePropertyShowAutosuggestion;
----------------
gedatsu217 wrote:
> teemperor wrote:
> > 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`).
> 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.
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).


================
Comment at: lldb/source/Host/common/Editline.cpp:1244
+    llvm::StringRef indent_chars =
+        "abcdefghijklmnopqrstuvwxzyABCDEFGHIJKLMNOPQRSTUVWXZY1234567890 ";
+    for (char c : indent_chars) {
----------------
gedatsu217 wrote:
> teemperor wrote:
> > `.-/()[]{};\"'\\!@#$%^&*_` 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.
> 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?
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).


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

https://reviews.llvm.org/D81001





More information about the lldb-commits mailing list