Simplify Comment Parsing Code #3399
Conversation
| @@ -648,48 +648,44 @@ namespace | |||
| if (stripMarkup) | |||
| { | |||
| // Strip HTML markup. | |||
There was a problem hiding this comment.
Switching to a while loop lets us avoid a redundant check for pos != string::npos.
There was a problem hiding this comment.
The code assumes anything between < and > is an html tag.
What about something like:
a < b or x > y , won't this strip the comment to something like a y?
It is not a new problem, but would be nice to fix now that we are looking at this code.
We can fix in a separate PR.
| } while (pos != string::npos); | ||
| comment.erase(pos, endpos - pos + 1); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This logic was nested inside if (stripMarkup). This doesn't seem right to me.
XML escaping should be unrelated to markdown striping.
| @@ -742,35 +738,27 @@ namespace | |||
| return result; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
This function took a bool namedTag parameter which was always hard-coded.
And even worse, it took a const string& name which was only used some of the time based on the bool.
So I split it into 2 functions: parseNamedCommentLine and parseCommentLine. No more bool.
| @@ -803,94 +807,69 @@ Slice::Contained::parseDocComment(function<string(string, string)> linkFormatter | |||
|
|
|||
| DocCommentPtr comment = make_shared<DocComment>(); | |||
There was a problem hiding this comment.
All the remaining code below this is where we parse the actual tags (@param etc) into their respective fields on DocComment. Before, we had an entire state machine, but this was overkill.
I removed all this logic, and replaced it with a StringList* currentSection,
which points to whichever part of the doc-comment we're currently parsing.
At the beginning it points to the overview, but as we encounter tags, we re-point it appropiately.
I find this approach simpler than using a state-machine, and switching on the state to determine where we're writing to.
| @@ -648,48 +648,44 @@ namespace | |||
| if (stripMarkup) | |||
| { | |||
| // Strip HTML markup. | |||
There was a problem hiding this comment.
The code assumes anything between < and > is an html tag.
What about something like:
a < b or x > y , won't this strip the comment to something like a y?
It is not a new problem, but would be nice to fix now that we are looking at this code.
We can fix in a separate PR.
|
I agree, it's also a problem I noticed, but didn't want to fix in this PR. |
bernardnormier
requested review from
externl
and removed request for
bernardnormier
This PR simplifies the implementation of
parseDocComment, which is the part oflibSliceresponsible for converting a rawstring commentinto a structuredDocComment.It's not meant to be a perfect cleanup.
But just to simplify some things before I add the last validation logic/fix the issues related to how
@links are formatted.I used my compiler-comparison script to check, and the generated code is identical before and after my changes here.