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

Change applyElement to call HTMLProcessor #60

Merged
merged 2 commits into from
May 12, 2022

Conversation

kojiishi
Copy link
Collaborator

This patch changes Parser.applyElement to call the HTMLProcessor class.

The HTMLProcessorOptions.separator is changed to accept a Node. This is because undefined had double meanings; i.e., use the default (ZWSP) and use the <wbr> element.

This patch changes `Parser.applyElement` to call the `HTMLProcessor` class.

The `HTMLProcessorOptions.separator` is changed to accept a `Node`. This is because `undefined` had double meanings; i.e., use the default (ZWSP) and use the `<wbr>` element.
@kojiishi
Copy link
Collaborator Author

@tushuhei Please feel free to request changes or reject if this is not desirable as we discussed.

This patch tries not to change the arguments and the separator of applyElement to keep the backward compat. I can go any ways if you prefer to change.

@kojiishi kojiishi requested a review from tushuhei April 30, 2022 07:15
Copy link
Member

@tushuhei tushuhei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR! I just put some nit comments.

}
};
processChildren(parentElement);
const html_processor = new HTMLProcessor(this, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit. Let's rename this to htmlProcessor just for naming consistency :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, sorry I was using wrong styleguide ;-P


```javascript
import { HTMLProcessor } from 'budoux';
const ele = document.querySelector('p.budou-this');
const applier = new HTMLProcessor(parser);
applier.applyToElement(ele);
const html_processor = new HTMLProcessor(parser, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@tushuhei
Copy link
Member

Thanks for the fix!

@tushuhei tushuhei merged commit fcc9156 into google:main May 12, 2022
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.

2 participants