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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change PluginCommand API to be more like Command #12279

Merged

Conversation

devyn
Copy link
Contributor

@devyn devyn commented Mar 25, 2024

Description

This is something that was discussed in the core team meeting last Wednesday. @ayax79 is building nu-plugin-polars with all of the dataframe commands into a plugin, and there are a lot of them, so it would help to make the API more similar. At the same time, I think the Command API is just better anyway. I don't think the difference is justified, and the types for core commands have the benefit of requiring less .into() because they often don't own their data

  • Broke signature() up into name(), usage(), extra_usage(), search_terms(), examples()
  • signature() returns nu_protocol::Signature
  • examples() returns Vec<nu_protocol::Example>
  • PluginSignature and PluginExample no longer need to be used by plugin developers

User-Facing Changes

Breaking API for plugins yet again 馃槃

Tests + Formatting

  • 馃煝 toolkit fmt
  • 馃煝 toolkit clippy
  • 馃煝 toolkit test
  • 馃煝 toolkit test stdlib

After Submitting

  • Change book documentation to reflect
  • Make announcement on Discord

@ayax79
Copy link
Contributor

ayax79 commented Mar 25, 2024

Looks great!

@fdncred
Copy link
Collaborator

fdncred commented Mar 25, 2024

This makes the plugins look pretty familiar. Thanks!

@fdncred fdncred added pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes pr:api-change This PR should be mentioned in #api-updates channel on Discord labels Mar 25, 2024
@fdncred
Copy link
Collaborator

fdncred commented Mar 27, 2024

@devyn i'm thinking we should probably land this if you can fix the conflicts.

- Broke `signature()` up into `name()`, `usage()`, `extra_usage()`,
  `search_terms()`, `examples()`
- `signature()` returns `nu_protocol::Signature`
- `examples()` returns `Vec<nu_protocol::Example>`
- `PluginSignature` and `PluginExample` no longer need to be used by
   plugin developers

This should help with splitting core commands into plugins, but also
offers a nicer interface in my opinion.
@devyn devyn force-pushed the make-plugincommand-more-like-command branch from 7d3d126 to 7187425 Compare March 27, 2024 06:14
@devyn
Copy link
Contributor Author

devyn commented Mar 27, 2024

I'm always creating my own conflicts, aren't I 馃ぃ

Copy link
Member

@sholderbach sholderbach left a comment

Choose a reason for hiding this comment

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

Let's land this to save @ayax79 some pain

@sholderbach sholderbach merged commit 01d30a4 into nushell:main Mar 27, 2024
15 checks passed
@hustcer hustcer added this to the v0.92.0 milestone Mar 27, 2024
sholderbach pushed a commit that referenced this pull request Mar 28, 2024
# Description

I broke this, I think in #12279, because I forgot a `#[cfg(plugin)]`
@fdncred fdncred added the pr:plugins This PR is related to plugins label Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:api-change This PR should be mentioned in #api-updates channel on Discord pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes pr:plugins This PR is related to plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants