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

Replace "ast" gem with custom implementation #123

Open
Forthoney opened this issue Nov 11, 2023 · 3 comments
Open

Replace "ast" gem with custom implementation #123

Forthoney opened this issue Nov 11, 2023 · 3 comments

Comments

@Forthoney
Copy link

Forthoney commented Nov 11, 2023

The "ast" gem, a core dependency of better-html, is no longer actively maintained, with its most recent non-trivial commit happening nearly 3 years ago. Having crucial functionality of the gem be dependent on "ast" is unsustainable at best and dangerous at worst. Therefore, better-html should ideally move towards making its own AST class. It frankly does not utilize much of the features from the "ast" gem anyways, so writing a tailormade AST class for better-html should not be too challenging.

Tasks

Preview Give feedback
No tasks being tracked yet.
@vinistock
Copy link
Member

Indeed, having a handcrafted AST will be more robust, organized and will allow for additional functionality to be built on top of better-html. We'd gladly accept a contribution!

@Forthoney
Copy link
Author

Forthoney commented Nov 30, 2023

@vinistock
A bit of bad news with a silver lining.
Turns out the rewriting the AST class is not so simple because it is tightly coupled with the parser in two aspects. First is via the tokenizer of better-html. The second link through Whitequark's Parser that is used in small parts of the gem. I believe it is only used to provide test helpers for the gem users, but it is used nonetheless, and we must accomodate it at the moment.

My proposal is that this issue should be addressed as part of a larger refactor/rewrite that replaces Whitequark's parser with Prism. I understand that the Prism team is interested in finding gems that use non-canonical parsers, so I hope this information is of use to them.

@vinistock
Copy link
Member

Agreed, we should definitely switch to using Prism. It might be a larger refactor, but if you're interested in taking a stab at it, I believe that's the right path forward.

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

No branches or pull requests

2 participants