Skip to content

Add markdown renderers and renderer factory #2152

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

Merged
merged 5 commits into from
Mar 9, 2020

Conversation

jdkoren
Copy link
Contributor

@jdkoren jdkoren commented Feb 26, 2020

No description provided.

@googlebot googlebot added the cla: yes Google CLA check succeeded. label Feb 26, 2020
@jdkoren jdkoren requested a review from jcollins-g February 28, 2020 18:25
jdkoren added 5 commits March 4, 2020 08:33
Only enough to mirror the few tests we have that exercise the existing
renderers. In the future, we should have a more comprehensive suite
that covers both html and md.
This avoids calling back to Category, which immediately calls the
renderer again anyway, except that during a test the second call went
to an html renderer because the packageGraph was holding a
HtmlRendererFactory.
Copy link
Contributor

@jcollins-g jcollins-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this might be enough now to actually write out some files. Are there any screenshots or other bits that can give a signal as to the progress? Particularly, if you can provide sample pages for the templates impacted (class, callable) -- even if they're not perfect, it would help me get a better sense of where we're at.

CategoryRenderer get categoryRenderer => CategoryRendererMd();

// We render documentation as HTML for now.
// TODO(jdkoren): explore using documentation directly in the output file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be a challenge, particularly when it comes to tools. I'm also curious as to whether tool execution works correctly end-to-end here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, @tool was one of the reasons I thought to leave documentation as html inside the markdown page. Same for @video.

We could in theory run the tools as they are and expect that whoever is running Dartdoc has taken the output format into account when they added the tools...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... that's not completely unreasonable if we provide a way to tell the tool what sort of output to generate. Keeping things HTML for now though makes a lot of sense as if you want any sort of compatibility with what Flutter is doing we'll either have to extend those tools or make it safe for them to continue outputting HTML.

@jdkoren
Copy link
Contributor Author

jdkoren commented Mar 5, 2020

Index
_tmp_dartdoc_test_index md

Apple class
_tmp_dartdoc_test_ex_Apple-class md

isGreaterThan method
_tmp_dartdoc_test_ex_Apple_isGreaterThan md

Copy link
Contributor

@jcollins-g jcollins-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty impressive -- we can actually create some markdown now!

@jdkoren jdkoren merged commit f5aaae5 into dart-lang:master Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants