davide added a comment. In https://reviews.llvm.org/D43048#1001293, @davide wrote:
> 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. Also, there's a bit of overengineering in the API. As far as I can tell, max_return_elements only supports `-1`, so that argument is bogus (at least, this is what the comment says, but comment and code could go out of sync, so). https://reviews.llvm.org/D43048 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
