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

Support multiple interceptors together #1405

Closed
aa-dit-yuh opened this issue Dec 15, 2023 · 5 comments
Closed

Support multiple interceptors together #1405

aa-dit-yuh opened this issue Dec 15, 2023 · 5 comments

Comments

@aa-dit-yuh
Copy link

aa-dit-yuh commented Dec 15, 2023

Is your feature request related to a problem? Please describe.
Add support for invoking two or more interceptors.

Describe the solution you'd like
Replace SetInterceptor on IConfigurator with AddInterceptor method that allows chaining of interceptors.

Describe alternatives you've considered
Today, one can create the class ChainedInterceptor that delegates to multiple interceptors to circumvent this feature gap.

class ChainedInterceptor(params ICommandInterceptor[] interceptors) : ICommandInterceptor
{
    public void Intercept(CommandContext context, CommandSettings settings)
    {
        foreach (var interceptor in interceptors)
        {
            interceptor.Intercept(context, settings);
        }
    }
}

I'm interested in having library support for interceptor chaining.

Additional context

@aa-dit-yuh
Copy link
Author

I'll be happy to contribute code to close this feature request. However, I'm not sure how to achieve this feature request without causing backward-incompatible changes to either the IConfigurator or the ICommandAppSettings interface.

@patriksvensson
Copy link
Contributor

Wouldn't it be better if we allowed multiple interceptors to be registered instead of a single one?

@aa-dit-yuh
Copy link
Author

Yes, that's the intention of the feature request too. The SetInterceptor method can be deprecated in favour of an AddInterceptor method on the IConfigurator interface IMO.

@nils-a
Copy link
Contributor

nils-a commented Dec 23, 2023

@aditnryn would #1412 solve your problem?

@FrankRay78
Copy link
Contributor

Fixed by #1412.

@github-project-automation github-project-automation bot moved this from Todo 🕑 to Done 🚀 in Spectre Console Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done 🚀
Development

No branches or pull requests

4 participants