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

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Jun 25 04:42:32 PDT 2020


labath added inline comments.


================
Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:1875-1878
+    if (entry.startswith(line)) {
+      llvm::StringRef res = entry.substr(line.size());
+      result = res.str();
+      return result;
----------------
teemperor wrote:
> gedatsu217 wrote:
> > labath wrote:
> > > ```
> > > if (entry.consume_front(line)) {
> > >   result = entry.str();
> > >   return result; // Why is this returning the result both as a proper return value and a by-ref argument?
> > > }
> > > ```
> > That is because using the llvm::Optional as a return value instead of void would make it more obvious what the function returns in case there is no suggestion.
> > 
> > We've discussed this issue before, so please see the comments above.
> Pavel's point is that the function is returning >both<. You can just remove the `result` parameter and do `result = ... GetAutoSuggestionForCommand(line);` when you call it is his point.
Yes that was one of my points. Thank you, Raphael.

My other point was that using `consume_front` is better than a `startswith`+`substr` combo.


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

https://reviews.llvm.org/D81001





More information about the lldb-commits mailing list