ArcsinX wrote:
> The reason I haven't changed the result of `shouldRunCompletion` is twofold:
>
> 1. If we return false, we'll just reply with empty result as you noticed,
> logging `vlog("ignored auto-triggered completion, preceding char did not
> match");`. I'm not sure this is the intended behaviour of the API - I assumed
> passing invalid location should be reported to the client as error as it
> shouldn't really happen. Unless we change the signature of
> `shouldRunCompletion` we can't really trigger error response from there.
> 2. There's already precedent for letting the completion code handle error
> reporting in the line before (the `getDraft` call can return nullptr - we
> still return `true` there knowing the completion code will return error to
> the client).
>
> Would we rather prefer silently (from client's perspective) ignoring the
> invalid position?
Sorry, maybe I wasn't clear enough.
I don't see any difference in behavior if we return `false` in
`shouldRunCompletion`. We'll still return
```json
{
"isIncomplete": false,
"items": []
}
```
Just like if we returned `true` from `shouldRunCompletion`.
So, from LSP standpoint we don't notify the client of an error, we just return
an empty result.
The only difference is in the log output. Therefore, I suggest returning
`false` and replacing `vlog` with `elog`. Then, from a diagnostic standpoint,
everything will remain the same, but we return empty result earlier.
https://github.com/llvm/llvm-project/pull/196112
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
