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

Make ComrakExtensionOptions non-exhaustive #305

Merged

Conversation

CosmicHorrorDev
Copy link
Contributor

Resolves #303

Note: This is a breaking change, so it will require a breaking semver bump on next release

I would also be willing to add builders for constructing all parts of ComrakOptions which is a bit more ergonomic to construct things with. All intermediate states of constructing ComrakOptions are valid, so they can just be implemented with setters on the structs instead of requiring a separate type

@tgross35
Copy link
Contributor

I am not a maintainer (no connection to the project) but would you maybe consider adding it to all the config structs?

ComrakExtensionOptions
ComrakOptions
ComrakParseOptions
ComrakPlugins
ComrakRenderOptions
ComrakRenderPlugins

If there is to be a one-time breaking change for exhaustiveness, may as well apply it to everything that might want to add options in the future

@kivikakk
Copy link
Owner

If there is to be a one-time breaking change for exhaustiveness, may as well apply it to everything that might want to add options in the future

I agree. I'll rebase this shortly and add those before I merge.

@kivikakk kivikakk force-pushed the make-comrak-ext-options-non-exhaustive branch from aaaa634 to 4ddd159 Compare June 17, 2023 03:40
@kivikakk kivikakk linked an issue Jun 17, 2023 that may be closed by this pull request
@kivikakk
Copy link
Owner

I've decided to leave the top-level Options struct out of this change for now.

@kivikakk kivikakk merged commit 8934b49 into kivikakk:main Jun 17, 2023
@CosmicHorrorDev
Copy link
Contributor Author

@kivikakk are you fine with me adding builders for the non-exhaustive options to make them less verbose to construct?

@kivikakk
Copy link
Owner

I'm just rebasing @YJDoc2's #292 now! :)

@CosmicHorrorDev
Copy link
Contributor Author

Oh I somehow missed that 😅

Thanks for the great library btw!

@tgross35
Copy link
Contributor

Thanks for the great library btw!

Agreed, we apprecaite it!

@kivikakk
Copy link
Owner

Thanks both, that means a lot! The contributors list is getting longer — thanks for all your help. 😊

@digitalmoksha
Copy link
Collaborator

🤔 does this change require an update to the README?

@CosmicHorrorDev
Copy link
Contributor Author

What part do you think would need a change? I don't see any code examples that use options or plugins aside from passing Options::default()

@digitalmoksha
Copy link
Collaborator

Ah you’re right. I was thinking the calling interface was changing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants