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

fix: Don't strip new lines in dhautofunction #805

Merged
merged 5 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions sphinx_ext/deephaven_autodoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class FunctionMetadata(TypedDict):
name: str


# total is False to allow for popping some keys
class SignatureData(TypedDict):
parameters: Params
return_description: str
Expand All @@ -44,6 +45,8 @@ class SignatureData(TypedDict):

SignatureValue = Union[str, Params]

AUTOFUNCTION_COMMENT_PREFIX = "AutofunctionCommentPrefix:"


def extract_parameter_defaults(
node: sphinx.addnodes.desc_parameterlist,
Expand Down Expand Up @@ -101,9 +104,9 @@ def extract_list_item(node: docutils.nodes.list_item) -> ParamData:
Returns:
The param data
"""
field = node.astext().replace("\n", " ")
field = node.astext().replace("\n", "<br />")
try:
match = re.match(r"(.+) \((.*)\) -- (.+)", field)
match = re.match(r"(.+?) \((.*?)\) -- (.+)", field, re.DOTALL)
if match is None:
raise ValueError(
f"Could not match {field} to extract param data. "
Expand Down Expand Up @@ -154,7 +157,7 @@ def extract_field_body(
# this is still a parameter, likely the only one in its signature
return [extract_list_item(child)]
elif is_paragraph:
return node.astext().replace("\n", " ")
return node.astext().replace("\n", "<br />")
elif isinstance(child, docutils.nodes.bullet_list):
return extract_list_items(child)
raise ValueError(
Expand Down Expand Up @@ -225,7 +228,7 @@ def extract_content_data(
if isinstance(child_node, docutils.nodes.field_list):
result.update(extract_field_list(child_node))
elif isinstance(child_node, docutils.nodes.paragraph):
result["description"] = child_node.astext().replace("\n", " ")
result["description"] = child_node.astext().replace("\n", "<br />")

return result

Expand Down Expand Up @@ -305,11 +308,19 @@ def to_mdx(node: sphinx.addnodes.desc) -> docutils.nodes.comment:

dat = json.dumps(result)

param_table = f"<ParamTable param={{{dat}}} />"
return_description = result["return_description"]
return_type = result["return_type"]

autofunction_markdown = (
f"{AUTOFUNCTION_COMMENT_PREFIX}"
rf"{return_description}<br /><br />"
rf"**Returns:** {return_type}<br /><br />"
rf"<ParamTable param={{{dat}}} />"
Comment on lines +311 to +318
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these
actually inserted in output?"

If so, it's markdown, new lines are respected.

Also should this output be conditional? If no return type or no description?

Copy link
Contributor

Choose a reason for hiding this comment

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

image

I would expect output that looks more like this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you mean \n are respected they are not at least in this case. This is inserting a raw string (which is has to do because otherwise sphinx tries to escape characters it shouldn't) so it writes a raw \n, it doesn't create a new line.

Currently return_type and return_description are required. This can change but will require a bit more rework than just here.

Copy link
Collaborator Author

@jnumainville jnumainville Sep 30, 2024

Choose a reason for hiding this comment

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

I'll see if I can replace those <br /> with a different sort of new line in the post processing, but ideally I don't want actual markdown new lines until then because it makes the processing harder than it needs to be

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done, that seems to have the needed output

)

# This is a little hacky, but this way the markdown renderer will not escape the special characters
# such as * and \. The comment markers will be removed by make_docs.py.
return docutils.nodes.comment("", "", docutils.nodes.raw("", param_table))
return docutils.nodes.comment("", "", docutils.nodes.raw("", autofunction_markdown))


class DeephavenAutodoc(AutodocDirective):
Expand Down
8 changes: 6 additions & 2 deletions sphinx_ext/make_docs_utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import os
import contextlib
from typing import IO, Generator
from deephaven_autodoc import AUTOFUNCTION_COMMENT_PREFIX
jnumainville marked this conversation as resolved.
Show resolved Hide resolved

BUILT_DOCS = "docs/build/markdown"

Expand Down Expand Up @@ -111,12 +112,15 @@ def remove_paramtable_comment(
Returns:
str: The line with the comment markers removed
"""
if line.startswith("<!-- <ParamTable param={{") and line.endswith("}} /> -->\n"):
if line.startswith(f"<!-- {AUTOFUNCTION_COMMENT_PREFIX}") and line.endswith(
"}} /> -->\n"
):
# remove the comment markers
# these are added in deephaven_autodoc.py to prevent special characters from being escaped
# by the markdown renderer
line = line.replace("<!-- ", "")
line = line.replace(f"<!-- {AUTOFUNCTION_COMMENT_PREFIX}", "")
line = line.replace(" -->", "")
line = line.replace("<br />", "\n")
return line


Expand Down
Loading