Introduce interactive clc mode [API-689] #32
Conversation
commands/root.go
Outdated
| if config := internal.Configuration; config != nil { | ||
| return fmt.Sprintf("hzc %s@%s> ", config.Cluster.Network.Addresses[0], config.Cluster.Name), true | ||
| } | ||
| return "hzc address@clusterName> ", true |
There was a problem hiding this comment.
Is it probable that the configuration is nil? I think when this function is called, we should have already connected to the cluster, shouldn't we?
There was a problem hiding this comment.
Thank you, fixed as suggested
- try to connect on start, fail fast if not successful - provide longer versions of flags as suggestion
… flags - add timeout to map operations
- add shell style parsing for interactive mode to work properly on inputs such as map put -n my-map -k "hello world" -v '{"json": 1}'
utku-caglayan
force-pushed
the
interactive-clc
branch
from
60e5756
to
d02ccc6
Compare
commands/root.go
Outdated
| cmd.PersistentFlags().StringVar(&token, "cloud-token", "", "your Hazelcast Cloud token.") | ||
| cmd.PersistentFlags().StringVarP(&internal.CfgFile, "config", "c", internal.DefautConfigPath(), fmt.Sprintf("config file (default is %s)", internal.DefautConfigPath())) | ||
| cmd.PersistentFlags().StringVarP(&internal.Address, "address", "a", "", "addresses of the instances in the cluster.") | ||
| cmd.PersistentFlags().StringVarP(&internal.Cluster, "cluster-name", "n", "", "name of the cluster that contains the instances.") |
There was a problem hiding this comment.
Don't we use -n for the map name?
There was a problem hiding this comment.
Yes correct, I removed the shorthand for this.
| // DynamicSuggestionsFunc will be executed if a command has CallbackAnnotation as an annotation. If it's included | ||
| // the value will be provided to the DynamicSuggestionsFunc function. |
There was a problem hiding this comment.
How about not-breaking comment lines, and making each comment line its own sentence?
internal/cobraprompt/prompt.go
Outdated
| if co.RootCmd == nil { | ||
| panic("RootCmd is not set. Please set RootCmd") | ||
| } | ||
|
|
There was a problem hiding this comment.
Let's try to use blank lines sparingly. Maybe we could use that space for a comment that explains the line below?
internal/cobraprompt/prompt.go
Outdated
| } | ||
|
|
||
| co.prepare() | ||
|
|
There was a problem hiding this comment.
Let's try to use blank lines sparingly. Maybe we could use that space for a comment that explains the code below?
commands/root.go
Outdated
| DisableCompletionCommand: true, | ||
| AddDefaultExitCommand: true, | ||
| GoPromptOptions: []prompt.Option{ | ||
| prompt.OptionTitle("hazelcast commandline client"), |
There was a problem hiding this comment.
Is this title displayed as the title of the terminal window? How about: "Hazelcast Commandline Client" ?
There was a problem hiding this comment.
Yes it is, sure that makes sense. I would also prefer a shorter name since there are no guarantees that we will have sufficient space to display the whole title. What do you think of "Hazelcast Cli"?
There was a problem hiding this comment.
How about "Hazelcast Client"?
commands/root.go
Outdated
| RootCmd.Example = "> map put -k key -m myMap -v someValue\n" + | ||
| "> map get -k key -m myMap\n" + | ||
| "> cluster version" |
There was a problem hiding this comment.
Instead of concatenating strings and adding line breaks, backticks (````) may be used.
There was a problem hiding this comment.
Great catch, thanks
|
|
||
| func findSuggestions(co *CobraPrompt, d *prompt.Document) []prompt.Suggest { | ||
| command := co.RootCmd | ||
| args := strings.Fields(d.CurrentLine()) |
There was a problem hiding this comment.
Will this cause a problem with args which have spaces?
There was a problem hiding this comment.
Great catch, thanks. Fixed it with an external lib
internal/cobraprompt/prompt.go
Outdated
| command = found | ||
| } | ||
| var suggestions []prompt.Suggest | ||
| persistFlagValues, _ := command.Flags().GetBool(PersistFlagValuesFlag) |
There was a problem hiding this comment.
The error is ignored here. Is that intended?
There was a problem hiding this comment.
I tried to add a workaround, because initiation of commands etc.. are a mess because of the global state. Changes are on 28bd268. I will keep this in mind for a refactor session if that is okay with you too.
No description provided.