Skip to content

Conversation

@byte2pixel
Copy link
Contributor

@byte2pixel byte2pixel commented Feb 19, 2025

Fixes #1219

  • I have read the Contribution Guidelines
  • I have commented on the issue above and discussed the intended changes
  • A maintainer has signed off on the changes and the issue was assigned to me
    • see above link to discussion.
  • All newly added code is adequately covered by tests
  • All existing tests are still running without errors
  • The documentation was modified to reflect the changes OR no documentation changes are required.

Changes

Changed IConfigurator to return IConfigurator instead of void for the following interface methods:

  • SetHelpProvider
  • SetHelpProvider<T>
  • AddExample

However, other methods like AddCommand narrow the configurator so the order of chaining methods does matter. Just to point that out. Chaining is optional obviously.

I saw the ICofiguratorOfT also returns void for SetDescription, and AddExample. However, it becomes a bigger challenge to change that because Configurator<T> implements both IUnsafeBranchConfigurator and IConfigurator<T> and both share the SetDescription and AddExample but one does not take in the generic type. Probably would need "unsafe configurator of T" or something?


Please upvote 👍 this pull request if you are interested in it.

- SetHelpProvider
- SetHelpProvider<T>
- AddExample

However, other methods like AddCommand narrow the configurator so order ends up making a difference.
@FrankRay78
Copy link
Contributor

Thanks, I'll review this shortly.

@FrankRay78 FrankRay78 merged commit 93668e9 into spectreconsole:main Feb 24, 2025
3 checks passed
@FrankRay78
Copy link
Contributor

Many thanks @byte2pixel, much appreciated and welcome to the git commit history.

@byte2pixel
Copy link
Contributor Author

byte2pixel commented Feb 25, 2025

Many thanks @byte2pixel, much appreciated and welcome to the git commit history.

@FrankRay78 Thanks, hopefully more to come. Hitting a busy patch at work with adding an embedded db to an application.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Change the IConfigurator Extension Methods to all return IConfigurator.

2 participants