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.

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

Add a Data map[string]any to store arbitrary command data #2085

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maxlandon
Copy link

@maxlandon maxlandon commented Dec 4, 2023

This PR aims to add an arbitrary map[string]any field to the Command type, so that consumer projects/plugins can store data pertaining to a given command, and for this data to share the same lifetime as the command itself.

An example related to #2019 and requested in #2083 is for completion plugins, which might need to store various completion data for commands, and in the case where the commands are being garbage-collected in a closed-loop console (such as this one, for this associated completion data to be correctly released along with the command itself.

The principle is the same as for command.Annotations field, except that the values in the key/value pairs can be anything. Equally, initializing the map is left at the consumers' responsibility.

@Luap99
Copy link
Contributor

Luap99 commented Dec 5, 2023

Shouldn't this be just a any? Making this map[string]any is already somewhat limiting depending on the use case. If this is just any you can still put in a map[string]any with that or if you do not need it you may directly store a value without having to go through the string map lookup.

@maxlandon
Copy link
Author

Hello @Luap99, thanks for the input.

If several plugins/paths need different objects stored with the command, who decides who has precedence ?

If plugin A wants a struct on monday, and then on tuesday plugin B wants another, then it can only delete Plugin A struct to replace it.

Now plugin A can't work, because plugin B replaced the contents altogether, and it has no way of communicating it to plugin A.

So I think there is no way around a map here...

@Luap99
Copy link
Contributor

Luap99 commented Dec 28, 2023

Yeah

Hello @Luap99, thanks for the input.

If several plugins/paths need different objects stored with the command, who decides who has precedence ?

If plugin A wants a struct on monday, and then on tuesday plugin B wants another, then it can only delete Plugin A struct to replace it.

Now plugin A can't work, because plugin B replaced the contents altogether, and it has no way of communicating it to plugin A.

So I think there is no way around a map here...

Ok yes that sounds like a valid reason for a map. I am not really sure how to common several plugins that need to store data attached to the command struct are but I guess it is better to be safe and use the map.

Maybe rethinking, is it really necessary to store the data inside the cobra command? Couldn't the "plugin" just define a wrapper type like

type PluginCommand struct {
    *cobra.Command
    data ...
}

and just accept this type in the plugin APIs?

Copy link
Collaborator

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

The extra field seems reasonable to me.
Please see one comment which will fix CI.

// Data are key/value pairs of arbitrary types that can be used by applications or "plugin" libraries
// that wish to store and use data associated with a given command. This data will thus share the same
// lifetime as the command itself.
Data map[string]any
Copy link
Collaborator

Choose a reason for hiding this comment

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

any was introduced in go 1.18.
Cobra still supports go 1.15. I know these are very old versions, but I am the opinion that if we don't need to drop support, why do it.

So, can we change any to interface{}?

@marckhouzam marckhouzam added this to the 1.9.0 milestone Dec 29, 2023
@marckhouzam marckhouzam added kind/feature A feature request for cobra; new or enhanced behavior area/cobra-command Core `cobra.Command` implementations labels Dec 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cobra-command Core `cobra.Command` implementations kind/feature A feature request for cobra; new or enhanced behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants