[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In https://reviews.llvm.org/D43048#1001287, @jingham wrote:

> The current auto-completer tests aren't interactive - they do exactly the same thing your command does, but from Python.  It's fine if you want to add tests but please don't remove the current tests since they directly test what the SB clients use.


This is a drop-in replacement for the other test. I'm not sure why we need to keep the other test, but I'll let others comment.

> This will only allow you to test some of the auto-completers, for instance you don't have symbols so you can't test the symbol completer.  But since the symbol commands in this lldb-test have some way to feed symbols in maybe you can crib that.  I think you'll also need to make a target and some other bits as well.  As you start adding these you might find that this becomes onerous, but that will be an interesting experiment.

I voluntarily left that as as follow up.

> You didn't get the HandleCompletion API quite right.  That's my fault this isn't well documented.  The way it works is that if all the potential matches share a common substring, then the 0th element of the result contains the common substring.  If there is no common substring, then the possible matches will be stored from element 1 on in the Results list.  If you want examples take a closer look at how the Python test does it.

The API is a little confusing.
There are multiple issues IMHO:

1. We shouldn't really discriminate based on the position of the elements of the list, as it's easy to get them wrong, as I did. Instead, the API might return a, let's say, pair, where the first element is the common substring and the second element is a list containing etc..
2. We pass strings for the second and third argument (cursor/end), when we should just pass offsets
3. The return value is N and the list contains N +1 values. This is very error prone.

So, I think this might call for a better API?
Also, please note that I read the API and was aware of the oddities, just decided to defer the discussion to another day. I think my usage of the API is correct, as I don't necessarily care about the common substring, if any.


https://reviews.llvm.org/D43048