feat: use GitHub's default README as index page #255
Conversation
docfx_yaml/extension.py
Outdated
| """Cleans extra whitespace that breaks image links in index.html file.""" | ||
| image_link_pattern='\[\s*!\[image\]\(.*\)\s*\]\(.*\)' | ||
| new_lines = [] | ||
| with open(mdfile_path) as mdfile_iterator: |
There was a problem hiding this comment.
Just name this mdfile? No need for the type suffix.
docfx_yaml/extension.py
Outdated
| broken_image_links = [ | ||
| [m.start(), m.end()] | ||
| for m in re.finditer(image_link_pattern, file_content) | ||
| ] |
There was a problem hiding this comment.
This seems like it could be done in a single loop, rather than creating this list then iterating over it below?
There was a problem hiding this comment.
You're right. Removed the redundant list.
docfx_yaml/extension.py
Outdated
|
|
||
| new_lines.append(file_content[prev_end:]) | ||
|
|
||
| with open(mdfile_path, 'w') as mdfile_iterator: |
There was a problem hiding this comment.
Same naming comment (especially confusing because this writes to something named iterator, which doesn't really make sense to me).
| @@ -1448,16 +1477,16 @@ def prepend_markdown_header(filename: str, mdfile: Iterable[str]): | |||
| def find_markdown_pages(app, outdir): | |||
| # Use this to ignore markdown files that are unnecessary. | |||
| files_to_ignore = [ | |||
| "index.md", # merge index.md and README.md and index.yaml later. | |||
| # See https://github.com/googleapis/sphinx-docfx-yaml/issues/105. | |||
| "index.md", # use readme.md instead | |||
|
|
|||
| "reference.md", # Reference docs overlap with Overview. Will try and incorporate this in later. | |||
There was a problem hiding this comment.
Do we still want to ignore this?
There was a problem hiding this comment.
Yes - the current setup after running Sphinx build creates index.md and readme.md, the default index.md contains a bit more info than readme.md file that we want to use. So, we'll continue to ignore it, overwrite it with readme.md file instead.
docfx_yaml/extension.py
Outdated
|
|
||
| shutil.copy(mdfile, f"{outdir}/{mdfile_name_to_use}") | ||
|
|
||
| # Do not add index file to the TOC. |
There was a problem hiding this comment.
Why not? Seems like we could set name to Overview here and not need the special handling down below ([{'name': 'Overview', 'href': 'index.md'}] + app.env.markdown_pages + pkg_toc_yaml)?
Perhaps for sorting? Still seems like it would be better to handle all md pages in this function.
There was a problem hiding this comment.
Putting the handler here did make the order tweak a bit (Overview doesn't always appear at the top). But I agree that the logic should be streamlined. Modified the handler here instead.
docfx_yaml/extension.py
Outdated
| # Do not add index file to the TOC. | ||
| if mdfile_name_to_use == 'index.md': | ||
| # Clean up any broken image links for the index file. | ||
| clean_image_links(f"{outdir}/{mdfile_name_to_use}") |
There was a problem hiding this comment.
This doesn't apply for every md file?
There was a problem hiding this comment.
I've only seen this for the README pages containing the image, but wouldn't hurt to look for in every page. Updated.
|
|
||
| [ | ||
|
|
||
|  |
There was a problem hiding this comment.
Seems like GitHub doesn't render these properly?
There was a problem hiding this comment.
Currently it doesn't because they're broken up - when the whitespaces are all trimmed then it should look fine, see https://github.com/googleapis/sphinx-docfx-yaml/blob/d016f0422bfb829df4eeb9fe8a9d10a5617f0e69/tests/markdown_example_bad_image_links_want.md
docfx_yaml/extension.py
Outdated
| """Cleans extra whitespace that breaks image links in index.html file.""" | ||
| image_link_pattern='\[\s*!\[image\]\(.*\)\s*\]\(.*\)' | ||
| new_lines = [] | ||
| with open(mdfile_path) as mdfile_iterator: |
docfx_yaml/extension.py
Outdated
| broken_image_links = [ | ||
| [m.start(), m.end()] | ||
| for m in re.finditer(image_link_pattern, file_content) | ||
| ] |
There was a problem hiding this comment.
You're right. Removed the redundant list.
docfx_yaml/extension.py
Outdated
|
|
||
| new_lines.append(file_content[prev_end:]) | ||
|
|
||
| with open(mdfile_path, 'w') as mdfile_iterator: |
| @@ -1448,16 +1477,16 @@ def prepend_markdown_header(filename: str, mdfile: Iterable[str]): | |||
| def find_markdown_pages(app, outdir): | |||
| # Use this to ignore markdown files that are unnecessary. | |||
| files_to_ignore = [ | |||
| "index.md", # merge index.md and README.md and index.yaml later. | |||
| # See https://github.com/googleapis/sphinx-docfx-yaml/issues/105. | |||
| "index.md", # use readme.md instead | |||
|
|
|||
| "reference.md", # Reference docs overlap with Overview. Will try and incorporate this in later. | |||
There was a problem hiding this comment.
Yes - the current setup after running Sphinx build creates index.md and readme.md, the default index.md contains a bit more info than readme.md file that we want to use. So, we'll continue to ignore it, overwrite it with readme.md file instead.
|
|
||
| [ | ||
|
|
||
|  |
There was a problem hiding this comment.
Currently it doesn't because they're broken up - when the whitespaces are all trimmed then it should look fine, see https://github.com/googleapis/sphinx-docfx-yaml/blob/d016f0422bfb829df4eeb9fe8a9d10a5617f0e69/tests/markdown_example_bad_image_links_want.md
docfx_yaml/extension.py
Outdated
| # Do not add index file to the TOC. | ||
| if mdfile_name_to_use == 'index.md': | ||
| # Clean up any broken image links for the index file. | ||
| clean_image_links(f"{outdir}/{mdfile_name_to_use}") |
There was a problem hiding this comment.
I've only seen this for the README pages containing the image, but wouldn't hurt to look for in every page. Updated.
docfx_yaml/extension.py
Outdated
|
|
||
| shutil.copy(mdfile, f"{outdir}/{mdfile_name_to_use}") | ||
|
|
||
| # Do not add index file to the TOC. |
There was a problem hiding this comment.
Putting the handler here did make the order tweak a bit (Overview doesn't always appear at the top). But I agree that the logic should be streamlined. Modified the handler here instead.
Instead of using a custom index page that provided little to no benefit, this PR is incorporating the existing README as the main index page. This was asked by a lot of DPEs for their client libraries' landing page.
The markdown generator unfortunately messed around with the spacing for image links (see tests/markdown_example_bad_image_links.md file for reference), but all other parts I've verified is looking good. (See staged version of the doc for https://cloud.google.com/python/docs/reference/storage/latest). Adding a small parser to help fix that for the index file.
Fixes #105.
Fixes #107.