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

feature: implement graphql, json, xml and yaml syntax highlighting #5176

Conversation

norbert-mieczkowski-codilime
Copy link
Collaborator

Closes #5098

What's Changed

  • Introduce client-side GraphQL, JSON, XML and YAML syntax highlighting with highlight.js library.
  • Embed a subset of highlight.js in the core application which supports only GraphQL, JSON, XML and YAML languages and provide two themes: github.min.css and github-dark.min.css.
  • Implement a main script which on page load highlights all the elements with language-graphql, language-json, language-xml and language-yaml class. These elements preferably have the following HTML structure: <pre><code class="language-json">...</pre></code>.
  • Add <code class="language-json">...</code> and <code class="language-yaml">...</code> wrappers to render_json and render_yaml Django template filters. This new default behavior can be disabled with syntax_highlight=False arg.
  • Update /theme-preview/ page with syntax highlighting examples.

Screenshots

Light theme Dark theme
image image_1

Copy link
Contributor

@glennmatthews glennmatthews left a comment

Choose a reason for hiding this comment

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

Looks straightforward - thank you! As was mentioned in standup, it would be good to add a note to the documentation for render_json and render_yaml in nautobot/docs/user-guide/platform-functionality/template-filters.md.

I wonder actually, should we enhance those two template filters to automatically wrap the result in an appropriate <code class="... block so that it's auto-magical, or would that be a potentially breaking change in some way?

nautobot/core/templates/inc/media.html Outdated Show resolved Hide resolved
@norbert-mieczkowski-codilime
Copy link
Collaborator Author

@glennmatthews, yeah I definitely see your point and I agree that adding a set of wrapping elements with a certain required class is not ideal. I tried to find a common pattern which could automate the process a bit and here's why I didn't go down that path:

  1. render_json is used in <textarea> element, making it currently impossible to wrap the code in <code class="...">...</code> at the template filter level. It's just a single usage, nevertheless it counts.
  2. Selecting pre or code alone on the other hand is too generic because they can contain all sorts of languages or pseudocode, not only JSON or YAML.

@glennmatthews
Copy link
Contributor

@glennmatthews, yeah I definitely see your point and I agree that adding a set of wrapping elements with a certain required class is not ideal. I tried to find a common pattern which could automate the process a bit and here's why I didn't go down that path:

1. `render_json` is used in `<textarea>` element, making it currently impossible to wrap the code in `<code class="...">...</code>` at the template filter level. It's just a single usage, nevertheless it counts.

Great thought/catch. I hadn't considered that case.

In that case, let's add this as a highlighted feature under the Release Overview in nautobot/docs/release-notes/version-2.2.md and use that space to instruct app developers as to what they need to add to take advantage of this feature.

@lampwins
Copy link
Member

Should we introduce a new template tag that does this though? I want to make sure it is as easy as possible to consume.

@HanlinMiao
Copy link
Contributor

Should we introduce a new template tag that does this though? I want to make sure it is as easy as possible to consume.

For Norbert's reference, do you mean something like this?

@register.simple_tag
def json_field(value):
    return <pre><code class="language-json">{{value|render_json }}</code></pre>

@glennmatthews
Copy link
Contributor

glennmatthews commented Jan 26, 2024

I talked it over with John and we think we're OK with adding the <code class=...> tag as part of the render_json and render_yaml responses. Let's add an optional no_highlight parameter to the templatetags and use that parameter for the <textarea> case. This is technically a breaking change but I searched the other apps in nautobot organization and all of their uses of these two templatetags are in the <pre> pattern so they should be unaffected by this change.

@lampwins
Copy link
Member

Let's go with syntax_highlight as the argument and default to True to make it more explicit and avoid the double negative.

@glennmatthews
Copy link
Contributor

glennmatthews commented Jan 26, 2024

Something like (untested):

@library.filter()
@register.filter()
def render_json(value, syntax_highlight=True):
    """
    Render a dictionary as formatted JSON.

    Unless `syntax_highlight=False` is specified, the returned string will be wrapped in a
    `<code class="language-json>` HTML tag to flag it for syntax highlighting by highlight.js.
    """
    rendered_json = json.dumps(value, indent=4, sort_keys=True, ensure_ascii=False)
    if syntax_highlight:
        return format_html('<code class="language-json">{}</code>', rendered_json)
    return rendered_json

@glennmatthews
Copy link
Contributor

How difficult/costly is it to enable additional languages? Might be useful at least to include XML as I think there's some work in several Apps that might benefit from that.

@norbert-mieczkowski-codilime
Copy link
Collaborator Author

@glennmatthews, adding more languages boils down to increasing library size which ultimately affects page load times. Here's a list of all languages available in highlight.js: Download a Custom Build - highlight.js. Supporting one, two or three more (I believe in Nautobot apart from JSON and YAML I also saw code snippets in GraphQL, Python and XML) shouldn't make much of a difference - it's just a couple of kBs (with no gizp) as long as we just don't grab them all blind.

@norbert-mieczkowski-codilime
Copy link
Collaborator Author

@glennmatthews @lampwins, now when the <code class="...">...</code> wrapper is moved to render_json/render_yaml template filter, is documenting this feature in nautobot/docs/release-notes/version-2.2.md still relevant?

@glennmatthews
Copy link
Contributor

IMO it's worth adding a note in the Release Overview still.

@glennmatthews
Copy link
Contributor

@glennmatthews, adding more languages boils down to increasing library size which ultimately affects page load times. Here's a list of all languages available in highlight.js: Download a Custom Build - highlight.js. Supporting one, two or three more (I believe in Nautobot apart from JSON and YAML I also saw code snippets in GraphQL, Python and XML) shouldn't make much of a difference - it's just a couple of kBs (with no gizp) as long as we just don't grab them all blind.

Let's confirm with @lampwins, but I think it'd be good to add GraphQL and XML now. We could already make use of GraphQL highlighting e.g. in nautobot/extras/templates/extras/graphqlquery.html for the query field. For that matter, we probably should also (re-)apply JSON highlighting to the query_output field when it's populated after running the saved query in question - see for example https://demo.nautobot.com/extras/graphql-queries/87b52a0d-f872-4cc9-961c-e848b9b91b83/.

@lampwins
Copy link
Member

Yes, let's add GraphQL and XML but draw the line there. Are we creating new template tags for these?

@glennmatthews
Copy link
Contributor

Yes, let's add GraphQL and XML but draw the line there. Are we creating new template tags for these?

I don't think we need to at this time - just maybe document that you need to use language-graphql and language-xml CSS classes on the containing HTML elements in order to take advantage?

@norbert-mieczkowski-codilime norbert-mieczkowski-codilime changed the title feature: implement json and yaml syntax highlighting feature: implement graphql, json, xml and yaml syntax highlighting Jan 30, 2024
changes/5098.added Outdated Show resolved Hide resolved
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Copy link
Contributor

@gsnider2195 gsnider2195 left a comment

Choose a reason for hiding this comment

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

When I save and reload a GraphQL query I'm seeing the HTML show up in the Query variables:

image

@glennmatthews
Copy link
Contributor

That's going to be in nautobot/core/templates/graphene/graphiql.html

@norbert-mieczkowski-codilime
Copy link
Collaborator Author

Oh wow, thanks @gsnider2195, due to lack of understanding of that piece of code I must've left it as-is

@gsnider2195 gsnider2195 merged commit 6742992 into next Feb 1, 2024
17 checks passed
@gsnider2195 gsnider2195 deleted the u/norbert-mieczkowski-codilime-json-and-yaml-syntax-highlighting branch February 1, 2024 15:43
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.

None yet

5 participants