This is an archive of the discontinued LLVM Phabricator instance.

Add support for descriptions with command completions.
ClosedPublic

Authored by teemperor on Aug 23 2018, 10:53 AM.

Details

Summary

This patch adds a framework for adding descriptions to the command completions we provide.
It also adds descriptions for completed top-level commands so that we can test this code.

Completions are in general supposed to be displayed alongside the completion itself. The descriptions
can be used to provide additional information about the completion to the user. Examples for descriptions
are function signatures when completing function calls in the expression command or the binary name
when providing completion for a symbol.

There is still some boilerplate code from the old completion API left in LLDB (mostly because the respective
APIs are reused for non-completion related purposes, so the CompletionRequest doesn't make sense to be
used), so that's why I still had to change some function signatures. Also, as the old API only passes around a
list of matches, and the descriptions are for these functions just another list, I had to add some code that
essentially just ensures that both lists are always the same side (e.g. all the manual calls to
descriptions->AddString(X) below a matches->AddString(Y) call).

The initial command descriptions that come with this patch are just reusing the existing
short help that is already added in LLDB.

An example completion with descriptions looks like this:

(lldb) pl
Available completions:
        platform -- Commands to manage and create platforms.
        plugin   -- Commands for managing LLDB plugins.

Diff Detail

Repository
rL LLVM

Event Timeline

teemperor created this revision.Aug 23 2018, 10:53 AM
teemperor edited the summary of this revision. (Show Details)

I don't know the completion API well enough to accept, but this looks like a nice improvement!

source/Utility/CompletionRequest.cpp
79 ↗(On Diff #162230)

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?

jingham requested changes to this revision.Sep 12 2018, 5:36 PM
jingham added a subscriber: jingham.

This looks good. I agree with Jonas, however that we might have cases where we end up with two identical completions that have different descriptions, and we shouldn't drop them.

source/Utility/CompletionRequest.cpp
79 ↗(On Diff #162230)

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.

This revision now requires changes to proceed.Sep 12 2018, 5:36 PM

@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)

source/Utility/CompletionRequest.cpp
79 ↗(On Diff #162230)

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)
jingham accepted this revision.Sep 13 2018, 10:26 AM
jingham added inline comments.
source/Utility/CompletionRequest.cpp
79 ↗(On Diff #162230)

I just missed that after adding the number of the completion, you then append the description to make the key. My bad.

This revision is now accepted and ready to land.Sep 13 2018, 10:26 AM
teemperor updated this revision to Diff 165379.Sep 13 2018, 2:25 PM
  • Rebasing before merging.
This revision was automatically updated to reflect the committed changes.