feat(highlight): highlight source code with the Gorge render service - #2
Merged
Merged
Conversation
Phorge's built-in highlighter covers only a handful of languages and this
image does not ship Pygments, so everything else renders colorless. Route
highlighting to gorge-render instead: it emits Pygments-compatible CSS class
names, so the existing stylesheets are reused unchanged, and no Python
runtime has to go into the image.
- client: add PhabricatorGorgeRenderClient, which reads the "{data, error}"
envelope before falling back to the HTTP status (the "error.code" it
carries is the more actionable diagnostic), authenticates with an
X-Service-Token header, strips a trailing slash off the configured URI, and
hands back unresolved futures so both sides of a diff can be resolved
together instead of serially. Documents why CURLOPT_CONNECTTIMEOUT is
deliberately not set: measured against a resolver which drops queries it
changed nothing but the label on the failure.
- future: add PhabricatorGorgeHighlightFuture, which rethrows failures as
PhutilSyntaxHighlighterException so DifferentialChangesetParser reports
that highlighting failed rather than quietly serving a colorless page, and
drops the synthetic "<?php" line the highlighter prepends to PHP fragments.
- engine: add PhabricatorGorgeSyntaxHighlighterEngine and
PhabricatorGorgeSyntaxHighlighter. The engine composes
PhutilDefaultSyntaxHighlighterEngine rather than extending it (that class
is final) and keeps text/txt, console, diviner, invisible, rainbow and
remarkup local, along with PHP whenever XHPAST is available, since XHPAST
parses rather than lexes and needs no round trip.
- config: add gorge.render.uri (locked) and gorge.render.token (hidden).
Setting the URI makes the service available but does not route
highlighting to it; that stays a separate manual switch so an operator can
verify the service before connecting it and can roll back with one command.
- setup: add PhabricatorGorgeSetupCheck, which probes /healthz and separately
reports a service which is healthy but never called, raising only one issue
at a time since they are two halves of one unfinished setup. Its remedy
includes "cache purge": what gets cached is highlighted HTML rather than
source, so without it a switched engine reads as having had no effect. Also
suppress PhabricatorPygmentSetupCheck's advice once Gorge is configured,
where it is permanent noise.
- library map: register the five new classes so PhutilClassMapQuery and the
config option pick them up.
- compose: add docker-compose.gorge.yml as an *overlay*, so the default
one-command start is untouched. It declares no ports (phorge reaches the
service by Compose service name) and gates phorge on
service_healthy, which is why the entrypoint does not poll /healthz itself:
a first page render against a not-yet-ready service would be cached and
need purging again.
- entrypoint: write gorge.render.uri / token with an idempotent bin/config
set on every start, deliberately outside the guarded local.json block.
These describe deployment topology and should follow the orchestration
rather than the phorge-conf volume, so changing .env and restarting is
enough. An empty value skips the key instead of writing or deleting it, and
a failed write warns rather than aborting, since highlighting degrades
gracefully.
- docs: document the two-step enable, the mandatory cache purge, rollback,
the GORGE_* variables and the 404/401 failure modes in DOCKER.md, and add
the same block to .env.example. Also document building the gorge-render
image locally (ghcr returns 403 for "does not exist yet", so docker login
is the wrong thing to chase) and that PHP sources are baked into the image,
which makes "up -d --build" plus "arc liberate src/" a prerequisite for new
classes.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phorge's built-in highlighter covers only a handful of languages and this image does not ship Pygments, so everything else renders colorless. Route highlighting to gorge-render instead: it emits Pygments-compatible CSS class names, so the existing stylesheets are reused unchanged, and no Python runtime has to go into the image.