Consider killing the TreeBuilderActions trait and its ilk

[nox]

They are only there for namespacing reasons.

[Ygg01]

What changes does this entail? Just moving methods from TreeBuilderActions/TreeBuilderStep to TreeBuilder, correct?

[nox]

Yep.

[Ygg01]

This task seems trivial. Do you guys want to nominate this for Servo starter tasks? Or is html5ever outside of scope for such tasks?

PS. Whoever fixes TreeBuilderActions, could also fix xml5ever, as well.

[nox]

Yes this should be made into an E-Easy issue.

[jdm]

Currently, these traits are only used for namespacing (see servo/html5ever#258).

This task entails:

* removing traits TreeBuilderActions/TreeBuilderStep/XmlTreeBuilderActions/XmlTreeBuilderStep
* moving implementations of above traits into TreeBuilder/XmlTreeBuilder structs.
* fixing any breaks this causes in project.

I think this could make a great first issue, for new comers.

[Januson]

Hello. I would like to contribute. May I take this issue?

[jdm]

@Januson Please do! Ask questions if anything is unclear.

[Januson]

Thank you. I have a question about visibility. If I understand current code correctly, methods from TreeBuilderStep and TreeBuilderActions traits are private to the tree_builder mod.

First idea how to get gid of these traits was to just copy them to TreeBuilder. But I figured that is a bad idea since tree_builder/mod.rs would grow past 3k lines. So keeping them separate I run into problem with visibility as private impl method in actions.rs is not visible in tree_builder/mod.rs but public method is visible everywhere.

Is there any other way? Or am I missing something? So far I found only a SO question and this issue.

[Ygg01]

@Januson I don't think having a file past 3k lines is an issue, in itself? Maybe @nox could weigh in?

[Januson]

Hello again. Ihave another question. I moved most of the required methods but I am struggling with methods from TreeBuilderStep. I can't figure out how to use match_token! macro outside of tree_builder/rules.rs. It is somehow generated in macros/match_token.rs and build.rs. How can I make it visible for methods in tree_builder/mod.rs?

[SimonSapin]

That sounds tricky. Can you move the relevant code into rules.rs? Or maybe a new module under tree_builder/?

[SimonSapin]

If want to go with the latter, add a new call to expand_match_tokens in build.rs, and in src/tree_builder/mod.rs instead of mod foo; for the new module use include! like for rules.rs.

[Januson]

Thank you for suggestion. I thought about splitting it into methods, so that functionality using match_token macro stays in rules module. But I think that would be too cumbersome splitting and I fear it would be beyond my current Rust skills. Other thing I thought of was to extend TreeBuilder with the methods of TreeBuilderStep but that would make those methods visible outside of TreeBuilder. I don't know if this is a problem. I don't know how to proceed with this last trait.

[jbg]

I made a pull request implementing these changes, feedback would be appreciated!

[Ygg01]

@jbg Congrats! Your change is implemented and this bug can be closed :D