-
Notifications
You must be signed in to change notification settings - Fork 9
support sublists & sentences splitted into multiple lines #5
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
Conversation
…&Text introduce & implement struct NestedString parser::Value & ast::ParsedDoxygen now uses mostly NestedString instead of string; Adapt code to work with NestedString add tests with sublists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! :)
I've wanted to fix this for some time now, but I didn't have the time to do so.
I'll probably review this tomorrow in depth, for now I would like you to run rustfmt, because it looks like it could fix a few formatting issues.
Thanks for the PR! 😄
I ran rustfmt with default settings, but I'm not sure if it's what you expected, as formatting in places I haven't touched also changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a question, but this looks like a sensible approach :)
I would like to change a few things about documentation, but I'll change them myself once the PR is merged
src/parser/mod.rs
Outdated
for ch in line.chars() { | ||
if ch.is_whitespace() && !sublist { | ||
leading_whitespaces += 1; | ||
} else if ch.is_whitespace() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this else if
needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it allows spaces between sublist mark('-', '*', '+') and sublist text to be skipped
eg. ( '_' is whitespace skipped by 'first if'; '.' is whitespace skipped by this else if):
_ _ _ _-....sublist text
but it can be done also in other ways (maybe simpler as I think about it now), eg.
for ch in line.chars() {
if ch.is_whitespace() {
leading_whitespaces += 1;
} else if ch == '-' || ch == '*' || ch == '+' {
sublist = true;
chars_to_skip = leading_whitespaces + 1;
break;
} else {
chars_to_skip = leading_whitespaces;
break;
};
}
line.drain(..chars_to_skip); //remove whitespace and sublist mark
line.trim_start();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that looks simpler, maybe you could change it
Thank you for this :) |
(all examples used below taken from NordicSemiconductor Softdevice API)
In doxygen param, retval and other Notation's are allowed to contain sublists eg.
currently doxygen-rs parses all lines separately and splits this into:
[in top/function description section ]
[in return values section]
Which produces total mess. After PR this would look like:
(all in one section)
Second thing currently not supported by doxygen-rs is line/sentence split into multiple line (probably to force maximum line width). eg.
and one more time doxygen-rs splits this and places it in different sections.
This Pull Request aims to improve behavior in this two cases
(how it works:
it replaces String in bunch of places with NestedString that is able to logicaly represent sublist;
in parser if indentation is detected it is assumed that current line is continuation to previous one
and previous NestedString is modified: either new element is pushed to NestedString.sub(in case of sublist) or line is appended to NestedString.top string(in case of multi line sentence)
)
sample function docs:
doxygen
before PR
after PR
generated docs before PR
generated docs after PR