JDevlieghere added inline comments.
================
Comment at: lldb/include/lldb/Host/Editline.h:381
void *m_completion_callback_baton = nullptr;
-
+ std::string m_current_autosuggestion = "";
+ bool m_use_autosuggestion = false;
----------------
Strings are empty by default, so you can omit the assignment: `= ""`.
================
Comment at: lldb/include/lldb/Host/Editline.h:382
+ std::string m_current_autosuggestion = "";
+ bool m_use_autosuggestion = false;
std::mutex m_output_mutex;
----------------
Why do we need this? Can we check that the `m_suggestion_callback` is null
instead?
================
Comment at: lldb/include/lldb/Host/Editline.h:320
+ unsigned char ApplyCompleteCommand(int ch);
+
----------------
teemperor wrote:
> This function and the one below should be documented like the rest.
It's not really clear to me what this method does based on the name or the
description above. It seems similar to `TabCommand`, so maybe
`AutosuggestCommand` would be a better name and more consistent?
================
Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:355
+ /// command line.
+ llvm::Optional<llvm::StringRef>
+ GetAutoSuggestionForCommand(llvm::StringRef line);
----------------
Although I believe the current implementation is correct (because the returned
string is backed by `m_command_history`) I think this interface looks rather
suspicious by returning a `StringRef`. Do we envision this returning //new//
string in the future? If so we might consider having it return a
`llvm::Optional<std::string>`.
================
Comment at: lldb/source/Core/CoreProperties.td:137
+ DefaultTrue,
+ Desc<"Whether to show autosuggestions or not.">;
}
----------------
teemperor wrote:
> Usually these settings start like `If true, LLDB will show suggestions on
> possible commands the user might want to type.` or something like that.
> If true, LLDB will show suggestions to complete the command the user typed.
================
Comment at: lldb/source/Core/IOHandler.cpp:271
m_editline_up->SetAutoCompleteCallback(AutoCompleteCallback, this);
+ m_editline_up->SetShowAutosuggestion(debugger.GetUseAutosuggestion());
// See if the delegate supports fixing indentation
----------------
Why should we set the callback if the Autosuggestions are off? Why not do:
```
if(debugger.GetUseAutosuggestion()))
m_editline_up->SetSuggestionCallback(SuggestionCallback, this);
```
Both have the same downside we already discussed, that the setting cannot be
changed for the current EditLine instance.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81001/new/
https://reviews.llvm.org/D81001
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
