Skip to content

Doc generation improvements #2

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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -22,88 +22,88 @@ public void setUp() {
@Test
public void testConvertCommonmarkToRstWithTitleAndParagraph() {
String html = "<html><body><h1>Title</h1><p>Paragraph</p></body></html>";
String expected = "\n\nTitle\n=====\nParagraph\n";
String expected = "Title\n=====\nParagraph";
String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
assertEquals(expected, result);
assertEquals(expected, result.trim());
}

@Test
public void testConvertCommonmarkToRstWithImportantNote() {
String html = "<html><body><important>Important note</important></body></html>";
String expected = "\n\n.. important::\n Important note\n";
String expected = ".. important::\n Important note";
String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
assertEquals(expected, result);
assertEquals(expected, result.trim());
}

@Test
public void testConvertCommonmarkToRstWithList() {
String html = "<html><body><ul><li>Item 1</li><li>Item 2</li></ul></body></html>";
String expected = "\n\n* Item 1\n\n* Item 2\n\n";
String expected = "* Item 1\n* Item 2";
String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
assertEquals(expected, result);
assertEquals(expected, result.trim());
}

@Test
public void testConvertCommonmarkToRstWithMixedElements() {
String html = "<html><body><h1>Title</h1><p>Paragraph</p><ul><li>Item 1</li><li>Item 2</li></ul></body></html>";
String expected = "\n\nTitle\n=====\nParagraph\n\n* Item 1\n\n* Item 2\n\n";
String expected = "Title\n=====\nParagraph\n\n\n* Item 1\n* Item 2";
String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
assertEquals(expected, result);
assertEquals(expected, result.trim());
}

@Test
public void testConvertCommonmarkToRstWithNestedElements() {
String html = "<html><body><h1>Title</h1><p>Paragraph with <strong>bold</strong> text</p></body></html>";
String expected = "\n\nTitle\n=====\nParagraph with **bold** text\n";
String expected = "Title\n=====\nParagraph with **bold** text";
String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
assertEquals(expected, result);
assertEquals(expected, result.trim());
}

@Test
public void testConvertCommonmarkToRstWithAnchorTag() {
String html = "<html><body><a href='https://example.com'>Link</a></body></html>";
String expected = "\n`Link <https://example.com>`_";
String expected = "`Link <https://example.com>`_";
String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
assertEquals(expected, result);
assertEquals(expected, result.trim());
}

@Test
public void testConvertCommonmarkToRstWithBoldTag() {
String html = "<html><body><b>Bold text</b></body></html>";
String expected = "\n**Bold text**";
String expected = "**Bold text**";
String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
assertEquals(expected, result);
assertEquals(expected, result.trim());
}

@Test
public void testConvertCommonmarkToRstWithItalicTag() {
String html = "<html><body><i>Italic text</i></body></html>";
String expected = "\n*Italic text*";
String expected = "*Italic text*";
String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
assertEquals(expected, result);
assertEquals(expected, result.trim());
}

@Test
public void testConvertCommonmarkToRstWithCodeTag() {
String html = "<html><body><code>code snippet</code></body></html>";
String expected = "\n``code snippet``";
String expected = "``code snippet``";
String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
assertEquals(expected, result);
assertEquals(expected, result.trim());
}

@Test
public void testConvertCommonmarkToRstWithNoteTag() {
String html = "<html><body><note>Note text</note></body></html>";
String expected = "\n\n.. note::\n Note text\n";
String expected = ".. note::\n Note text";
String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
assertEquals(expected, result);
assertEquals(expected, result.trim());
}

@Test
public void testConvertCommonmarkToRstWithNestedList() {
String html = "<html><body><ul><li>Item 1<ul><li>Subitem 1</li></ul></li><li>Item 2</li></ul></body></html>";
String expected = "\n\n* Item 1\n * Subitem 1\n\n* Item 2\n\n";
String expected = "* Item 1\n\n * Subitem 1\n* Item 2";
String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
assertEquals(expected, result);
assertEquals(expected, result.trim());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import software.amazon.smithy.python.codegen.integrations.PythonIntegration;
import software.amazon.smithy.python.codegen.integrations.RuntimeClientPlugin;
import software.amazon.smithy.python.codegen.sections.*;
import software.amazon.smithy.python.codegen.writer.MarkdownToRstDocConverter;
import software.amazon.smithy.python.codegen.writer.PythonWriter;
import software.amazon.smithy.utils.SmithyInternalApi;

Expand Down Expand Up @@ -60,6 +61,8 @@ private void generateService(PythonWriter writer) {
var docs = service.getTrait(DocumentationTrait.class)
.map(StringTrait::getValue)
.orElse("Client for " + serviceSymbol.getName());
String rstDocs =
MarkdownToRstDocConverter.getInstance().convertCommonmarkToRst(docs);
writer.writeDocs(() -> {
writer.write("""
$L
Expand All @@ -68,7 +71,7 @@ private void generateService(PythonWriter writer) {
endpoint for HTTP services or auth credentials.

:param plugins: A list of callables that modify the configuration dynamically. These
can be used to set defaults, for example.""", docs);
can be used to set defaults, for example.""", rstDocs);
});

var defaultPlugins = new LinkedHashSet<SymbolReference>();
Expand Down Expand Up @@ -866,14 +869,17 @@ private void writeSharedOperationInit(PythonWriter writer, OperationShape operat
.orElse("The operation's input.");

writer.write("""
:param input: $L

$L
""",docs);
writer.write("");
writer.write(":param input: $L", inputDocs);
writer.write("");
writer.write("""
:param plugins: A list of callables that modify the configuration dynamically.
Changes made by these plugins only apply for the duration of the operation
execution and will not affect any other operation invocations.
""");

$L
""", inputDocs, docs);
});

var defaultPlugins = new LinkedHashSet<SymbolReference>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ private static void writeConf(
}
}

autodoc_typehints = 'both'
autodoc_typehints = 'description'
""", projectName, version, projectName);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.StructureShape;
import software.amazon.smithy.model.traits.ClientOptionalTrait;
import software.amazon.smithy.model.traits.DefaultTrait;
import software.amazon.smithy.model.traits.DocumentationTrait;
import software.amazon.smithy.model.traits.ErrorTrait;
import software.amazon.smithy.model.traits.InputTrait;
import software.amazon.smithy.model.traits.OutputTrait;
import software.amazon.smithy.model.traits.RequiredTrait;
import software.amazon.smithy.model.traits.SensitiveTrait;
import software.amazon.smithy.model.traits.StreamingTrait;
import software.amazon.smithy.python.codegen.CodegenUtils;
Expand Down Expand Up @@ -316,8 +318,14 @@ private boolean hasDocs() {

private void writeMemberDocs(MemberShape member) {
member.getMemberTrait(model, DocumentationTrait.class).ifPresent(trait -> {
String descriptionPrefix = "";
if (member.hasTrait(RequiredTrait.class) && !member.hasTrait(ClientOptionalTrait.class)) {
descriptionPrefix = "**[Required]** - ";
}

String memberName = symbolProvider.toMemberName(member);
String docs = writer.formatDocs(String.format(":param %s: %s", memberName, trait.getValue()));
String docs = writer.formatDocs(String.format(":param %s: %s%s",
memberName, descriptionPrefix, trait.getValue()));
writer.write(docs);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import static org.jsoup.nodes.Document.OutputSettings.Syntax.html;

import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.commonmark.node.BlockQuote;
import org.commonmark.node.FencedCodeBlock;
import org.commonmark.node.Heading;
Expand All @@ -21,6 +23,7 @@
import org.jsoup.nodes.TextNode;
import org.jsoup.select.NodeVisitor;
import software.amazon.smithy.utils.SetUtils;
import software.amazon.smithy.utils.SimpleCodeWriter;
import software.amazon.smithy.utils.SmithyInternalApi;

/**
Expand Down Expand Up @@ -51,17 +54,19 @@ public static MarkdownToRstDocConverter getInstance() {
}

public String convertCommonmarkToRst(String commonmark) {
String html =
HtmlRenderer.builder().escapeHtml(false).build().render(MARKDOWN_PARSER.parse(commonmark));
String html = HtmlRenderer.builder().escapeHtml(false).build().render(MARKDOWN_PARSER.parse(commonmark));
//Replace the outer HTML paragraph tag with a div tag
Pattern pattern = Pattern.compile("^<p>(.*)</p>$", Pattern.DOTALL);
Matcher matcher = pattern.matcher(html);
html = matcher.replaceAll("<div>$1</div>");
Document document = Jsoup.parse(html);
RstNodeVisitor visitor = new RstNodeVisitor();
document.body().traverse(visitor);
return "\n" + visitor;
}

private static class RstNodeVisitor implements NodeVisitor {
//TODO migrate away from StringBuilder to use a SimpleCodeWriter
private final StringBuilder sb = new StringBuilder();
SimpleCodeWriter writer = new SimpleCodeWriter();
private boolean inList = false;
private int listDepth = 0;

Expand All @@ -71,47 +76,73 @@ public void head(Node node, int depth) {
TextNode textNode = (TextNode) node;
String text = textNode.text();
if (!text.trim().isEmpty()) {
sb.append(text);
// Account for services making a paragraph tag that's empty except
// for a newline
if (text.startsWith(":param ")) {
int secondColonIndex = text.indexOf(':', 1);
writer.write(text.substring(0, secondColonIndex + 1));
//TODO right now the code generator gives us a mixture of
// RST and HTML (for instance :param xyz: <p> docs
// </p>). Since we standardize to html above, that <p> tag
// starts a newline. We account for that with this if/else
// statement, but we should refactor this in the future to
// have a more elegant codepath.
if (secondColonIndex +1 == text.strip().length()) {
writer.indent();
writer.ensureNewline();
} else {
writer.ensureNewline();
writer.indent();
writer.write(text.substring(secondColonIndex + 1));
writer.dedent();
}
} else {
writer.writeInline(text);
}
// Account for services making a paragraph tag that's empty except
// for a newline
} else if (node.parent() instanceof Element && ((Element) node.parent()).tagName().equals("p")) {
sb.append(text.replaceAll("[ \\t]+", ""));
writer.writeInline(text.replaceAll("[ \\t]+", ""));
}
} else if (node instanceof Element) {
Element element = (Element) node;
switch (element.tagName()) {
case "a":
sb.append("`");
writer.writeInline("`");
break;
case "b":
case "strong":
sb.append("**");
writer.writeInline("**");
break;
case "i":
case "em":
sb.append("*");
writer.writeInline("*");
break;
case "code":
sb.append("``");
writer.writeInline("``");
break;
case "important":
sb.append("\n.. important::\n ");
writer.ensureNewline();
writer.write("");
writer.openBlock(".. important::");
break;
case "note":
sb.append("\n.. note::\n ");
writer.ensureNewline();
writer.write("");
writer.openBlock(".. note::");
break;
case "ul":
if (inList) {
writer.indent();
}
inList = true;
listDepth++;
sb.append("\n");
writer.ensureNewline();
writer.write("");
break;
case "li":
if (inList) {
sb.append(" ".repeat(listDepth - 1)).append("* ");
}
writer.writeInline("* ");
break;
case "h1":
sb.append("\n");
writer.ensureNewline();
break;
default:
break;
Expand All @@ -125,41 +156,47 @@ public void tail(Node node, int depth) {
Element element = (Element) node;
switch (element.tagName()) {
case "a":
sb.append(" <").append(element.attr("href")).append(">`_");
String href = element.attr("href");
if (!href.isEmpty()) {
writer.writeInline(" <").writeInline(href).writeInline(">`_");
} else {
writer.writeInline("`");
}
break;
case "b":
case "strong":
sb.append("**");
writer.writeInline("**");
break;
case "i":
case "em":
sb.append("*");
writer.writeInline("*");
break;
case "code":
sb.append("``");
writer.writeInline("``");
break;
case "important":
case "note":
writer.closeBlock("");
break;
case "p":
sb.append("\n");
writer.ensureNewline();
writer.write("");
break;
case "ul":
listDepth--;
if (listDepth == 0) {
inList = false;
} else {
writer.dedent();
}
if (sb.charAt(sb.length() - 1) != '\n') {
sb.append("\n\n");
}
writer.ensureNewline();
break;
case "li":
if (sb.charAt(sb.length() - 1) != '\n') {
sb.append("\n\n");
}
writer.ensureNewline();
break;
case "h1":
String title = element.text();
sb.append("\n").append("=".repeat(title.length())).append("\n");
writer.ensureNewline().writeInline("=".repeat(title.length())).ensureNewline();
break;
default:
break;
Expand All @@ -169,7 +206,7 @@ public void tail(Node node, int depth) {

@Override
public String toString() {
return sb.toString();
return writer.toString();
}
}
}
Loading