Skip to content

Enable preformatting of job attempt logs#530

Closed
jwarner112 wants to merge 1 commit into
riverqueue:masterfrom
jwarner112:jwarner/job-attempt-log-preformatting
Closed

Enable preformatting of job attempt logs#530
jwarner112 wants to merge 1 commit into
riverqueue:masterfrom
jwarner112:jwarner/job-attempt-log-preformatting

Conversation

@jwarner112

Copy link
Copy Markdown

I recognize that it's ill-advised to use the metadata to store lots of logs. However, the logs I do provide, I'd like to have control over the output for. The <code> block in the PlaintextPanel prevents any attempts at pre-formatting.

In the backend, these logs are glommed together as a single []byte and that's fine, but upon output, that should make rendering the output of the TextHandler or JSONHandler easy; both of them separate records with newlines. With the use of a simple <code> tag, there's no way to perform any kind of formatting. It will strip it all.

This PR changes the PlainText component's <code></code> to a <pre><code></code></pre>, and I hope you'll consider approval.

I recognize that it's ill-advised to use the metadata to store lots of logs. However, the logs I do provide, I'd like to have control over the output for. The `<code>` block in the [PlaintextPanel][a] prevents any attempts at pre-formatting.

In the backend, these logs are glommed together as a single `[]byte` and that's fine, but upon output, that should make rendering the output of the `TextHandler` or `JSONHandler` easy; both of them separate records with newlines. With the use of a simple `<code>` tag, there's no way to perform any kind of formatting. It will strip it all.

This PR changes the PlainText component's `<code></code>` to a `<pre><code></code></pre>`, and I hope you'll consider approval.

[a]: https://github.com/riverqueue/riverui/blob/9df43c2f4f86cd726997ada749ac42e07be8b3f2/src/components/PlaintextPanel.tsx#L127-L129
@brandur

brandur commented Mar 17, 2026

Copy link
Copy Markdown
Collaborator

@bgentry Ugh, Blake I might have to kick this one over to you. I think it seems okay from a general HTML sense (it's common for the pre and code elements to be found together), but not sure if there'd be any gotchas to look out here given the expansive TS stack.

@jwarner112

Copy link
Copy Markdown
Author

Thanks for the reply! From my research, <code> was intended for one-liners and <pre><code> is what you want for multi-line snippets. In fact, the single-backtick inliner vs three-backtick code fence appears to be implemented in GitHub's own markdown previewer as <code> and <pre><code>, respectively.

@jwarner112

Copy link
Copy Markdown
Author

Hey @bgentry, just following up on this. There's no rush, of course. I just left the tab open.

@brandur

brandur commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3ba0d79296

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

<code className={`${styleConfig.content.code} ${codeClassName || ""}`}>
{content}
</code>
<pre>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Add the required changelog entry

This is a user-facing behavior change to how job attempt logs/plaintext are rendered, but the commit only changes PlaintextPanel.tsx. The repo instructions in AGENTS.md require user-facing changes to add an Unreleased CHANGELOG.md entry with the project’s PR reference link convention, so this should be documented before merging.

Useful? React with 👍 / 👎.

@bgentry

bgentry commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

@jwarner112 I'm so sorry for the inexcusable delay here—it's been a long couple of months with company acquisition and new job 😅 . I opened #592 to address these issues in a slightly different way that preserves the unique behaviors of the different components.

@bgentry bgentry closed this in #592 Jun 12, 2026
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.

3 participants