[v1] gdb/DAP - Add completionsRequest

  Self-explanatory.
---
 gdb/data-directory/Makefile.in       |  1 +
 gdb/python/lib/gdb/dap/__init__.py   |  1 +
 gdb/python/lib/gdb/dap/completion.py | 45 ++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)
 create mode 100644 gdb/python/lib/gdb/dap/completion.py
  

> +def _completions(text: str, column: int, line: Optional[int], frameId: Optional[int]):
> +    cmd_output = gdb.execute("complete " + text, to_string=True)
> +    result = []
> +    for line in cmd_output.splitlines():
> +        if "List may be truncated" not in line:

Hi Simon,

Instead of filtering out this message, would it make sense to allow GDB
to list all possible completions without size limit?

This can be done with:

    cmd_output = gdb.execute("with max-completions unlimited -- complete " + text,
                             to_string=True)

I am not very familiar with the DAP protocol, but looking at it I did
not see a mechanism to limit the number of items returned, or a way for
GDB to let the client know that more options are available.  Is there a
way to do this?

Best,
Lancelot.

> +            result.append({"label": line, "type": "function", "length": len(text)})
> +    return {"targets": result}
> +
> +
> +@request("completions")
> +@capability("supportsCompletionsRequest")
> +@capability("completionTriggerCharacters", [" ", "."])
> +def completions(
> +    *,
> +    text: str,
> +    column: int,
> +    line: Optional[int] = None,
> +    frameId: Optional[int] = None,
> +    **extra
> +):
> +
> +    return send_gdb_with_response(lambda: _completions(text, column, line, frameId))
> -- 
> 2.41.0
>
  

> Hi Simon,
>
> Instead of filtering out this message, would it make sense to allow GDB
> to list all possible completions without size limit?
>
> This can be done with:
>
>      cmd_output = gdb.execute("with max-completions unlimited -- complete " + text,
>                               to_string=True)
>
> I am not very familiar with the DAP protocol, but looking at it I did
> not see a mechanism to limit the number of items returned, or a way for
> GDB to let the client know that more options are available.  Is there a
> way to do this?

That is a great idea, I had no idea that it could be done like that. 
Like you said, unfortunately (for some reason) a "max count"
is not provided by the request in the protocol.

I'll fix this in v2 and for good measure throw in tests for the request 
as well.

Thanks for your input!
  

>>>>> "Simon" == Simon Farre via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> Self-explanatory.

I think it's preferable to write something anyway.

Simon> +
Simon> +def _completions(text: str, column: int, line: Optional[int], frameId: Optional[int]):

This should be marked @in_gdb_thread.

Since 'line' isn't used, I think it can just be removed here and also
from 'completions' itself, as it is optional anyway.

Shouldn't 'column' be used?  Like perhaps the text should be truncated
at the given column.  (It's also fine to do this work in the main
thread.)

Simon> +def completions(
Simon> +    *,
Simon> +    text: str,
Simon> +    column: int,
Simon> +    line: Optional[int] = None,
Simon> +    frameId: Optional[int] = None,

I notice that frameId doesn't do anything either.  Since it's optional,
I suppose it could just be dropped here.  gdb doesn't really have this
concept, though we did add it for expression evaluation to support DAP,
so maybe it should also be added for completion.

Tom
  

> I think it's preferable to write something anyway.
I'll update next patch with descriptive commit message.
>
> Simon> +
> Simon> +def _completions(text: str, column: int, line: Optional[int], frameId: Optional[int]):
>
> This should be marked @in_gdb_thread.
>
> Since 'line' isn't used, I think it can just be removed here and also
> from 'completions' itself, as it is optional anyway.

Right, I added it to make it look 1-to-1 with the DAP spec, _even 
though_ we don't actually use them in GDB,
as I find this approach less confusing - if a GDB contributor (or user) 
were to look up the signature, then look at the DAP spec
and not see the same things. But I'll remove the things that doesn't 
make sense, as it were, in GDB-land in the next patch.

> Shouldn't 'column' be used?  Like perhaps the text should be truncated
> at the given column.  (It's also fine to do this work in the main
> thread.)
I tried doing that, and no matter how or what I did, VSCode ended up 
just vomitting out the wrong result. I'm not sure
if that's an error I made or VSCode not being exactly compliant with the 
spec. This patch was the one that finally made
the request work in VSCode. I'll have another look at it though.

Thanks!
  

>>>>> "Simon" == Simon Farre via Gdb-patches <gdb-patches@sourceware.org> writes:

>> Since 'line' isn't used, I think it can just be removed here and also
>> from 'completions' itself, as it is optional anyway.

Simon> Right, I added it to make it look 1-to-1 with the DAP spec, _even
Simon> though_ we don't actually use them in GDB,
Simon> as I find this approach less confusing - if a GDB contributor (or
Simon> user) were to look up the signature, then look at the DAP spec
Simon> and not see the same things. But I'll remove the things that doesn't
Simon> make sense, as it were, in GDB-land in the next patch.

For the DAP side (the @request) this is arguable but also fine.
I personally think it's clearer to only name parameters that are
actually used, but if you want to do it that way, I'll approve it.

On the gdb side (@in_gdb_thread) I think it's better to only accept
things that are actually used.  It's also arguably better here to do any
needed pre-processing in the DAP thread.

thanks,
Tom