cppalliance
feat: tagfiles generation for cross-referencing in doxygen format
However, we will still have a problem with the USR from Krystian's commit blocking CI.
I want to start by comparing the XML in the artifacts. Do you have the result for Boost.URL multipage? That's the one we can't risk breaking because it's already in production. It has to be compatible with the existing one.
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
The implementation of the tagfile write is excellent.
CI failed after I rebased. That was a good thing because it helped me catch a bug.
So, the reason CI is failing is trivial. The TestRunner calls if(auto err = gen_->buildOneString(generatedDocs, **corpus)), which calls Generator::buildOneString, where you changed the call to buildOne to buildOne(ss, corpus, "");. We could fix CI by just adapting AdocGenerator::buildOne to not generate the tagfile when this string is empty. However, this doesn't address the deeper issue here.
The real issue causing all of that is the API design. Your new fileName parameter in Generator::buildOne is breaking the contract of this abstract class.
- First of all, the new parameter is documented as
@param fileName The file name to use for the output., but that's not what the implementation is doing. The filename is the filename for the tagfile, which is unrelated to the filename of the output. The parameter isn't even used in theXMLGenerator. It's even more confusing becausefileNameis not the filename of the tagfile but the basename to which "tag.xml" will be appended to form the tagfile filename. - Most importantly, the contract of
Generator::buildOneis that it renders the documentation to any output stream regardless of the filename because (i) there's not even a filename for most output types and (ii) if there were a file for each stream, the stream would become redundant. For instance, tests usebuildOneString, which outputs the result to a string.
This bug was already there, but CI only made it evident. It worked before rebasing because Generator::buildOneString wasn't being covered with the AdocGenerator.
Because of how Generator expects the output to be decoupled from any filename and buildOne is based on streaming only that result to the output, the Generator should have received new virtual functions to buildTagfile(std::ostream& os, Corpus const& corpus) or something instead of buildOne trying to fill this role.
If this is hard to achieve because we can't decouple the logic of buildOne and buildTagfile, a second-best would be buildOne(std::ostream& os, Corpus const& corpus, std::ostream& tagFileOs) instead of buildOne(std::ostream& os, Corpus const& corpus, std::string_view fileName) and a new overload in Generator without std::ostream& tagFileOs which would call buildOne with a noop std::ostream for the tagFileOs.
This fixes the underlying problem and the original issue with buildOneString that's breaking CI.
The implementation of the tagfile write is excellent.
CI failed after I rebased. That was a good thing because it helped me catch a bug.
So, the reason CI is failing is trivial. The
TestRunnercallsif(auto err = gen_->buildOneString(generatedDocs, **corpus)), which callsGenerator::buildOneString, where you changed the call tobuildOnetobuildOne(ss, corpus, "");. We could fix CI by just adaptingAdocGenerator::buildOneto not generate the tagfile when this string is empty. However, this doesn't address the deeper issue here.The real issue causing all of that is the API design. Your new
fileNameparameter inGenerator::buildOneis breaking the contract of this abstract class.
- First of all, the new parameter is documented as
@param fileName The file name to use for the output., but that's not what the implementation is doing. The filename is the filename for the tagfile, which is unrelated to the filename of the output. The parameter isn't even used in theXMLGenerator. It's even more confusing becausefileNameis not the filename of the tagfile but the basename to which "tag.xml" will be appended to form the tagfile filename.- Most importantly, the contract of
Generator::buildOneis that it renders the documentation to any output stream regardless of the filename because (i) there's not even a filename for most output types and (ii) if there were a file for each stream, the stream would become redundant. For instance, tests usebuildOneString, which outputs the result to a string.This bug was already there, but CI only made it evident. It worked before rebasing because
Generator::buildOneStringwasn't being covered with theAdocGenerator.Because of how
Generatorexpects the output to be decoupled from any filename andbuildOneis based on streaming only that result to the output, theGeneratorshould have received new virtual functions tobuildTagfile(std::ostream& os, Corpus const& corpus)or something instead ofbuildOnetrying to fill this role.If this is hard to achieve because we can't decouple the logic of
buildOneandbuildTagfile, a second-best would bebuildOne(std::ostream& os, Corpus const& corpus, std::ostream& tagFileOs)instead ofbuildOne(std::ostream& os, Corpus const& corpus, std::string_view fileName)and a new overload inGeneratorwithoutstd::ostream& tagFileOswhich would callbuildOnewith a noopstd::ostreamfor thetagFileOs.This fixes the underlying problem and the original issue with
buildOneStringthat's breaking CI.
This issue is addressed. I’ll let you know once CI passes so you can review, and if it’s good to go, feel free to merge.
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html
An automated preview of the documentation is available at https://670.mrdocs.prtest2.cppalliance.org/index.html