fix: parse xrefs differently with new xref format #90
Conversation
google-cla
bot
added
the
cla: yes
|
Testing on Kokoro with |
docfx_yaml/extension.py
Outdated
| initial_index += summary_part.find("<xref") | ||
| original_type = parsed_text[initial_index:initial_index+(parsed_text[initial_index:].find('>'))+1] | ||
| original_type = parsed_text[initial_index:initial_index+(parsed_text[initial_index:].find('/xref>'))+6] |
There was a problem hiding this comment.
What is 6? Can we make that a constant or something?
| initial_index += len(original_type) | ||
| original_type = " ".join(filter(None, re.split(r'\n| |\|\s|\t', original_type))) | ||
| safe_type = 'xref_' + original_type[6:-1] | ||
| # Extract text from "<xref uid="uid">text</xref>" |
There was a problem hiding this comment.
It's a bit late now, but we could also add this UID to the references section with however we want it to show up. Then, we wouldn't need to do any HTML parsing.
There was a problem hiding this comment.
I'll keep that in mind perhaps for non-Cloud or third party xrefs.
| if arg_name not in summary_info[var_types[cur_type]] and ':raises' not in cur_type: | ||
| summary_info[var_types[cur_type]][arg_name] = {} | ||
| except KeyError: | ||
| raise KeyError(f"Encountered wrong formatting, please check docstring for {name}") |
There was a problem hiding this comment.
I'm not sure this will work. What if there is a release with bad docs? We still need to generate docs for it. So, we should make our best guess and move on.
There was a problem hiding this comment.
Repos should have a docs-presubmit check for PRs, which runs the docs job to see whether it successfully runs this plugin on the repo for sphinx-build or not. For example see googleapis/python-firestore#407
While it's usually not set to be a required check, this lets contributors know that docs job would fail or pass before triggering a release.
There was a problem hiding this comment.
Sounds good. Might need to update this depending on how back filling old libraries goes.
There was a problem hiding this comment.
I do have the Kokoro job set up that could try and run it for all versions, I'll try testing on that for older versions.
| if arg_name not in summary_info[var_types[cur_type]] and ':raises' not in cur_type: | ||
| summary_info[var_types[cur_type]][arg_name] = {} | ||
| except KeyError: | ||
| raise KeyError(f"Encountered wrong formatting, please check docstring for {name}") |
There was a problem hiding this comment.
Sounds good. Might need to update this depending on how back filling old libraries goes.
Before you open a pull request, note that this repository is forked from here.
Unless the issue you're trying to solve is unique to this specific repository,
please file an issue and/or send changes upstream to the original as well.
Updating xref parser to accommodate the new and only xref format:
<xref uid="uid">text</xref>.We need to be careful on two parts:
textportion to replace so GoogleDocstring doesn't complain on the leftover xref syntaxFixes #89
Fixes googleapis/python-firestore#408