Reduce copy/paste code in HTMLParser using some template functions
Created attachment 56675 [details] Patch
I used awk, sort, and some search-replace to write this patch, so I'm confident that it's functionally correct. (I also ran all the layout tests.) The real question in review is if this code is now clearer to folks. If it is, great. If not, I'm happy to chuck the patch. The copy/paste code bothered me when looking at how much of HTMLParser could be re-used by HTML5Parser, so I went through and did this cleanup.
Comment on attachment 56675 [details] Patch Ok. I'm not 100% convinced this is better, but it looks ok.
Comment on attachment 56675 [details] Patch I'm certainly interested in any feedback from others. This will sit in the cq for a while I'm sure... :)
Btw, I got hyatt's feedback in the channel. He said something to the effect of it "looked slightly better" (that is not a direct quote).
Comment on attachment 56675 [details] Patch I think it's clearer. A few nits, though (I'm not a reviewer so I won't touch the review flag). > +template< size_t ArraySize > > +static void addTags(TagNameSet& set, QualifiedName (&names)[ArraySize]) Can we get a comment with an explanation of this syntax or a link or something? I just looked at this so I know how to read the second declaration but by next week I will probably forget and have to spend a while pondering what the parens signify here. > +{ > + for (size_t x = 0; x < ArraySize; x++) { x is an odd choice for a loop index. Why not i? > + const QualifiedName& name = names[x]; I don't think the temp var here adds very much > + set.add(name.localName().impl()); > + } > > +static void mapTagToFunc(FunctionMap& map, const QualifiedName& tag, CreateErrorCheckFunc func) > +{ > + map.set(tag.localName().impl(), func); > +} > + > +template< size_t ArraySize > > +static void mapTagsToFunc(FunctionMap& map, QualifiedName (&names)[ArraySize], CreateErrorCheckFunc func) > +{ > + for (size_t x = 0; x < ArraySize; x++) { > + const QualifiedName& name = names[x]; Same comments as above (x is unusual for a loop index, temp is of iffy value). > + mapTagToFunc(map, name, func); > + } > +} > + > PassRefPtr<Node> HTMLParser::getNode(Token* t) > { > // Init our error handling table.
Comment on attachment 56675 [details] Patch Clearing flags on attachment: 56675 Committed r60031: <http://trac.webkit.org/changeset/60031>
All reviewed patches have been landed. Closing bug.