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

Pass a ScopeStack to tokens_to_classed_spans #337

Merged
merged 2 commits into from
Aug 1, 2021

Conversation

Keats
Copy link
Contributor

@Keats Keats commented Jun 22, 2021

Also remove deprecated functions

Also remove deprecated functions
src/html.rs Show resolved Hide resolved
@trishume
Copy link
Owner

This looks good, but want to pitch me on why to go with the major version bump rather than just renaming and then writing a tiny wrapper that passes an empty scopestack under the old name and marking it deprecated? It isn't that hard and it allows postponing a major version bump.

@Keats
Copy link
Contributor Author

Keats commented Jun 29, 2021

It would mean there is an accessible broken API but I can do that if you want.

@keith-hall
Copy link
Collaborator

I guess we are waiting for this before we make a new release? :)

@trishume trishume merged commit 6b36f5e into trishume:master Aug 1, 2021
@trishume
Copy link
Owner

trishume commented Aug 1, 2021

I reverted the removal of the deprecated methods myself and merged this in 77514b2

I prefer deprecation because I think it actually makes it more likely people will update to the less broken version. There's no notification for new major versions of your cargo dependencies, and you need to map breaking changes to required upgrades yourself, but if you or your users pull a new semver-compatible syntect then you get a deprecation message pointing directly at your code and telling you how to update it and why.

@trishume
Copy link
Owner

trishume commented Aug 1, 2021

This was released in v4.6.0

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.

3 participants