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

Tweak the semantic convention code generation. #3394

Merged
merged 2 commits into from
Jul 13, 2021

Conversation

jkwatson
Copy link
Contributor

In particular, to better support links and html in the docs.

In particular, to better support links and html in the docs.
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@Oberon00 Oberon00 left a comment

Choose a reason for hiding this comment

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

You could also try using render_markdown(code="{{@code {0}}}"). Pro: Will look a bit more idiomatic, con: Would probably break if there are } in the input.

@@ -64,10 +64,10 @@ public final class {{class}} {
{%- for attribute in attributes if attribute.is_local and not attribute.ref %}

/**
* {% filter escape %}{{attribute.brief | to_doc_brief}}.{% endfilter %}
* {% filter safe %}{{attribute.brief | render_markdown}}{% endfilter %}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* {% filter safe %}{{attribute.brief | render_markdown}}{% endfilter %}
* {{attribute.brief | render_markdown | safe}}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't even think it needs the | safe which I believe is a no-op.

{%- if attribute.note %}
*
* Note: {% filter escape %}{{attribute.note | to_doc_brief}}.{% endfilter %}
* <p>Notes: {% filter safe %}{{attribute.note | render_markdown}}{% endfilter %}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* <p>Notes: {% filter safe %}{{attribute.note | render_markdown}}{% endfilter %}
* <p>Notes: {{attribute.note | render_markdown | safe}}

@jkwatson
Copy link
Contributor Author

You could also try using render_markdown(code="{{@code {0}}}"). Pro: Will look a bit more idiomatic, con: Would probably break if there are } in the input.

Can you explain/point to docs about what this is doing? @breedx-splk and I spent an hour yesterday trying to figure out how to send code blocks into the python via the jinja template and never saw anything like this (and never got anything working, either).

@Oberon00
Copy link
Member

Unfortunately, there is no documentation for this one. 😬

@Oberon00
Copy link
Member

So instead of <code>sometext</code> you will end up with {@code sometext}.

@jkwatson
Copy link
Contributor Author

Unfortunately, there is no documentation for this one. 😬

Also unfortunately, it doesn't look like it works. :(

  File "/templates/SemanticAttributes.java.j2", line 67, in template
    * {{attribute.brief | render_markdown(code="{{@code {0}}}"}}
jinja2.exceptions.TemplateSyntaxError: unexpected '}', expected ')'

@jkwatson jkwatson merged commit 3925b50 into open-telemetry:main Jul 13, 2021
@jkwatson jkwatson deleted the tweak_semconv_generation branch July 13, 2021 21:39
@Oberon00
Copy link
Member

Also unfortunately, it doesn't look like it works. :(

Because you forgot the closing ) between the " and the final }}. 😃

@jkwatson
Copy link
Contributor Author

Also unfortunately, it doesn't look like it works. :(

Because you forgot the closing ) between the " and the final }}. 😃

Dangit. This code ends up looking like perl.

@Oberon00
Copy link
Member

Yeah, too many nesting levels here with quotes and various brackets.

@jkwatson
Copy link
Contributor Author

Now I have the power! #3398

This was referenced Dec 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants