Skip to content

fix(vars): render /vars as an HTML name-value table - #3413

Open
darion-yaphet wants to merge 2 commits into
apache:masterfrom
darion-yaphet:fix-vars-html-table
Open

fix(vars): render /vars as an HTML name-value table#3413
darion-yaphet wants to merge 2 commits into
apache:masterfrom
darion-yaphet:fix-vars-html-table

Conversation

@darion-yaphet

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: resolve
Problem Summary:
The /vars HTML page rendered each bvar as a plain <p> line (name : value), so names and values were hard to scan when lengths varied. Expandable flot chart
blocks were also mixed into the same flow, which made the layout less structured.

What is changed and the side effects?

Changed:

  • Render /vars HTML output as a gridtable with Name / Value columns.
  • Put each variable in a <tr class="variable|nonplot-variable"> row.
  • Move expandable flot charts into a dedicated <tr class="detail-row"> under the corresponding variable.
  • Keep non-HTML (curl / plain text) /vars output unchanged.
    Side effects:
  • Performance effects: None expected. Only the HTML presentation path of /vars is changed.
  • Breaking backward compatibility: No. Plain-text /vars responses are unchanged; only browser HTML layout is updated.

Check List:

image

Replace the plain paragraph list with a gridtable so variable
names and values align in columns, and keep expandable flot
charts in a dedicated detail row.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the usability of the built-in /vars HTML page by switching from a free-flowing paragraph layout to a structured two-column table, and by placing flot chart details into dedicated expandable rows to keep name/value scanning consistent.

Changes:

  • Render /vars HTML output as a gridtable with Name and Value columns.
  • Emit each bvar as a <tr class="variable|nonplot-variable"> and move flot charts into a following <tr class="detail-row">.
  • Update the page JavaScript/CSS to toggle and locate the new detail-row structure while keeping plain-text output unchanged.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/brpc/builtin/vars_service.cpp
Cover HTML responses triggered by a User-Agent, including the grid table, variable row, and expandable detail row. Preserve plain-text output coverage.

Constraint: HTML mode is selected by the User-Agent header
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Update both rendering-mode assertions when changing VarsService markup
Tested: BuiltinServiceTest.vars
Not-tested: Full suite; the default C++0x test configuration is incompatible with local GoogleTest 1.17
@wwbmmm

wwbmmm commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

What does it look like when a bvar row expand? Can you give a screenshot?

@darion-yaphet

Copy link
Copy Markdown
Contributor Author

What does it look like when a bvar row expand? Can you give a screenshot?

image

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