Skip to content
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

Try to make the docs a little better. #524

Merged
merged 14 commits into from
Feb 16, 2022
Merged

Try to make the docs a little better. #524

merged 14 commits into from
Feb 16, 2022

Conversation

aiuto
Copy link
Collaborator

@aiuto aiuto commented Feb 4, 2022

  • Fix stardoc adding <p align=center> in markdown tables.
  • Add rough capability to handle macros that wrap a rule.

The wrapping technique is:

  • In the wrapping macro, use the docstring @wraps(some_rule)
  • Then we don't emit the docs for the wrapper
  • In the docs for some_rule, replace the string "some_rule" with the name of the wrapper macro.

It is a first attempt to make things better. It is crude, but it mostly works. If Stardoc ever adds this capability (feature request) we can move to that.

- Fix stardoc adding <p align=center> in markdown tables.
- Add rough capability to handle macros that wrap a rule.

The wrapping technique is:
- In the wrapping macro, use the docstring  @wraps(some_rule)
- Then we don't emit the docs for the wrapper
- In the docs for some_rule, replace the string "some_rule" with the
  name of the wrapper macro.

It is a first attempt to make things better.  It is crude, but it
mostly works.
@aiuto aiuto requested a review from nacl as a code owner February 4, 2022 02:30
Copy link
Collaborator

@nacl nacl left a comment

Choose a reason for hiding this comment

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

Definitely an improvement on the output side, but the implementation is a bit hacky.

Might be worth saying that in the source file in question, but otherwise should be fine for our immediate purposes.

doc_build/BUILD Show resolved Hide resolved
doc_build/BUILD Show resolved Hide resolved
CENTER_RE = re.compile(r'<p align="center">([^<]*)</p>')


def merge_file(file: str, out, wrapper_map:typing.Dict[str, str]) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This forces use of python 3.6 or newer. Not that it's a problem, but this might cause interesting problems on older platforms. Guess we'll deal with them if/when they appear.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Isn't 3.6 EOL?

doc_build/merge.py Show resolved Hide resolved
doc_build/merge.py Show resolved Hide resolved
doc_build/BUILD Outdated Show resolved Hide resolved
@aiuto aiuto merged commit a8638d6 into bazelbuild:main Feb 16, 2022
@aiuto aiuto deleted the docx branch February 16, 2022 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants