[Lldb-commits] [PATCH] D51175: Add support for descriptions with command completions.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

@jingham I'm confused, what are the requested changes? Because the behavior we seem to agree on is that we keep completions with the same value but different descriptions which is already the current implementation (see lines 138 and 148 in CompletionRequestTest.cpp)



================
Comment at: source/Utility/CompletionRequest.cpp:79
+  // Add the completion if we haven't seen the same value before.
+  if (m_added_values.insert(r.GetUniqueKey()).second)
+    m_results.push_back(r);
----------------
jingham wrote:
> JDevlieghere wrote:
> > Do you think there's any value in in checking the description? For example, if the description was empty for the existing value but a description is provided for the duplicate?
> This seems reasonable.  For instance, for process attach we might want to do something like:
> 
> (lldb) process attach -n Foo<TAB>
>     Foo - pid 123
>     Foo - pid 234
>     FooBar - pid 345
> 
> But then the two Foo's would have the same unique key and you would only print one.
Also, when providing for example completions for the expression command, we would have the same function name with different argument lists. E.g.
```
  foo( -- int foo(int, int)
  foo( -- int foo(double, double)
```


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51175