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

doc/user: use Materialize-dialect SQL syntax highlighting #26222

Closed
wants to merge 1 commit into from

Conversation

arusahni
Copy link
Contributor

Motivation

  • This PR adds a feature that has not yet been specified.

Syntax highlighting in our docs uses a general SQL highlighter. This is fine, but we don't have highlighting on Mz-specific keywords, which is suboptimal. I have upstreamed Materialize SQL dialect highlighting to Chroma, the highlighting library used by Hugo. The most recent Hugo release includes it! So, this PR:

  1. Upgrades Hugo to the latest stable version. I had to make a slight update to our footer to account for .File being nil on pages without a directly attributable file (e.g., directories without an _index.md).
  2. Applies the new syntax highlighting to all examples of Materialize SQL dialect in the docs. I took care to not update SQL Server, MySQL, and Postgres examples.

Before

Without Mz syntax highlighting

After

With Mz syntax highlighting

If we're happy with this, and once it's merged, I'll work on formalizing the Chroma dialect update motion as a script so we can upstream new keywords as they land.

Tips for reviewer

Sorry for the large diff! If you'd prefer I introduce this section by section I can break up the PR.

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • This PR includes the following user-facing behavior changes:

@arusahni arusahni requested review from morsapaes and a team as code owners March 22, 2024 14:50
@arusahni
Copy link
Contributor Author

arusahni commented Mar 22, 2024

This has seemingly revealed a change in how Hugo computes relative URLs for links when the baseURL is a subdirectory. There's quite a robust discussion in the Hugo community around this. It seems as if the guidance is to convert relative Markdown URLs to use cross references. So,

[secrets](/sql/create-secret/)

becomes

[secrets]({{< ref "/sql/create-secret/" >}})

When I patch our render-link hook to add the /docs prefix, the linter is far happier. I wonder if we'd be best suited by punching through some custom config that the hook reads and applies to the URLs (or extracts the subdir from the baseURL), and then gradually shifting to the (more correct) cross reference impl.

@arusahni
Copy link
Contributor Author

Closing. Landed the Hugo changes in #27679 , and will get the syntax highlighting changes staged in a separate PR.

@arusahni arusahni closed this Jun 18, 2024
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

1 participant