-
Notifications
You must be signed in to change notification settings - Fork 122
Make the useful of suggestion in node console from services. #1643
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.
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
base: nightly
Are you sure you want to change the base?
Conversation
// fix the suggestion of greedy | ||
var lastWhitespace = commandInput.input().lastIndexOf(' '); | ||
if (lastWhitespace != -1) { | ||
commandInput.cursor(lastWhitespace); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's worth mentioning that this is a potentially destructive fix.
actually, I've tried setting the cursor
in the Parser
, but it obviously doesn't work for the Greedy
argument, because CommandTree#L841(Incendo/cloud) restores the cursor
modified in the Parser
.
at least in my opinion, there is no better way to solve it properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The biggest issue I have with the current implementation is that this introduces a dependency of the node on the bridge, which might not be installed. I think it would be better if the node provides an interface that can be implemented by modules and resolves it using the service registry. This way a module can hook into the node rather than the node hooking into the bridge.
@@ -411,6 +412,19 @@ public void restart() { | |||
this.updateLifecycle(ServiceLifeCycle.RUNNING); | |||
} | |||
|
|||
@Override | |||
public Collection<String> consoleSuggestion(@NonNull String line) { | |||
var listPropertyType = TypeFactory.parameterizedClass(List.class, String.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this method should return a future instead so that the caller can better handle errors and apply timeouts.
Motivation
As the title says, it enables node console to directly get suggestions from the service, especially when you need to use cloudnet to debug the service.
The java sub-process started by cloudnet does not have a complete terminal environment, so it cannot directly get the service suggestions. Therefore, it can only communicate with the bridge through
ChannelMessage
to get suggestions.Modification
ServiceCommand#suggestCommands
SpecificCloudServiceProvider#consoleSuggestion
in wrapper, Implementation inAbstractService
to request suggestions frombridge
.@Greedy
processes inDefaultSuggestionProcessor
Service Platfrom Implementations
Result
Perfect. Like these:
Other context
Once merged, module-rest will be directly compatible with this change, further enhancing the web-interface console of service.