Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add signatureHelp and hover to Python autocomplete #3607

Merged
merged 19 commits into from
Mar 31, 2023
Merged

Add signatureHelp and hover to Python autocomplete #3607

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
31cb6a4
Update proto file for more LSP requests
mattrunyon Mar 14, 2023
8a828cc
Adjust protobuf to not be breaking
mattrunyon Mar 15, 2023
13ed808
Promote more fields to top AutoCompleteRequest type
mattrunyon Mar 15, 2023
edaa7ba
Generated gwt bindings
niloc132 Mar 16, 2023
6bdfa2b
Add cancel, diagnostic, hover to proto
mattrunyon Mar 21, 2023
73f0c3b
regenerated gwt bindings
niloc132 Mar 22, 2023
cd06bc4
Proto changes for cancel token and diagnostics
mattrunyon Mar 27, 2023
405e3fe
Regenerate gwt bindings
niloc132 Mar 27, 2023
af65bda
Generate AuthWiring for CancelAutoComplete
nbauernfeind Mar 27, 2023
b715e02
Add signature help and hover support to python
mattrunyon Mar 27, 2023
ad47e89
Spotless
mattrunyon Mar 27, 2023
c0fb064
Update CPP Protobuf
nbauernfeind Mar 27, 2023
671fb45
Update Python Protobuf
nbauernfeind Mar 27, 2023
84a57c4
Update Go Protobuf
nbauernfeind Mar 27, 2023
2729e20
Address review comments and pull in Nate proto generations
mattrunyon Mar 28, 2023
7478368
Revert to throwable in python observer
mattrunyon Mar 28, 2023
5ab3b2e
Update Go Protobuf per Comment Change
nbauernfeind Mar 28, 2023
0b7ac8c
Address review comment and filter path completions
mattrunyon Mar 31, 2023
f24449d
Address Jinafeng comments
mattrunyon Mar 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions authorization/src/main/java/io/deephaven/auth/codegen/impl/ConsoleServiceAuthWiring.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import io.deephaven.auth.ServiceAuthWiring;
import io.deephaven.proto.backplane.script.grpc.AutoCompleteRequest;
import io.deephaven.proto.backplane.script.grpc.BindTableToVariableRequest;
import io.deephaven.proto.backplane.script.grpc.CancelAutoCompleteRequest;
import io.deephaven.proto.backplane.script.grpc.CancelCommandRequest;
import io.deephaven.proto.backplane.script.grpc.ConsoleServiceGrpc;
import io.deephaven.proto.backplane.script.grpc.ExecuteCommandRequest;
Expand Down Expand Up @@ -52,6 +53,8 @@ default ServerServiceDefinition intercept(ConsoleServiceGrpc.ConsoleServiceImplB
serviceBuilder.addMethod(ServiceAuthWiring.intercept(
service, "AutoCompleteStream", this::onCallStartedAutoCompleteStream,
this::onMessageReceivedAutoCompleteStream));
serviceBuilder.addMethod(ServiceAuthWiring.intercept(
service, "CancelAutoComplete", null, this::onMessageReceivedCancelAutoComplete));
serviceBuilder.addMethod(ServiceAuthWiring.intercept(
service, "OpenAutoCompleteStream", null, this::onMessageReceivedOpenAutoCompleteStream));
serviceBuilder.addMethod(ServiceAuthWiring.intercept(
Expand Down Expand Up @@ -141,6 +144,16 @@ void onMessageReceivedBindTableToVariable(AuthContext authContext,
*/
void onMessageReceivedAutoCompleteStream(AuthContext authContext, AutoCompleteRequest request);

/**
* Authorize a request to CancelAutoComplete.
*
* @param authContext the authentication context of the request
* @param request the request to authorize
* @throws io.grpc.StatusRuntimeException if the user is not authorized to invoke CancelAutoComplete
*/
void onMessageReceivedCancelAutoComplete(AuthContext authContext,
CancelAutoCompleteRequest request);

/**
* Authorize a request to OpenAutoCompleteStream.
*
Expand Down Expand Up @@ -187,6 +200,9 @@ public void onCallStartedAutoCompleteStream(AuthContext authContext) {}
public void onMessageReceivedAutoCompleteStream(AuthContext authContext,
AutoCompleteRequest request) {}

public void onMessageReceivedCancelAutoComplete(AuthContext authContext,
CancelAutoCompleteRequest request) {}

public void onMessageReceivedOpenAutoCompleteStream(AuthContext authContext,
AutoCompleteRequest request) {}

Expand Down Expand Up @@ -238,6 +254,11 @@ public void onMessageReceivedAutoCompleteStream(AuthContext authContext,
ServiceAuthWiring.operationNotAllowed();
}

public void onMessageReceivedCancelAutoComplete(AuthContext authContext,
CancelAutoCompleteRequest request) {
ServiceAuthWiring.operationNotAllowed();
}

public void onMessageReceivedOpenAutoCompleteStream(AuthContext authContext,
AutoCompleteRequest request) {
ServiceAuthWiring.operationNotAllowed();
Expand Down Expand Up @@ -313,6 +334,13 @@ public void onMessageReceivedAutoCompleteStream(AuthContext authContext,
}
}

public void onMessageReceivedCancelAutoComplete(AuthContext authContext,
CancelAutoCompleteRequest request) {
if (delegate != null) {
delegate.onMessageReceivedCancelAutoComplete(authContext, request);
}
}

public void onMessageReceivedOpenAutoCompleteStream(AuthContext authContext,
AutoCompleteRequest request) {
if (delegate != null) {
Expand Down
142 changes: 132 additions & 10 deletions proto/proto-backplane-grpc/src/main/proto/deephaven/proto/console.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ service ConsoleService {
* time.
*/
rpc AutoCompleteStream(stream AutoCompleteRequest) returns (stream AutoCompleteResponse) {}
rpc CancelAutoComplete(CancelAutoCompleteRequest) returns (CancelAutoCompleteResponse) {}

/*
* Half of the browser-based (browser's can't do bidirectional streams without websockets)
Expand Down Expand Up @@ -120,7 +121,18 @@ message CancelCommandResponse {

}

message CancelAutoCompleteRequest {
io.deephaven.proto.backplane.grpc.Ticket console_id = 1;
int32 request_id = 2;
}

message CancelAutoCompleteResponse {

}

message AutoCompleteRequest {
io.deephaven.proto.backplane.grpc.Ticket console_id = 5;
int32 request_id = 6;
oneof request {
// Starts a document in a given console - to end, just close the stream, the server will hang up right away
OpenDocumentRequest open_document = 1;
Expand All @@ -131,21 +143,36 @@ message AutoCompleteRequest {
// Requests that a response be sent back with completion items
GetCompletionItemsRequest get_completion_items = 3;

// Request for help about the method signature at the cursor
GetSignatureHelpRequest get_signature_help = 7;

// Request for help about what the user is hovering over
GetHoverRequest get_hover = 8;

// Request to perform file diagnostics
GetDiagnosticRequest get_diagnostic = 9;

// Closes the document, indicating that it will not be referenced again
CloseDocumentRequest close_document = 4;
}
}
message AutoCompleteResponse {
int32 request_id = 2;
bool success = 3;
oneof response {
GetCompletionItemsResponse completion_items = 1;
GetSignatureHelpResponse signatures = 4;
mattrunyon marked this conversation as resolved.
Show resolved Hide resolved
GetHoverResponse hover = 5;
GetPullDiagnosticResponse diagnostic = 6;
GetPublishDiagnosticResponse diagnostic_publish = 7;
}
}

message BrowserNextResponse {
}

message OpenDocumentRequest {
io.deephaven.proto.backplane.grpc.Ticket console_id = 1;
io.deephaven.proto.backplane.grpc.Ticket console_id = 1 [deprecated=true];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have a server safety layer that ensures if the deprecated fields and new fields are set, they are equal; otherwise blowup? Alternatively, we could say either the new fields must be set, or the deprecated fields must be set, but not both.

Copy link
Contributor Author

@mattrunyon mattrunyon Mar 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only reason for deprecation here (as opposed to just ripping out these fields) is if a user had an older web client but newer engine version. Realistically I think the only place that may happen right now is with web developers, so I would be fine just removing these fields instead of deprecating, but I know @niloc132 had other thoughts on that

Although one main reason to not rip this out would be in DnD where at the moment we might have the enterprise JS API talking to the community engine still

TextDocumentItem text_document = 2;
}
message TextDocumentItem {
Expand All @@ -156,13 +183,13 @@ message TextDocumentItem {
}

message CloseDocumentRequest {
io.deephaven.proto.backplane.grpc.Ticket console_id = 1;
VersionedTextDocumentIdentifier text_document = 2;//TODO actually just uri?
io.deephaven.proto.backplane.grpc.Ticket console_id = 1 [deprecated=true];
VersionedTextDocumentIdentifier text_document = 2;
}

message ChangeDocumentRequest {
io.deephaven.proto.backplane.grpc.Ticket console_id = 1;
VersionedTextDocumentIdentifier text_document = 2;//TODO actually just uri?
io.deephaven.proto.backplane.grpc.Ticket console_id = 1 [deprecated=true];
VersionedTextDocumentIdentifier text_document = 2;
repeated TextDocumentContentChangeEvent content_changes = 3;

message TextDocumentContentChangeEvent {
Expand All @@ -184,14 +211,19 @@ message Position {
int32 character = 2;
}

message MarkupContent {
string kind = 1;
string value = 2;
}

message GetCompletionItemsRequest {
io.deephaven.proto.backplane.grpc.Ticket console_id = 1;
io.deephaven.proto.backplane.grpc.Ticket console_id = 1 [deprecated=true];

CompletionContext context = 2;
VersionedTextDocumentIdentifier text_document = 3;
Position position = 4;

int32 request_id = 5;
int32 request_id = 5 [deprecated=true];
}
message CompletionContext {
int32 trigger_kind = 1;
Expand All @@ -200,16 +232,18 @@ message CompletionContext {
message GetCompletionItemsResponse {
repeated CompletionItem items = 1;

int32 request_id = 2;
bool success = 3;
// These fields are maintained for backwards compatibility
// Use the same fields on AutoCompleteResponse instead
mattrunyon marked this conversation as resolved.
Show resolved Hide resolved
int32 request_id = 2 [deprecated=true];
bool success = 3 [deprecated=true];
}
message CompletionItem {
int32 start = 1;
int32 length = 2;
string label = 3;
int32 kind = 4;
string detail = 5;
string documentation = 6;
reserved 6; // Old documentation as a string. Was never used by us
bool deprecated = 7;
bool preselect = 8;
TextEdit text_edit = 9;
Expand All @@ -218,12 +252,100 @@ message CompletionItem {
int32 insert_text_format = 12;
repeated TextEdit additional_text_edits = 13;
repeated string commit_characters = 14;
MarkupContent documentation = 15;
}
message TextEdit {
DocumentRange range = 1;
string text = 2;
}

message GetSignatureHelpRequest {
SignatureHelpContext context = 1;
VersionedTextDocumentIdentifier text_document = 2;
Position position = 3;
}
message SignatureHelpContext {
int32 trigger_kind = 1;
optional string trigger_character = 2;
bool is_retrigger = 3;
GetSignatureHelpResponse active_signature_help = 4;
}

message GetSignatureHelpResponse {
repeated SignatureInformation signatures = 1;
optional int32 active_signature = 2;
optional int32 active_parameter = 3;
}

message SignatureInformation {
string label = 1;
MarkupContent documentation = 2;
repeated ParameterInformation parameters = 3;
optional int32 active_parameter = 4;
}

message ParameterInformation {
string label = 1;
MarkupContent documentation = 2;
}

message GetHoverRequest {
VersionedTextDocumentIdentifier text_document = 1;
Position position = 2;
}

message GetHoverResponse {
MarkupContent contents = 1;
DocumentRange range = 2;
}

message GetDiagnosticRequest {
VersionedTextDocumentIdentifier text_document = 1;
optional string identifier = 2;
optional string previous_result_id = 3;
}

message GetPullDiagnosticResponse {
string kind = 1;
optional string result_id = 2;
repeated Diagnostic items = 3;
}

message GetPublishDiagnosticResponse {
string uri = 1;
optional int32 version = 2;
repeated Diagnostic diagnostics = 3;
}

message Diagnostic {
enum DiagnosticSeverity {
NOT_SET_SEVERITY = 0;
ERROR = 1;
WARNING = 2;
INFORMATION = 3;
HINT = 4;
}

enum DiagnosticTag {
NOT_SET_TAG = 0;
UNNECESSARY = 1;
DEPRECATED = 2;
}

message CodeDescription {
string href = 1;
}

DocumentRange range = 1;
DiagnosticSeverity severity = 2;
optional string code = 3;
optional CodeDescription code_description = 4;
optional string source = 5;
string message = 6;
repeated DiagnosticTag tags = 7;
optional bytes data = 9;
}

message FigureDescriptor {
optional string title = 1;
string title_font = 2;
Expand Down
Loading