feat: add description column in Headers, VarsTable, and other components to allow multi-line descriptions#7129
feat: add description column in Headers, VarsTable, and other components to allow multi-line descriptions#7129Pragadesh-45 wants to merge 2 commits intousebruno:mainfrom
Conversation
Pragadesh-45
requested review from
bijin-bruno,
helloanoop,
lohit-bruno and
naman-bruno
as code owners
WalkthroughAdds an optional Description field end-to-end: UI toggleable Description columns with MultiLineEditor across tables, EditableTable add-row labeling, schema/types and converters updated to carry description, and BRU/ENV parser & serializer extended to extract and emit Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as EditableTable/UI
participant Editor as MultiLineEditor
participant Form as App Form State
participant Parser as BRU/ENV Parser
participant Serializer as BRU/ENV Serializer
User->>UI: Toggle "Description" column
UI->>Editor: render per-row MultiLineEditor
User->>Editor: Edit description text
Editor->>UI: onChange(description)
UI->>Form: update item.description
Form->>Form: persist/update model
User->>Serializer: Export BRU/ENV
Serializer->>Form: read items (with description)
Serializer->>Serializer: append `@description` suffix
Serializer->>User: output BRU/ENV text
User->>Parser: Import BRU/ENV
Parser->>Parser: extractDescription(pair)
Parser->>Form: populate item.description
Form->>UI: render description column values
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/bruno-app/src/components/EditableTable/index.js (1)
31-37:⚠️ Potential issue | 🟡 MinorColumn widths may become stale when columns are dynamically added/removed.
columnWidthsis initialized once from the initialcolumnsprop. When the description column is toggled on/off, the stored percentage widths (set after resize) for existing columns won't redistribute to accommodate the new column. This could cause layout overflow or underfill after a user has resized columns and then toggles the description column.Consider resetting
columnWidthswhen thecolumnsarray length changes:Proposed fix
+ useEffect(() => { + const initialWidths = {}; + columns.forEach((col) => { + initialWidths[col.key] = col.width || 'auto'; + }); + setColumnWidths(initialWidths); + }, [columns.length]);packages/bruno-app/src/components/EnvironmentVariablesTable/index.js (1)
70-79:⚠️ Potential issue | 🟡 MinorResize handler drops the
descriptionwidth key.
setColumnWidthson line 76-79 replaces the entire state object with only[columnKey]and[nextColumnKey](hardcoded to'value'). When the description column is visible,columnWidths.descriptionbecomesundefinedafter any resize, so the description column loses its explicit25%width (line 452/524).Proposed fix — spread existing widths
setColumnWidths({ + ...columnWidths, [columnKey]: `${startWidth + clampedDiff}px`, [nextColumnKey]: `${nextColumnStartWidth - clampedDiff}px` });Note: you'll need to add
columnWidthsto theuseCallbackdependency array or use a functional updater (setColumnWidths((prev) => ({ ...prev, ... }))).packages/bruno-converters/src/postman/bruno-to-postman.js (1)
231-243:⚠️ Potential issue | 🟠 Major
generateHeadersmust emitdescriptionfield to support bidirectional conversion and match form converters pattern.Looking at the codebase,
formUrlEncoded(line 263) andmultipartForm(line 276) both emitdescription: bodyItem.description || ''. Similarly, thepostman-to-brunoconverter reads header descriptions at line 468. The test suite also validates that header descriptions are preserved during conversion.Since Bruno headers now carry a
descriptionproperty and Postman's v2.1.0 schema supports it,generateHeadersshould emit the description field consistently with other body converters:return { key: item.name || '', value: item.value || '', disabled: !item.enabled, type: 'default', description: item.description || '' };packages/bruno-lang/v2/src/bruToJson.js (1)
278-297:⚠️ Potential issue | 🟡 MinorMissing
_.isString(pair.value)guard inmapPairListToKeyValPairsFile(line 283).The multipart equivalent on line 268 has a defensive
_.isString(pair.value)check before callingstartsWith. This file path is missing the same guard, which could throw ifpair.valueis unexpectedly non-string.🛡️ Add defensive check
- if (pair.value.startsWith('@file(') && pair.value.endsWith(')')) { + if (_.isString(pair.value) && pair.value.startsWith('@file(') && pair.value.endsWith(')')) {packages/bruno-lang/v2/src/jsonToBru.js (1)
186-205:⚠️ Potential issue | 🟡 MinorAdd
getDescriptionSuffixto metadata items for consistent description handling.Metadata items (lines 191, 199) don't include the description suffix while headers, params, formUrlEncoded, and vars all do. The converters already handle descriptions on metadata, so jsonToBru should output them consistently.
🤖 Fix all issues with AI agents
In `@packages/bruno-app/src/components/CollectionSettings/Headers/index.js`:
- Around line 146-152: The Description toggle button omits the shared
"btn-action" class causing inconsistent styling; update the button rendered in
the component that uses showDescriptionColumn and setShowDescriptionColumn to
include "btn-action" alongside the existing "text-link select-none" className so
it matches other toggles like QueryParams and VarsTable.
In `@packages/bruno-app/src/components/EditableTable/index.js`:
- Around line 238-240: handleAddRow can be a no-op visually because
rowsWithEmpty already shows a trailing empty row when showAddRow is true; either
stop rendering that inline empty row when an explicit "Add" action is present or
provide focus/scroll feedback to the newly persisted row. Update the
rowsWithEmpty logic to skip appending a virtual empty row when addRowLabel (or
showAddRow) is truthy so the Add button creates the only visible empty row, or
keep rowsWithEmpty but augment handleAddRow to call onChange([...rows,
createEmptyRow()]) then programmatically focus/scroll the newly added row (use a
row ref or a focusRow(index) helper) so users see the new persisted row; adjust
any rendering hooks that compute rowsWithEmpty and the Add button handler
(handleAddRow, createEmptyRow, rows, onChange, showAddRow, addRowLabel)
accordingly.
In `@packages/bruno-app/src/components/FolderSettings/Headers/index.js`:
- Around line 152-158: The toggle button for description in the Headers
component is missing the "btn-action" utility class which causes inconsistent
styling; update the button that calls setShowDescriptionColumn and reads
showDescriptionColumn so its className becomes "btn-action text-link
select-none" (matching the other toggles) to restore consistent styling.
In `@packages/bruno-app/src/components/RequestPane/FormUrlEncodedParams/index.js`:
- Around line 96-104: The Description toggle is placed above the table here but
should match the Headers components' placement below the table; move the button
rendering out of the current div above the table and render it alongside the
Bulk Edit controls below the params table, using the same state handlers
(showDescriptionColumn and setShowDescriptionColumn) and button className
("btn-action text-link select-none") so the toggle behavior and styling remain
unchanged; locate usages of showDescriptionColumn and setShowDescriptionColumn
in this component (FormUrlEncodedParams) and in
RequestHeaders/FolderSettings/Headers for reference to mirror layout and order.
🧹 Nitpick comments (11)
packages/bruno-app/src/components/RequestPane/FormUrlEncodedParams/index.js (1)
39-57: Significant code duplication:descriptionColumnis nearly identical across 8+ components.This exact pattern (key, name, placeholder, width, MultiLineEditor render) is repeated in FormUrlEncodedParams, MultipartFormParams, QueryParams, RequestHeaders, CollectionSettings/Headers, FolderSettings/Headers, and both VarsTable components. The only variation is whether
onRunanditemprops are passed.Consider extracting a shared factory, e.g.:
// utils/descriptionColumn.js const createDescriptionColumn = ({ storedTheme, onSave, onChange, onRun, collection, item }) => ({ ... });This would reduce the per-component boilerplate to a single call.
packages/bruno-app/src/components/RequestPane/MultipartFormParams/index.js (1)
194-216: Minor inconsistency: description column defined inline instead of as a separatedescriptionColumnconst.Other components in this PR extract the description column into a
const descriptionColumn = { ... }for readability. This inline definition works but breaks the pattern.packages/bruno-app/src/components/RequestPane/QueryParams/index.js (2)
39-53:pathParamsin the dependency array defeatsuseCallbackmemoization.
pathParamsis derived from a.filter()call on line 22, producing a new array reference every render. This means the callback is recreated on every render regardless, makinguseCallbacka no-op here.Consider using a stable reference (e.g.,
paramsoritem) and moving the.find()inside the callback to look up the path param at call time:Proposed fix
const handlePathParamChange = useCallback( (rowUid, key, value) => { + const currentParams = item.draft ? get(item, 'draft.request.params') : get(item, 'request.params'); + const pathParam = currentParams.find((p) => p.type === 'path' && p.uid === rowUid); - const pathParam = pathParams.find((p) => p.uid === rowUid); if (pathParam) { dispatch( updatePathParam({ pathParam: { ...pathParam, [key]: value }, itemUid: item.uid, collectionUid: collection.uid }) ); } }, - [dispatch, pathParams, item.uid, collection.uid] + [dispatch, item, collection.uid] );
67-105: Significant duplication betweendescriptionColumnQueryanddescriptionColumnPath.These two objects are nearly identical. The only difference is that the path variant ignores the
onChangeprop and callshandlePathParamChangedirectly. This is fine within a single file, but noting that this samedescriptionColumnpattern is also duplicated across 6+ other components (Headers, VarsTable, MultipartFormParams, FormUrlEncodedParams, etc.). Consider extracting a shared factory or helper.packages/bruno-app/src/components/CollectionSettings/Vars/VarsTable/index.js (1)
91-99: Toggle button placement differs from other components.Here the toggle is placed above the
EditableTable(lines 91-99), while inCollectionSettings/Headers/index.jsit's placed below (line 145-152). This creates an inconsistent UX within the same Collection Settings view. Consider aligning the placement.packages/bruno-converters/tests/postman/bruno-to-postman.spec.js (1)
196-219: Consider adding a test formultipartFormitems with description fallback.The
formUrlEncodedpath now has coverage for thedescriptionfield (lines 244-245), but themultipartFormpath only tests withnull(empty array). A parallel test with actual items would confirm thedescription: ''fallback forformdataentries as well.🧪 Example test to add
+ it('should handle null or undefined items in multipart form data', () => { + const simpleCollection = { + items: [ + { + name: 'Test Request', + type: 'http-request', + request: { + method: 'POST', + url: 'https://example.com', + body: { + mode: 'multipartForm', + multipartForm: [ + { name: null, value: 'test-value', enabled: true }, + { name: 'field', value: null, enabled: true } + ] + } + } + } + ] + }; + + const result = brunoToPostman(simpleCollection); + expect(result.item[0].request.body.formdata).toEqual([ + { key: '', value: 'test-value', disabled: false, type: 'default', description: '' }, + { key: 'field', value: '', disabled: false, type: 'default', description: '' } + ]); + });packages/bruno-lang/v2/src/collectionBruToJson.js (1)
82-98: DuplicatedextractDescription— consider importing fromsemantic-utils.js.This exact function is copy-pasted in 4 files:
semantic-utils.js,bruToJson.js,envToJson.js, and here. Sincesemantic-utils.jsalready exports it, importing from there would eliminate the maintenance burden of keeping all copies in sync.♻️ Suggested change
+const { extractDescription } = require('./common/semantic-utils'); -const extractDescription = (pair) => { - if (!_.isString(pair.value)) { - return; - } - const tripleMatch = pair.value.match(/\s*@description\('''([\s\S]*?)'''\)\s*$/); - if (tripleMatch) { - pair.description = tripleMatch[1].trim(); - pair.value = pair.value.slice(0, -tripleMatch[0].length).trim(); - return; - } - const doubleMatch = pair.value.match(/\s*@description\("([^"]*)"\)\s*$/); - if (doubleMatch) { - pair.description = doubleMatch[1].trim(); - pair.value = pair.value.slice(0, -doubleMatch[0].length).trim(); - return; - } -};packages/bruno-lang/v2/src/envToJson.js (1)
49-65: SameextractDescriptionduplication as other files — reuse fromsemantic-utils.js.See comment on
collectionBruToJson.js. This is the same function duplicated again.♻️ Suggested change
+const { extractDescription } = require('./common/semantic-utils'); -const extractDescription = (pair) => { - ... -};packages/bruno-lang/v2/src/jsonToBru.js (1)
9-16:getDescriptionSuffixis duplicated acrossjsonToBru.js,jsonToEnv.js, andjsonToCollectionBru.js.Consider extracting this to
utils.jsalongsidegetValueStringandgetKeyStringto keep a single source of truth.packages/bruno-lang/v2/src/common/semantic-utils.js (1)
132-148: RedundantextractDescriptioncall inmapPairListToKeyValPairsMultipart.
mapPairListToKeyValPairs(called on line 133) already invokesextractDescriptionon each pair (line 59). The second call on line 137 is a no-op since the@description(...)annotation has already been stripped frompair.value. Not harmful, but unnecessary work.♻️ Remove redundant call
return pairs.map((pair) => { pair.type = 'text'; - extractDescription(pair); multipartExtractContentType(pair);packages/bruno-lang/v2/src/jsonToCollectionBru.js (1)
8-15: Duplicated helper — consider extracting toutils.js.
getDescriptionSuffixis now identical in bothjsonToBru.jsandjsonToCollectionBru.js. Extracting it intoutils.jsalongsidegetValueStringandgetKeyStringwould keep a single source of truth.#!/bin/bash # Verify the duplication between the two files echo "=== jsonToBru.js ===" rg -n 'getDescriptionSuffix' --type=js -C2
| <button | ||
| type="button" | ||
| className="text-link select-none" | ||
| onClick={() => setShowDescriptionColumn((v) => !v)} | ||
| > | ||
| {showDescriptionColumn ? 'Hide Description' : 'Description'} | ||
| </button> |
There was a problem hiding this comment.
Missing btn-action class on the Description toggle button.
Other components (e.g., QueryParams line 191, VarsTable line 94, EnvironmentVariablesTable line 565) include btn-action alongside text-link select-none. This button omits it, causing inconsistent styling.
<button
type="button"
- className="text-link select-none"
+ className="btn-action text-link select-none"
onClick={() => setShowDescriptionColumn((v) => !v)}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| type="button" | |
| className="text-link select-none" | |
| onClick={() => setShowDescriptionColumn((v) => !v)} | |
| > | |
| {showDescriptionColumn ? 'Hide Description' : 'Description'} | |
| </button> | |
| <button | |
| type="button" | |
| className="btn-action text-link select-none" | |
| onClick={() => setShowDescriptionColumn((v) => !v)} | |
| > | |
| {showDescriptionColumn ? 'Hide Description' : 'Description'} | |
| </button> |
🤖 Prompt for AI Agents
In `@packages/bruno-app/src/components/CollectionSettings/Headers/index.js` around
lines 146 - 152, The Description toggle button omits the shared "btn-action"
class causing inconsistent styling; update the button rendered in the component
that uses showDescriptionColumn and setShowDescriptionColumn to include
"btn-action" alongside the existing "text-link select-none" className so it
matches other toggles like QueryParams and VarsTable.
| const handleAddRow = useCallback(() => { | ||
| onChange([...rows, createEmptyRow()]); | ||
| }, [rows, onChange, createEmptyRow]); |
There was a problem hiding this comment.
handleAddRow may produce a no-op visual effect when showAddRow is true.
When showAddRow is true, rowsWithEmpty already renders a trailing empty row (not persisted in rows). Clicking the "Add" button calls handleAddRow, which persists a new empty row into rows — but the visual result is the same empty row the user already saw. This could confuse users expecting a visible change.
Consider either hiding the inline empty row when addRowLabel is provided, or scrolling/focusing the newly added row to give feedback.
🤖 Prompt for AI Agents
In `@packages/bruno-app/src/components/EditableTable/index.js` around lines 238 -
240, handleAddRow can be a no-op visually because rowsWithEmpty already shows a
trailing empty row when showAddRow is true; either stop rendering that inline
empty row when an explicit "Add" action is present or provide focus/scroll
feedback to the newly persisted row. Update the rowsWithEmpty logic to skip
appending a virtual empty row when addRowLabel (or showAddRow) is truthy so the
Add button creates the only visible empty row, or keep rowsWithEmpty but augment
handleAddRow to call onChange([...rows, createEmptyRow()]) then programmatically
focus/scroll the newly added row (use a row ref or a focusRow(index) helper) so
users see the new persisted row; adjust any rendering hooks that compute
rowsWithEmpty and the Add button handler (handleAddRow, createEmptyRow, rows,
onChange, showAddRow, addRowLabel) accordingly.
| <button | ||
| type="button" | ||
| className="text-link select-none" | ||
| onClick={() => setShowDescriptionColumn((v) => !v)} | ||
| > | ||
| {showDescriptionColumn ? 'Hide Description' : 'Description'} | ||
| </button> |
There was a problem hiding this comment.
Missing btn-action class on the description toggle button.
All other description toggle buttons in this PR use className="btn-action text-link select-none", but this one omits btn-action. This will cause inconsistent button styling.
Proposed fix
<button
type="button"
- className="text-link select-none"
+ className="btn-action text-link select-none"
onClick={() => setShowDescriptionColumn((v) => !v)}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| type="button" | |
| className="text-link select-none" | |
| onClick={() => setShowDescriptionColumn((v) => !v)} | |
| > | |
| {showDescriptionColumn ? 'Hide Description' : 'Description'} | |
| </button> | |
| <button | |
| type="button" | |
| className="btn-action text-link select-none" | |
| onClick={() => setShowDescriptionColumn((v) => !v)} | |
| > | |
| {showDescriptionColumn ? 'Hide Description' : 'Description'} | |
| </button> |
🤖 Prompt for AI Agents
In `@packages/bruno-app/src/components/FolderSettings/Headers/index.js` around
lines 152 - 158, The toggle button for description in the Headers component is
missing the "btn-action" utility class which causes inconsistent styling; update
the button that calls setShowDescriptionColumn and reads showDescriptionColumn
so its className becomes "btn-action text-link select-none" (matching the other
toggles) to restore consistent styling.
| <div className="flex justify-end mt-2 mb-2"> | ||
| <button | ||
| type="button" | ||
| className="btn-action text-link select-none" | ||
| onClick={() => setShowDescriptionColumn((v) => !v)} | ||
| > | ||
| {showDescriptionColumn ? 'Hide Description' : 'Description'} | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
Toggle button placement is inconsistent with Headers components.
Here the description toggle is rendered above the table, while in RequestHeaders and FolderSettings/Headers it's rendered below the table alongside "Bulk Edit". Consider a consistent placement across all components for a uniform UX.
🤖 Prompt for AI Agents
In `@packages/bruno-app/src/components/RequestPane/FormUrlEncodedParams/index.js`
around lines 96 - 104, The Description toggle is placed above the table here but
should match the Headers components' placement below the table; move the button
rendering out of the current div above the table and render it alongside the
Bulk Edit controls below the params table, using the same state handlers
(showDescriptionColumn and setShowDescriptionColumn) and button className
("btn-action text-link select-none") so the toggle behavior and styling remain
unchanged; locate usages of showDescriptionColumn and setShowDescriptionColumn
in this component (FormUrlEncodedParams) and in
RequestHeaders/FolderSettings/Headers for reference to mirror layout and order.
b0ea18c to
e80c6c6
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bruno-app/src/components/EnvironmentVariablesTable/index.js (1)
70-79:⚠️ Potential issue | 🟠 MajorBug: Resize drops the
descriptionwidth from state.
setColumnWidthson line 76 replaces the entire state object with only the two columns being resized (nameandvalue). Thedescriptionkey initialized on line 51 is lost after any resize operation.After a resize,
columnWidths.descriptionbecomesundefined, and the description column at line 526 (style={{ width: columnWidths.description }}) loses its width styling.🐛 Proposed fix — spread existing widths
setColumnWidths({ + ...columnWidths, [columnKey]: `${startWidth + clampedDiff}px`, [nextColumnKey]: `${nextColumnStartWidth - clampedDiff}px` });Note: Since
handleResizeStarthas[]as its dependency array, you'd also need to either addcolumnWidthsto the deps or use the functional updater form:- setColumnWidths({ - [columnKey]: `${startWidth + clampedDiff}px`, - [nextColumnKey]: `${nextColumnStartWidth - clampedDiff}px` - }); + setColumnWidths((prev) => ({ + ...prev, + [columnKey]: `${startWidth + clampedDiff}px`, + [nextColumnKey]: `${nextColumnStartWidth - clampedDiff}px` + }));
🤖 Fix all issues with AI agents
In `@packages/bruno-app/src/components/EnvironmentVariablesTable/index.js`:
- Around line 563-580: The Description toggle button uses the wrong CSS classes
causing inconsistent styling; update the button for toggling description
visibility (the element that calls setShowDescriptionColumn) to use the same
classes as other toggles (e.g., "btn-action text-link select-none") instead of
"submit description-toggle ml-2" so it visually matches
MultipartFormParams/QueryParams/VarsTable; ensure the Save and Reset buttons
remain unchanged and keep the onClick handler that toggles showDescriptionColumn
intact.
In `@packages/bruno-app/src/components/RequestPane/MultipartFormParams/index.js`:
- Around line 229-237: The Description toggle button is rendered above the
EditableTable here, causing inconsistent UX; move the toggle block so it appears
after the EditableTable component. Locate the JSX that renders the button using
showDescriptionColumn and onClick={() => setShowDescriptionColumn((v) => !v)}
and cut/paste that <div> so it sits immediately below the <EditableTable />
render (matching placement used in CollectionSettings/Headers and VarsTable),
preserving the same className and handlers.
In `@packages/bruno-lang/v2/src/common/semantic-utils.js`:
- Around line 7-25: extractDescription currently captures description text but
doesn't reverse the escaping done by getDescriptionSuffix, and its single-line
regex fails to handle escaped quotes; update extractDescription to (1) use a
safer double-line regex for the single-line case (change
/\s*@description\("([^"]*)"\)\s*$/ to something like
/\s*@description\("((?:[^"\\]|\\.)*)"\)\s*$/) so escaped quotes are preserved in
the capture, and (2) after obtaining tripleMatch[1] or doubleMatch[1] unescape
sequences produced by getDescriptionSuffix by first replacing escaped
backslashes (\\ -> \) and then escaped quotes (\" -> ") before assigning to
pair.description, and then trim and remove the suffix from pair.value as before;
apply this to both tripleMatch and doubleMatch handling so round-trip saves do
not double-escape.
🧹 Nitpick comments (2)
packages/bruno-app/src/components/EnvironmentVariablesTable/StyledWrapper.js (1)
183-189: Inconsistent!importantusage between.secret-columnand.actions-column.
.secret-columnuses!importantto override thenth-childwidth rules, but.actions-columndoes not. If both classes are applied to<td>elements that also match annth-childselector with awidth, theactions-columnwidth may be silently overridden. Either both should use!importantor neither should (relying on specificity instead).♻️ Suggested consistency fix
.secret-column { width: 8% !important; } .actions-column { - width: 5%; + width: 5% !important; }packages/bruno-lang/v2/src/common/semantic-utils.js (1)
132-148:extractDescriptionis called twice on multipart pairs — redundant.
mapPairListToKeyValPairs(invoked at line 133) already callsextractDescriptionat line 59. The second call at line 137 is always a no-op since the annotation has already been stripped. Remove line 137.Proposed fix
return pairs.map((pair) => { pair.type = 'text'; - extractDescription(pair); multipartExtractContentType(pair);
| <div className="flex justify-end mt-2 mb-2"> | ||
| <button | ||
| type="button" | ||
| className="btn-action text-link select-none" | ||
| onClick={() => setShowDescriptionColumn((v) => !v)} | ||
| > | ||
| {showDescriptionColumn ? 'Hide Description' : 'Description'} | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
Description toggle placed above the table here, but below in other components.
In CollectionSettings/Headers, VarsTable, etc., the toggle button appears below the table. Here it's rendered above the EditableTable. This creates an inconsistent UX across the app.
Consider moving it below the table (after EditableTable) for consistency with the other editors in this PR.
🤖 Prompt for AI Agents
In `@packages/bruno-app/src/components/RequestPane/MultipartFormParams/index.js`
around lines 229 - 237, The Description toggle button is rendered above the
EditableTable here, causing inconsistent UX; move the toggle block so it appears
after the EditableTable component. Locate the JSX that renders the button using
showDescriptionColumn and onClick={() => setShowDescriptionColumn((v) => !v)}
and cut/paste that <div> so it sits immediately below the <EditableTable />
render (matching placement used in CollectionSettings/Headers and VarsTable),
preserving the same className and handlers.
| const extractDescription = (pair) => { | ||
| if (!_.isString(pair.value)) { | ||
| return; | ||
| } | ||
| // Multiline: @description('''...''') | ||
| const tripleMatch = pair.value.match(/\s*@description\('''([\s\S]*?)'''\)\s*$/); | ||
| if (tripleMatch) { | ||
| pair.description = tripleMatch[1].trim(); | ||
| pair.value = pair.value.slice(0, -tripleMatch[0].length).trim(); | ||
| return; | ||
| } | ||
| // Single-line: @description("...") | ||
| const doubleMatch = pair.value.match(/\s*@description\("([^"]*)"\)\s*$/); | ||
| if (doubleMatch) { | ||
| pair.description = doubleMatch[1].trim(); | ||
| pair.value = pair.value.slice(0, -doubleMatch[0].length).trim(); | ||
| return; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Round-trip escaping mismatch: getDescriptionSuffix escapes but extractDescription never unescapes.
getDescriptionSuffix applies replace(/\\/g, '\\\\') and replace(/"/g, '\\"') when serializing. But extractDescription returns the captured group verbatim without reversing these escapes. After one round-trip, every \ in a description becomes \\, and the corruption doubles on each subsequent save.
Add unescaping after extraction:
Proposed fix
const tripleMatch = pair.value.match(/\s*@description\('''([\s\S]*?)'''\)\s*$/);
if (tripleMatch) {
- pair.description = tripleMatch[1].trim();
+ pair.description = tripleMatch[1].trim().replace(/\\\\/g, '\\');
pair.value = pair.value.slice(0, -tripleMatch[0].length).trim();
return;
}
// Single-line: `@description`("...")
- const doubleMatch = pair.value.match(/\s*@description\("([^"]*)"\)\s*$/);
+ const doubleMatch = pair.value.match(/\s*@description\("((?:[^"\\]|\\.)*)"\)\s*$/);
if (doubleMatch) {
- pair.description = doubleMatch[1].trim();
+ pair.description = doubleMatch[1].trim().replace(/\\"/g, '"').replace(/\\\\/g, '\\');
pair.value = pair.value.slice(0, -doubleMatch[0].length).trim();
return;
}The double-quote regex is also changed from [^"]* to (?:[^"\\]|\\.)* to correctly handle escaped quotes (e.g., descriptions containing both ''' and ").
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const extractDescription = (pair) => { | |
| if (!_.isString(pair.value)) { | |
| return; | |
| } | |
| // Multiline: @description('''...''') | |
| const tripleMatch = pair.value.match(/\s*@description\('''([\s\S]*?)'''\)\s*$/); | |
| if (tripleMatch) { | |
| pair.description = tripleMatch[1].trim(); | |
| pair.value = pair.value.slice(0, -tripleMatch[0].length).trim(); | |
| return; | |
| } | |
| // Single-line: @description("...") | |
| const doubleMatch = pair.value.match(/\s*@description\("([^"]*)"\)\s*$/); | |
| if (doubleMatch) { | |
| pair.description = doubleMatch[1].trim(); | |
| pair.value = pair.value.slice(0, -doubleMatch[0].length).trim(); | |
| return; | |
| } | |
| }; | |
| const extractDescription = (pair) => { | |
| if (!_.isString(pair.value)) { | |
| return; | |
| } | |
| // Multiline: `@description`('''...''') | |
| const tripleMatch = pair.value.match(/\s*@description\('''([\s\S]*?)'''\)\s*$/); | |
| if (tripleMatch) { | |
| pair.description = tripleMatch[1].trim().replace(/\\\\/g, '\\'); | |
| pair.value = pair.value.slice(0, -tripleMatch[0].length).trim(); | |
| return; | |
| } | |
| // Single-line: `@description`("...") | |
| const doubleMatch = pair.value.match(/\s*@description\("((?:[^"\\]|\\.)*)"\)\s*$/); | |
| if (doubleMatch) { | |
| pair.description = doubleMatch[1].trim().replace(/\\"/g, '"').replace(/\\\\/g, '\\'); | |
| pair.value = pair.value.slice(0, -doubleMatch[0].length).trim(); | |
| return; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In `@packages/bruno-lang/v2/src/common/semantic-utils.js` around lines 7 - 25,
extractDescription currently captures description text but doesn't reverse the
escaping done by getDescriptionSuffix, and its single-line regex fails to handle
escaped quotes; update extractDescription to (1) use a safer double-line regex
for the single-line case (change /\s*@description\("([^"]*)"\)\s*$/ to something
like /\s*@description\("((?:[^"\\]|\\.)*)"\)\s*$/) so escaped quotes are
preserved in the capture, and (2) after obtaining tripleMatch[1] or
doubleMatch[1] unescape sequences produced by getDescriptionSuffix by first
replacing escaped backslashes (\\ -> \) and then escaped quotes (\" -> ") before
assigning to pair.description, and then trim and remove the suffix from
pair.value as before; apply this to both tripleMatch and doubleMatch handling so
round-trip saves do not double-escape.
…tions - Introduced a description column in Headers, VarsTable, and other components to allow multi-line descriptions. - Added toggle buttons to show/hide the description column in relevant tables. - Updated EditableTable component to support dynamic row addition with customizable labels. - Enhanced UI for better user experience with consistent button styles and spacing adjustments.
Pragadesh-45
force-pushed
the
feat/descriptions
branch
from
72f5d61 to
1b93b18
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/bruno-app/src/components/EnvironmentVariablesTable/StyledWrapper.js (1)
66-71:⚠️ Potential issue | 🟡 MinorDouble borders on header cells after adding
border-left.Line 70 adds
border-leftto everythead td, but line 69 already appliesborder-right. Adjacent cells will now render a 2px seam (right border of cell N + left border of cell N+1). If this is intentional for a visual effect, please disregard — otherwise, consider using onlyborder-right(or onlyborder-left) and handling the edge cell separately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/EnvironmentVariablesTable/StyledWrapper.js` around lines 66 - 71, The header cell CSS selector (thead td) is creating double seams because both border-right and border-left are applied; remove border-left from the thead td rule (or keep only border-left and remove border-right) and instead add a single-border rule for the table edge using a pseudo/edge selector (e.g., thead td:first-child or thead td:last-child) to ensure only one border between adjacent header cells; update the StyledWrapper.js thead td rule accordingly and adjust any tests/styles that expect the previous double-border behavior.packages/bruno-app/src/components/EnvironmentVariablesTable/index.js (2)
63-66:⚠️ Potential issue | 🟡 Minor
nextColumnKeyis hardcoded to'value'— resize logic ignores description column.When
showDescriptionColumnis true, there are more columns in play, butnextColumnKeyis always'value'. If the user resizes thenamecolumn, thevaluecolumn shrinks as expected. But the description column's resize handle (if any) also targets'value', which is incorrect. Consider derivingnextColumnKeyfrom the actual next sibling column's data attribute or from the column order.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/EnvironmentVariablesTable/index.js` around lines 63 - 66, The resize handler currently hardcodes nextColumnKey = 'value', causing wrong column targeting when showDescriptionColumn is true; update the logic in the resize start (where startX, startWidth, nextCell are set) to derive nextColumnKey from the actual next sibling column element or from the columns order (e.g., read a data-column-key attribute on nextCell or use the columns array to find the column after currentCell) so the resize updates the correct column (name -> value or name -> description depending on showDescriptionColumn) and adjust any code paths that reference nextColumnKey accordingly.
70-79:⚠️ Potential issue | 🟠 MajorResize handler drops
descriptionwidth — column collapses after any resize.
setColumnWidthson lines 76-79 sets onlynameandvaluekeys, overwriting the entire state object. Thedescriptionkey from the initial state (line 51) is lost. After any column resize, the description column will lose its25%width and render with no explicitwidthstyle.Spread existing state or include the
descriptionkey:Proposed fix
- setColumnWidths({ - [columnKey]: `${startWidth + clampedDiff}px`, - [nextColumnKey]: `${nextColumnStartWidth - clampedDiff}px` - }); + setColumnWidths((prev) => ({ + ...prev, + [columnKey]: `${startWidth + clampedDiff}px`, + [nextColumnKey]: `${nextColumnStartWidth - clampedDiff}px` + }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/EnvironmentVariablesTable/index.js` around lines 70 - 79, handleMouseMove currently calls setColumnWidths with an object containing only [columnKey] and [nextColumnKey], which overwrites the entire widths state and drops the existing description width; change the update to merge with the existing state (e.g., use functional setState and spread prev: setColumnWidths(prev => ({ ...prev, [columnKey]: ..., [nextColumnKey]: ... }))) so description (and any other keys) are preserved; locate handleMouseMove and the setColumnWidths call, and use the functional updater pattern referencing columnKey, nextColumnKey, startWidth, nextColumnStartWidth and MIN_COLUMN_WIDTH to apply the clamped widths without replacing other keys.
🧹 Nitpick comments (14)
packages/bruno-app/src/components/RequestPane/QueryParams/index.js (4)
67-105: Near-duplicate column definitions for description.
descriptionColumnQueryanddescriptionColumnPathshare the samekey,name,placeholder,width, and nearly identicalrenderbodies — the only real difference is howonChangeis handled. Consider extracting a shared factory:♻️ Optional: shared factory
- const descriptionColumnQuery = { - key: 'description', - name: 'Description', - placeholder: 'Description', - width: '25%', - render: ({ value, onChange }) => ( - <MultiLineEditor - value={value || ''} - theme={storedTheme} - onSave={onSave} - onChange={onChange} - allowNewlines={true} - onRun={handleRun} - collection={collection} - item={item} - placeholder={!value ? 'Description' : ''} - /> - ) - }; - - const descriptionColumnPath = { - key: 'description', - name: 'Description', - placeholder: 'Description', - width: '25%', - render: ({ row, value, onChange }) => ( - <MultiLineEditor - value={value || ''} - theme={storedTheme} - onSave={onSave} - onChange={(newValue) => handlePathParamChange(row.uid, 'description', newValue)} - allowNewlines={true} - onRun={handleRun} - collection={collection} - item={item} - placeholder={!value ? 'Description' : ''} - /> - ) - }; + const makeDescriptionColumn = (onChangeFn) => ({ + key: 'description', + name: 'Description', + placeholder: 'Description', + width: '25%', + render: ({ row, value, onChange }) => ( + <MultiLineEditor + value={value || ''} + theme={storedTheme} + onSave={onSave} + onChange={onChangeFn ? (newValue) => onChangeFn(row.uid, 'description', newValue) : onChange} + allowNewlines={true} + onRun={handleRun} + collection={collection} + item={item} + placeholder={!value ? 'Description' : ''} + /> + ) + }); + + const descriptionColumnQuery = makeDescriptionColumn(null); + const descriptionColumnPath = makeDescriptionColumn(handlePathParamChange);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/QueryParams/index.js` around lines 67 - 105, descriptionColumnQuery and descriptionColumnPath are almost identical and should be merged via a factory to avoid duplication and accidental drift; create a helper like createDescriptionColumn({ onChangeOverride }) or createDescriptionColumn({ isPath }) that returns the column object used for both cases, keeping shared properties (key, name, placeholder, width, theme, onSave, allowNewlines, onRun, collection, item, placeholder logic) and injecting the differing onChange behavior (either the passed onChange or a wrapper that calls handlePathParamChange(row.uid, 'description', newValue)); update the uses of descriptionColumnQuery and descriptionColumnPath to call this factory and remove the duplicated definitions.
187-196: Toggle button label could be clearer when collapsed.When the column is hidden the label reads just
"Description", which doesn't immediately signal an action. Consider"Show Description"/"Hide Description"for symmetry so users know it's a toggle.♻️ Suggested tweak
- {showDescriptionColumn ? 'Hide Description' : 'Description'} + {showDescriptionColumn ? 'Hide Description' : 'Show Description'}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/QueryParams/index.js` around lines 187 - 196, The toggle button currently shows "Description" when the column is hidden; update the button label logic in the QueryParams component to be explicit by using the showDescriptionColumn state so it reads "Show Description" when showDescriptionColumn is false and "Hide Description" when true; locate the JSX that renders the button (the onClick uses setShowDescriptionColumn) and replace the existing ternary/string with a clear "Show Description" / "Hide Description" pair so the action is unambiguous to users.
92-97:onChangeprop silently ignored — worth a brief inline comment.
descriptionColumnPath.renderreceivesonChangefrom EditableTable but deliberately bypasses it in favor ofhandlePathParamChange. This is correct (path params use a per-field dispatch), but a one-liner comment would save future readers a head-scratch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/QueryParams/index.js` around lines 92 - 97, descriptionColumnPath.render currently receives an onChange prop from EditableTable but intentionally ignores it by calling handlePathParamChange instead; add a concise inline comment next to the MultiLineEditor onChange (or above descriptionColumnPath.render) stating that the prop is deliberately bypassed because path params use a per-field dispatch via handlePathParamChange(row.uid, 'description', ...) so future readers won't be confused. Keep the comment to one line and reference the use of handlePathParamChange and row.uid for clarity.
39-53:useCallbackis effectively a no-op here.
pathParams(line 52) is derived via.filter()on line 22, producing a new array reference every render. This means the dependency array always changes and the callback is never truly memoized.Two options to actually get memoization benefit:
- Move the
pathParams.find()lookup out of the callback and passuiddirectly to the dispatch, performing the lookup at the reducer level.- Use
useMemoto stabilizepathParamsfirst, then reference it in this callback's deps.Not a bug — just dead memoization.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/QueryParams/index.js` around lines 39 - 53, handlePathParamChange is never truly memoized because pathParams is recreated each render; either move the find() lookup into the reducer action (so handlePathParamChange sends the rowUid and key/value to dispatch and let updatePathParam resolve the pathParam by uid), or stabilize pathParams with useMemo before using it in the useCallback dependency array. Specifically, either change handlePathParamChange to dispatch({ type: updatePathParam, payload: { uid: rowUid, key, value }, itemUid: item.uid, collectionUid: collection.uid }) and resolve the item in the reducer, or wrap the pathParams filter in useMemo (e.g. const pathParams = useMemo(() => originalPathParams.filter(...), [originalPathParams]) ) and keep the existing handlePathParamChange that references pathParams, dispatch, item.uid, collection.uid.packages/bruno-app/src/components/FolderSettings/Vars/VarsTable/index.js (1)
38-55: Optional: memoizedescriptionColumnwithuseMemo.
descriptionColumn(and itsrenderclosure) is recreated on every render. IfEditableTableuses column reference equality for bailouts, this will defeat them. WhileonSaveitself is also not memoized (pre-existing), wrappingdescriptionColumninuseMemowould at least scope the instability.♻️ Proposed refactor
- const descriptionColumn = { - key: 'description', - name: 'Description', - placeholder: 'Description', - width: '25%', - render: ({ value, onChange }) => ( - <MultiLineEditor - value={value || ''} - theme={storedTheme} - onSave={onSave} - onChange={onChange} - allowNewlines={true} - collection={collection} - item={folder} - placeholder={!value ? 'Description' : ''} - /> - ) - }; + const descriptionColumn = useMemo(() => ({ + key: 'description', + name: 'Description', + placeholder: 'Description', + width: '25%', + render: ({ value, onChange }) => ( + <MultiLineEditor + value={value || ''} + theme={storedTheme} + onSave={onSave} + onChange={onChange} + allowNewlines={true} + collection={collection} + item={folder} + placeholder={!value ? 'Description' : ''} + /> + ) + }), [storedTheme, onSave, collection, folder]);Note:
useMemois already imported from React in this file (viauseCallback). You'd need to add it to the import on line 1.Based on learnings: "Memoize only when necessary (
useMemo/useCallback), and prefer moving logic into hooks first (SHOULD)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/FolderSettings/Vars/VarsTable/index.js` around lines 38 - 55, The descriptionColumn object (including its render closure that returns MultiLineEditor) is recreated on every render which can break EditableTable’s column reference equality optimizations; wrap the descriptionColumn creation in React's useMemo (and add useMemo to the existing React imports) so the column reference is stable unless its dependencies change (e.g., storedTheme, onSave, collection, folder); ensure the memo dependency array includes storedTheme, onSave, collection and folder (and any values used inside render) so the column is recomputed only when needed.packages/bruno-app/src/components/CollectionSettings/Vars/VarsTable/index.js (1)
33-49:descriptionColumnis always constructed even when hidden.The object and its
renderclosure are recreated on every render regardless ofshowDescriptionColumn. Sincecolumnsis also not memoized (existing pattern), this is low-priority — but wrapping inuseMemowith[storedTheme, onSave, collection]deps would align with the project's "memoize when beneficial" guideline.♻️ Proposed memoization
- const descriptionColumn = { + const descriptionColumn = useMemo(() => ({ key: 'description', name: 'Description', placeholder: 'Description', width: '25%', render: ({ value, onChange }) => ( <MultiLineEditor value={value || ''} theme={storedTheme} onSave={onSave} onChange={onChange} allowNewlines={true} collection={collection} placeholder={!value ? 'Description' : ''} /> ) - }; + }), [storedTheme, onSave, collection]);Based on learnings: "Memoize only when necessary (
useMemo/useCallback), and prefer moving logic into hooks first (SHOULD)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/CollectionSettings/Vars/VarsTable/index.js` around lines 33 - 49, descriptionColumn is rebuilt on every render even when hidden; wrap its creation in useMemo and only return the object when showDescriptionColumn is true. Use React.useMemo(() => { ... }, [storedTheme, onSave, collection]) so the render closure passed to MultiLineEditor captures stable storedTheme, onSave and collection, and skip creating descriptionColumn when showDescriptionColumn is false (return null/undefined) so columns assembly can conditionally include it without recreating the function each render.packages/bruno-app/src/components/RequestPane/Vars/VarsTable/index.js (1)
48-66: SamedescriptionColumnmemoization opportunity as the CollectionSettings variant.Same rationale applies here; deps would be
[storedTheme, onSave, handleRun, collection, item].♻️ Proposed memoization
- const descriptionColumn = { + const descriptionColumn = useMemo(() => ({ key: 'description', name: 'Description', placeholder: 'Description', width: '25%', render: ({ value, onChange }) => ( <MultiLineEditor value={value || ''} theme={storedTheme} onSave={onSave} onChange={onChange} allowNewlines={true} onRun={handleRun} collection={collection} item={item} placeholder={!value ? 'Description' : ''} /> ) - }; + }), [storedTheme, onSave, handleRun, collection, item]);Note:
useMemois already imported viauseCallbacksibling — just add it to the import destructuring.Based on learnings: "Memoize only when necessary (
useMemo/useCallback), and prefer moving logic into hooks first (SHOULD)."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/RequestPane/Vars/VarsTable/index.js` around lines 48 - 66, descriptionColumn is recreated on every render; wrap its definition in useMemo and include the dependencies [storedTheme, onSave, handleRun, collection, item] so the rendered MultiLineEditor props are stable, and add useMemo to the import list; keep the same shape and return value but memoize the const descriptionColumn = useMemo(() => ({ key: 'description', name: 'Description', placeholder: 'Description', width: '25%', render: ({ value, onChange }) => (<MultiLineEditor ... />) }), [storedTheme, onSave, handleRun, collection, item]).packages/bruno-lang/v2/src/jsonToEnv.js (1)
4-11:getDescriptionSuffixis duplicated verbatim fromjsonToBru.js— move it toutils.js.
packages/bruno-lang/v2/src/jsonToBru.jslines 8-15 already define an identical function (same logic, same escaping, only parameter name differs). Keeping two copies will diverge silently when either needs a bug fix.Move
getDescriptionSuffixintoutils.jsand export it, then import it in bothjsonToBru.jsandjsonToEnv.js.♻️ Proposed refactor
In
utils.js, add and export the shared helper:+/** + * Returns a formatted `@description`(...) suffix for serialization. + * Uses triple-quote syntax unless the description itself contains triple quotes, + * in which case it falls back to double-quote syntax. + * `@param` {{ description?: string }} item + * `@returns` {string} + */ +const getDescriptionSuffix = (item) => { + const desc = item && item.description && String(item.description).trim(); + if (!desc) return ''; + if (desc.includes('\'\'\'')) { + return ' `@description`("' + desc.replace(/\\/g, '\\\\').replace(/"/g, '\\"') + '")'; + } + return ' `@description`(\'\'\'' + desc.replace(/\\/g, '\\\\') + '\'\'\')'; +}; + +module.exports = { indentString, getValueString, getDescriptionSuffix };In
jsonToEnv.js:-const { getValueString, indentString } = require('./utils'); +const { getValueString, indentString, getDescriptionSuffix } = require('./utils'); -const getDescriptionSuffix = (variable) => { - const desc = variable && variable.description && String(variable.description).trim(); - if (!desc) return ''; - if (desc.includes('\'\'\'')) { - return ' `@description`("' + desc.replace(/\\/g, '\\\\').replace(/"/g, '\\"') + '")'; - } - return ' `@description`(\'\'\'' + desc.replace(/\\/g, '\\\\') + '\'\'\')'; -};Apply the same import change in
jsonToBru.js.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-lang/v2/src/jsonToEnv.js` around lines 4 - 11, The getDescriptionSuffix function is duplicated in jsonToEnv.js and jsonToBru.js; extract it into packages/bruno-lang/v2/src/utils.js as an exported helper (preserve its current logic and escaping behavior and keep the function name getDescriptionSuffix), then remove the duplicate definitions from jsonToEnv.js and jsonToBru.js and replace them with imports that import { getDescriptionSuffix } from './utils'; so both modules use the single shared implementation.packages/bruno-app/src/components/EnvironmentVariablesTable/StyledWrapper.js (1)
183-189:secret-columnuses!importantbutactions-columndoes not.Inconsistent specificity approach. If
!importantis needed on.secret-columnto override thenth-childwidth, the same concern applies to.actions-column. Either both should use!importantor neither should (by increasing selector specificity instead).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/EnvironmentVariablesTable/StyledWrapper.js` around lines 183 - 189, The .secret-column currently uses !important while .actions-column does not, causing inconsistent specificity; update StyledWrapper.js so both column rules use consistent specificity — either remove the !important from .secret-column and increase selector specificity for both (.secret-column and .actions-column) to override the nth-child width, or add !important to .actions-column as well; locate and change the styles for .secret-column and .actions-column so they match in specificity and reliably override the nth-child selector.packages/bruno-app/src/components/EnvironmentVariablesTable/index.js (1)
427-432: Description search filter — minor redundancy.
variable.description && typeof variable.description === 'string'— the&&with a truthy check is redundant if you're already checkingtypeof ... === 'string'(a non-empty string is truthy, buttypeof '' === 'string'is also true, so the first clause excludes''unnecessarily). An empty description won't match any real query anyway, so it's functionally fine, just slightly misleading.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-app/src/components/EnvironmentVariablesTable/index.js` around lines 427 - 432, Summary: descriptionMatch uses a redundant truthy check before typeof; remove it. Update the filter expression in the EnvironmentVariablesTable component to compute descriptionMatch by only checking typeof variable.description === 'string' (and then calling .toLowerCase().includes(query) if true), so empty strings aren't specially excluded and the logic is clearer; adjust the variable named descriptionMatch and the return that uses nameMatch, valueMatch, descriptionMatch accordingly.packages/bruno-lang/v2/src/common/semantic-utils.js (1)
133-147: Redundant doubleextractDescriptioncall inmapPairListToKeyValPairsMultipart.
mapPairListToKeyValPairs(Line 133) already callsextractDescriptionon each pair whenparseEnabled = true(the default). The explicit call at Line 137 is always a no-op for that path — the annotation is already stripped before reaching it.The call is only meaningful when
parseEnabled = false, which skips the extraction in the inner function. Consider calling it conditionally:♻️ Suggested fix
return pairs.map((pair) => { pair.type = 'text'; - extractDescription(pair); + if (!parseEnabled) extractDescription(pair); multipartExtractContentType(pair);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-lang/v2/src/common/semantic-utils.js` around lines 133 - 147, In mapPairListToKeyValPairsMultipart, remove the redundant unconditional call to extractDescription for each pair (currently after mapping pairs from mapPairListToKeyValPairs) and instead only call extractDescription when parseEnabled is false; reference mapPairListToKeyValPairs (which already extracts descriptions when parseEnabled === true), the local parseEnabled flag, and the extractDescription function to implement the conditional so descriptions are only stripped once.packages/bruno-lang/v2/src/jsonToCollectionBru.js (1)
8-15:getDescriptionSuffixis duplicated across three files.Identical implementations exist in
jsonToBru.js(Lines 9–16) andjsonToEnv.js(Lines 3–10). Extract it to a shared utility (e.g.,common/semantic-utils.jsorutils.js) and import from there.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-lang/v2/src/jsonToCollectionBru.js` around lines 8 - 15, getDescriptionSuffix is duplicated across jsonToCollectionBru.js, jsonToBru.js, and jsonToEnv.js; extract this function into a shared utility module (e.g., common/semantic-utils or utils) and replace the copies with imports. Create a new exported function getDescriptionSuffix in the shared module using the same implementation (preserve string escaping logic and return values), update jsonToCollectionBru.js, jsonToBru.js, and jsonToEnv.js to import getDescriptionSuffix and remove their local implementations, and run tests/lint to ensure no references remain to the old local functions.packages/bruno-lang/v2/src/collectionBruToJson.js (1)
82-98:extractDescriptionduplicated instead of using the shared export.An identical implementation was extracted to
common/semantic-utils.jsand exported precisely for reuse. This local copy (and matching ones inbruToJson.jsandenvToJson.js) should import from the shared module instead of redefining it.♻️ Proposed fix
+const { extractDescription } = require('./common/semantic-utils'); + -const extractDescription = (pair) => { - if (!_.isString(pair.value)) { - return; - } - const tripleMatch = pair.value.match(/\s*@description\('''([\s\S]*?)'''\)\s*$/); - if (tripleMatch) { - pair.description = tripleMatch[1].trim(); - pair.value = pair.value.slice(0, -tripleMatch[0].length).trim(); - return; - } - const doubleMatch = pair.value.match(/\s*@description\("([^"]*)"\)\s*$/); - if (doubleMatch) { - pair.description = doubleMatch[1].trim(); - pair.value = pair.value.slice(0, -doubleMatch[0].length).trim(); - return; - } -};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-lang/v2/src/collectionBruToJson.js` around lines 82 - 98, Remove the local duplicated extractDescription function and instead import and use the shared exported helper from the common semantic utilities module; locate the local definition named extractDescription in this file and delete it, add an import for the exported extractDescription from the shared module (the same symbol used by common/semantic-utils), and ensure references in this file (and the other duplicates in bruToJson.js/envToJson.js) call that imported extractDescription so behavior remains identical.packages/bruno-lang/v2/tests/envToJson.spec.js (1)
38-59: New test covers the happy path but misses key edge cases.Two gaps worth adding:
- Disabled vars with descriptions —
~url: value@description("desc")should be parsed asenabled: falsewith description intact. This is a likely regression point.secretfield not asserted — all other tests verifysecret: falsefor non-secret vars; usingtoMatchObjectsilently omits that check.♻️ Suggested additions
+ it('should parse `@description` on disabled var', () => { + const input = ` +vars { + ~url: http://localhost:3000 `@description`("Disabled endpoint") +}`; + const output = parser(input); + expect(output.variables[0]).toMatchObject({ + name: 'url', + value: 'http://localhost:3000', + enabled: false, + secret: false, + description: 'Disabled endpoint' + }); + });As per coding guidelines: "Cover both the 'happy path' and the realistically problematic paths."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bruno-lang/v2/tests/envToJson.spec.js` around lines 38 - 59, Update the test for parsing `@description` to include the two missing edge cases: add a case where a disabled variable (e.g., "~url: http://... `@description`('desc')") is parsed and assert that parser(...) returns that variable with enabled: false and description preserved (check output.variables[n].enabled and .description); also assert the secret field for non-secret vars (e.g., expect(output.variables[n].secret).toBe(false)) instead of relying solely on toMatchObject so the secret flag is explicitly verified. Use the same parser invocation and output.variables indexing used in the spec to locate where to add these assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/bruno-app/src/components/EnvironmentVariablesTable/StyledWrapper.js`:
- Around line 45-58: The CSS rules for the description column are nested inside
the table block so the compiled selector becomes `table.has-description-column
td` which never matches because the `has-description-column` class is on the
StyledWrapper wrapper, not the table; move that whole block out of the `table {
... }` scope and change the selector to target the table as a descendant of the
wrapper (e.g. use `&.has-description-column table td { &:nth-child(4) { ... }
&:nth-child(5) { ... } &:nth-child(6) { ... } }`) so the nth-child(4/5/6) rules
apply when the wrapper has the `has-description-column` class.
In `@packages/bruno-app/src/components/RequestPane/MultipartFormParams/index.js`:
- Around line 193-216: MultipartFormParams is setting fixed column widths that
sum to 110% when showDescriptionColumn is true (resulting in horizontal
overflow); update the columns definition (the array that includes the column
with key 'description') to avoid hard-coding widths that force overflow—either
remove the explicit width on the 'value' column so it can flex (matching
RequestHeaders/FormUrlEncodedParams behavior) or reduce the widths of existing
columns proportionally when showDescriptionColumn is true; adjust the
render/column objects for keys like 'value' and 'description' in the
MultipartFormParams component so the table no longer exceeds 100% width.
In `@packages/bruno-lang/v2/src/jsonToCollectionBru.js`:
- Around line 8-15: getDescriptionSuffix currently embeds raw newlines and
mismatched escapes into BRU output causing parser breakage; update
getDescriptionSuffix to escape newlines (replace \n with \\n) and preserve
backslashes (\\\\) when building the `@description`(...) token, and update the
parser by implementing an extractDescription function that unescapes \\n →
newline and \\\\ → \\ when reading descriptions; additionally, adjust the
grammar by adding an optional descriptionannotation? to the pair rule (mirroring
the contenttypeannotation change on multilinetextblock) so description
annotations are allowed after multiline values, and ensure getValueString and
the parser consistently round-trip these escaped sequences.
---
Outside diff comments:
In `@packages/bruno-app/src/components/EnvironmentVariablesTable/index.js`:
- Around line 63-66: The resize handler currently hardcodes nextColumnKey =
'value', causing wrong column targeting when showDescriptionColumn is true;
update the logic in the resize start (where startX, startWidth, nextCell are
set) to derive nextColumnKey from the actual next sibling column element or from
the columns order (e.g., read a data-column-key attribute on nextCell or use the
columns array to find the column after currentCell) so the resize updates the
correct column (name -> value or name -> description depending on
showDescriptionColumn) and adjust any code paths that reference nextColumnKey
accordingly.
- Around line 70-79: handleMouseMove currently calls setColumnWidths with an
object containing only [columnKey] and [nextColumnKey], which overwrites the
entire widths state and drops the existing description width; change the update
to merge with the existing state (e.g., use functional setState and spread prev:
setColumnWidths(prev => ({ ...prev, [columnKey]: ..., [nextColumnKey]: ... })))
so description (and any other keys) are preserved; locate handleMouseMove and
the setColumnWidths call, and use the functional updater pattern referencing
columnKey, nextColumnKey, startWidth, nextColumnStartWidth and MIN_COLUMN_WIDTH
to apply the clamped widths without replacing other keys.
In
`@packages/bruno-app/src/components/EnvironmentVariablesTable/StyledWrapper.js`:
- Around line 66-71: The header cell CSS selector (thead td) is creating double
seams because both border-right and border-left are applied; remove border-left
from the thead td rule (or keep only border-left and remove border-right) and
instead add a single-border rule for the table edge using a pseudo/edge selector
(e.g., thead td:first-child or thead td:last-child) to ensure only one border
between adjacent header cells; update the StyledWrapper.js thead td rule
accordingly and adjust any tests/styles that expect the previous double-border
behavior.
---
Duplicate comments:
In `@packages/bruno-app/src/components/EnvironmentVariablesTable/index.js`:
- Around line 566-583: The Description toggle button styling is inconsistent:
update the button that toggles showDescriptionColumn (the onClick using
setShowDescriptionColumn and the label rendering showDescriptionColumn ? 'Hide
Description' : 'Description') to use the same classNames as other components
(replace "submit description-toggle ml-2" with "btn-action text-link
select-none" and preserve spacing utility classes like ml-2 if needed) so its
visual appearance matches the rest of the PR.
In `@packages/bruno-app/src/components/RequestPane/FormUrlEncodedParams/index.js`:
- Around line 96-104: The description-toggle button is rendered above the params
table but should be grouped with the "Bulk Edit" controls below the table to
match RequestHeaders and FolderSettings/Headers; remove the existing div that
renders the toggle (the element using showDescriptionColumn and
setShowDescriptionColumn) and instead render that same toggle button alongside
the "Bulk Edit" control in the footer/controls area beneath the table (the same
container used for Bulk Edit), preserving the onClick handler that toggles
setShowDescriptionColumn and the displayed label based on showDescriptionColumn
so the behavior is unchanged but placement matches sibling components.
In `@packages/bruno-app/src/components/RequestPane/MultipartFormParams/index.js`:
- Around line 229-237: The Description toggle div is placed above the
EditableTable causing inconsistent UI; move the entire <div className="flex
justify-end mt-2 mb-2"> block (the button that uses showDescriptionColumn and
setShowDescriptionColumn) so it appears immediately after the <EditableTable />
component in this file, mirroring placement used in Headers and VarsTable;
ensure the onClick still toggles showDescriptionColumn and no other JSX is
changed.
In `@packages/bruno-lang/v2/src/bruToJson.js`:
- Around line 163-179: The local extractDescription function is duplicated and
should be removed and replaced by importing the shared implementation from
common/semantic-utils.js; delete the extractDescription definition in
bruToJson.js, add an import for extractDescription from the shared module, and
update any local references to use the imported extractDescription (ensure the
import symbol name matches usages). Also remove any lodash/string checks that
the shared function already handles so there are no redundant guards.
In `@packages/bruno-lang/v2/src/common/semantic-utils.js`:
- Around line 7-25: extractDescription currently strips `@description` blocks but
doesn't reverse the escapes added by getDescriptionSuffix, causing saved
descriptions to be corrupted; modify extractDescription (the function named
extractDescription) to unescape escaped backslashes and double-quotes when
assigning pair.description for both tripleMatch and doubleMatch cases (e.g.,
convert \\ to \ and \" to "), and then trim and remove the matched suffix from
pair.value as before so round-trip serialization/deserialization preserves
original text.
In `@packages/bruno-lang/v2/src/jsonToBru.js`:
- Around line 9-16: The getDescriptionSuffix function is duplicated (also
present in jsonToCollectionBru.js); extract it into a shared utility module
(e.g., export a getDescriptionSuffix function from a new or existing utils
file), replace the inline implementation in both jsonToBru.js and
jsonToCollectionBru.js with imports of that shared function, and ensure the
exported function preserves the current behavior (handling null/empty,
triple-quote detection, and proper backslash/quote escaping) and is used
wherever getDescriptionSuffix was referenced.
---
Nitpick comments:
In
`@packages/bruno-app/src/components/CollectionSettings/Vars/VarsTable/index.js`:
- Around line 33-49: descriptionColumn is rebuilt on every render even when
hidden; wrap its creation in useMemo and only return the object when
showDescriptionColumn is true. Use React.useMemo(() => { ... }, [storedTheme,
onSave, collection]) so the render closure passed to MultiLineEditor captures
stable storedTheme, onSave and collection, and skip creating descriptionColumn
when showDescriptionColumn is false (return null/undefined) so columns assembly
can conditionally include it without recreating the function each render.
In `@packages/bruno-app/src/components/EnvironmentVariablesTable/index.js`:
- Around line 427-432: Summary: descriptionMatch uses a redundant truthy check
before typeof; remove it. Update the filter expression in the
EnvironmentVariablesTable component to compute descriptionMatch by only checking
typeof variable.description === 'string' (and then calling
.toLowerCase().includes(query) if true), so empty strings aren't specially
excluded and the logic is clearer; adjust the variable named descriptionMatch
and the return that uses nameMatch, valueMatch, descriptionMatch accordingly.
In
`@packages/bruno-app/src/components/EnvironmentVariablesTable/StyledWrapper.js`:
- Around line 183-189: The .secret-column currently uses !important while
.actions-column does not, causing inconsistent specificity; update
StyledWrapper.js so both column rules use consistent specificity — either remove
the !important from .secret-column and increase selector specificity for both
(.secret-column and .actions-column) to override the nth-child width, or add
!important to .actions-column as well; locate and change the styles for
.secret-column and .actions-column so they match in specificity and reliably
override the nth-child selector.
In `@packages/bruno-app/src/components/FolderSettings/Vars/VarsTable/index.js`:
- Around line 38-55: The descriptionColumn object (including its render closure
that returns MultiLineEditor) is recreated on every render which can break
EditableTable’s column reference equality optimizations; wrap the
descriptionColumn creation in React's useMemo (and add useMemo to the existing
React imports) so the column reference is stable unless its dependencies change
(e.g., storedTheme, onSave, collection, folder); ensure the memo dependency
array includes storedTheme, onSave, collection and folder (and any values used
inside render) so the column is recomputed only when needed.
In `@packages/bruno-app/src/components/RequestPane/QueryParams/index.js`:
- Around line 67-105: descriptionColumnQuery and descriptionColumnPath are
almost identical and should be merged via a factory to avoid duplication and
accidental drift; create a helper like createDescriptionColumn({
onChangeOverride }) or createDescriptionColumn({ isPath }) that returns the
column object used for both cases, keeping shared properties (key, name,
placeholder, width, theme, onSave, allowNewlines, onRun, collection, item,
placeholder logic) and injecting the differing onChange behavior (either the
passed onChange or a wrapper that calls handlePathParamChange(row.uid,
'description', newValue)); update the uses of descriptionColumnQuery and
descriptionColumnPath to call this factory and remove the duplicated
definitions.
- Around line 187-196: The toggle button currently shows "Description" when the
column is hidden; update the button label logic in the QueryParams component to
be explicit by using the showDescriptionColumn state so it reads "Show
Description" when showDescriptionColumn is false and "Hide Description" when
true; locate the JSX that renders the button (the onClick uses
setShowDescriptionColumn) and replace the existing ternary/string with a clear
"Show Description" / "Hide Description" pair so the action is unambiguous to
users.
- Around line 92-97: descriptionColumnPath.render currently receives an onChange
prop from EditableTable but intentionally ignores it by calling
handlePathParamChange instead; add a concise inline comment next to the
MultiLineEditor onChange (or above descriptionColumnPath.render) stating that
the prop is deliberately bypassed because path params use a per-field dispatch
via handlePathParamChange(row.uid, 'description', ...) so future readers won't
be confused. Keep the comment to one line and reference the use of
handlePathParamChange and row.uid for clarity.
- Around line 39-53: handlePathParamChange is never truly memoized because
pathParams is recreated each render; either move the find() lookup into the
reducer action (so handlePathParamChange sends the rowUid and key/value to
dispatch and let updatePathParam resolve the pathParam by uid), or stabilize
pathParams with useMemo before using it in the useCallback dependency array.
Specifically, either change handlePathParamChange to dispatch({ type:
updatePathParam, payload: { uid: rowUid, key, value }, itemUid: item.uid,
collectionUid: collection.uid }) and resolve the item in the reducer, or wrap
the pathParams filter in useMemo (e.g. const pathParams = useMemo(() =>
originalPathParams.filter(...), [originalPathParams]) ) and keep the existing
handlePathParamChange that references pathParams, dispatch, item.uid,
collection.uid.
In `@packages/bruno-app/src/components/RequestPane/Vars/VarsTable/index.js`:
- Around line 48-66: descriptionColumn is recreated on every render; wrap its
definition in useMemo and include the dependencies [storedTheme, onSave,
handleRun, collection, item] so the rendered MultiLineEditor props are stable,
and add useMemo to the import list; keep the same shape and return value but
memoize the const descriptionColumn = useMemo(() => ({ key: 'description', name:
'Description', placeholder: 'Description', width: '25%', render: ({ value,
onChange }) => (<MultiLineEditor ... />) }), [storedTheme, onSave, handleRun,
collection, item]).
In `@packages/bruno-lang/v2/src/collectionBruToJson.js`:
- Around line 82-98: Remove the local duplicated extractDescription function and
instead import and use the shared exported helper from the common semantic
utilities module; locate the local definition named extractDescription in this
file and delete it, add an import for the exported extractDescription from the
shared module (the same symbol used by common/semantic-utils), and ensure
references in this file (and the other duplicates in bruToJson.js/envToJson.js)
call that imported extractDescription so behavior remains identical.
In `@packages/bruno-lang/v2/src/common/semantic-utils.js`:
- Around line 133-147: In mapPairListToKeyValPairsMultipart, remove the
redundant unconditional call to extractDescription for each pair (currently
after mapping pairs from mapPairListToKeyValPairs) and instead only call
extractDescription when parseEnabled is false; reference
mapPairListToKeyValPairs (which already extracts descriptions when parseEnabled
=== true), the local parseEnabled flag, and the extractDescription function to
implement the conditional so descriptions are only stripped once.
In `@packages/bruno-lang/v2/src/jsonToCollectionBru.js`:
- Around line 8-15: getDescriptionSuffix is duplicated across
jsonToCollectionBru.js, jsonToBru.js, and jsonToEnv.js; extract this function
into a shared utility module (e.g., common/semantic-utils or utils) and replace
the copies with imports. Create a new exported function getDescriptionSuffix in
the shared module using the same implementation (preserve string escaping logic
and return values), update jsonToCollectionBru.js, jsonToBru.js, and
jsonToEnv.js to import getDescriptionSuffix and remove their local
implementations, and run tests/lint to ensure no references remain to the old
local functions.
In `@packages/bruno-lang/v2/src/jsonToEnv.js`:
- Around line 4-11: The getDescriptionSuffix function is duplicated in
jsonToEnv.js and jsonToBru.js; extract it into
packages/bruno-lang/v2/src/utils.js as an exported helper (preserve its current
logic and escaping behavior and keep the function name getDescriptionSuffix),
then remove the duplicate definitions from jsonToEnv.js and jsonToBru.js and
replace them with imports that import { getDescriptionSuffix } from './utils';
so both modules use the single shared implementation.
In `@packages/bruno-lang/v2/tests/envToJson.spec.js`:
- Around line 38-59: Update the test for parsing `@description` to include the two
missing edge cases: add a case where a disabled variable (e.g., "~url:
http://... `@description`('desc')") is parsed and assert that parser(...) returns
that variable with enabled: false and description preserved (check
output.variables[n].enabled and .description); also assert the secret field for
non-secret vars (e.g., expect(output.variables[n].secret).toBe(false)) instead
of relying solely on toMatchObject so the secret flag is explicitly verified.
Use the same parser invocation and output.variables indexing used in the spec to
locate where to add these assertions.
| &.has-description-column td { | ||
| &:nth-child(4) { | ||
| vertical-align: top; | ||
| white-space: pre-wrap; | ||
| word-wrap: break-word; | ||
| word-break: break-word; | ||
| } | ||
| &:nth-child(5) { | ||
| width: 80px; | ||
| } | ||
| &:nth-child(6) { | ||
| width: 60px; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the StyledWrapper.js file
fd -n "StyledWrapper.js" packages/bruno-app/src/components/EnvironmentVariablesTable/Repository: usebruno/bruno
Length of output: 288
🏁 Script executed:
# Read the StyledWrapper.js file to see the CSS structure
cat -n packages/bruno-app/src/components/EnvironmentVariablesTable/StyledWrapper.jsRepository: usebruno/bruno
Length of output: 5635
🏁 Script executed:
# Also check the index.js to see where the class is applied
rg -n "has-description-column" packages/bruno-app/src/components/EnvironmentVariablesTable/ -A 3 -B 3Repository: usebruno/bruno
Length of output: 1539
CSS selector table.has-description-column td won't match — styles won't apply.
The rule at line 45 is nested inside table { }, so styled-components compiles &.has-description-column to table.has-description-column td. However, the has-description-column class is applied to the wrapper <StyledWrapper> div (index.js line 436), not to the <table> element. The selector requires the class to be on the table itself, so these rules are dead CSS.
Move the block outside the table scope to target table as a descendant:
}
- &.has-description-column td {
- &:nth-child(4) {
- vertical-align: top;
- white-space: pre-wrap;
- word-wrap: break-word;
- word-break: break-word;
- }
- &:nth-child(5) {
- width: 80px;
- }
- &:nth-child(6) {
- width: 60px;
- }
- }
}
+
+ &.has-description-column table td {
+ &:nth-child(4) {
+ vertical-align: top;
+ white-space: pre-wrap;
+ word-wrap: break-word;
+ word-break: break-word;
+ }
+ &:nth-child(5) {
+ width: 80px;
+ }
+ &:nth-child(6) {
+ width: 60px;
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-app/src/components/EnvironmentVariablesTable/StyledWrapper.js`
around lines 45 - 58, The CSS rules for the description column are nested inside
the table block so the compiled selector becomes `table.has-description-column
td` which never matches because the `has-description-column` class is on the
StyledWrapper wrapper, not the table; move that whole block out of the `table {
... }` scope and change the selector to target the table as a descendant of the
wrapper (e.g. use `&.has-description-column table td { &:nth-child(4) { ... }
&:nth-child(5) { ... } &:nth-child(6) { ... } }`) so the nth-child(4/5/6) rules
apply when the wrapper has the `has-description-column` class.
| }, | ||
| ...(showDescriptionColumn | ||
| ? [ | ||
| { | ||
| key: 'description', | ||
| name: 'Description', | ||
| placeholder: 'Description', | ||
| width: '25%', | ||
| render: ({ value, onChange }) => ( | ||
| <MultiLineEditor | ||
| value={value || ''} | ||
| theme={storedTheme} | ||
| onSave={onSave} | ||
| onChange={onChange} | ||
| allowNewlines={true} | ||
| onRun={handleRun} | ||
| collection={collection} | ||
| item={item} | ||
| placeholder={!value ? 'Description' : ''} | ||
| /> | ||
| ) | ||
| } | ||
| ] | ||
| : []) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine EditableTable component
fd -e js -e jsx -e ts -e tsx -g '*EditableTable*' -oRepository: usebruno/bruno
Length of output: 174
🏁 Script executed:
# Read the actual file context around lines 193-216
cat -n packages/bruno-app/src/components/RequestPane/MultipartFormParams/index.js | sed -n '185,225p'Repository: usebruno/bruno
Length of output: 1352
🏁 Script executed:
# Check if description column is added elsewhere with width adjustments
rg -n "showDescriptionColumn" packages/bruno-app/src/components/RequestPane/ -A 5 -B 5Repository: usebruno/bruno
Length of output: 19151
🏁 Script executed:
# Get the full columns array definition for MultipartFormParams
cat -n packages/bruno-app/src/components/RequestPane/MultipartFormParams/index.js | sed -n '150,220p'Repository: usebruno/bruno
Length of output: 2580
🏁 Script executed:
# Check how other components define their description column
rg -A 15 "descriptionColumn.*=" packages/bruno-app/src/components/RequestPane/FormUrlEncodedParams/index.js | head -30Repository: usebruno/bruno
Length of output: 513
🏁 Script executed:
# Search for EditableTable component definition to understand width handling
fd -e js -e jsx -e ts -e tsx | xargs rg -l "EditableTable" | xargs rg -l "class EditableTable|const EditableTable|export.*EditableTable" | head -5Repository: usebruno/bruno
Length of output: 278
🏁 Script executed:
# Read the EditableTable component to understand width handling
wc -l packages/bruno-app/src/components/EditableTable/index.jsRepository: usebruno/bruno
Length of output: 118
🏁 Script executed:
# Get the relevant parts of EditableTable that handle column widths
cat -n packages/bruno-app/src/components/EditableTable/index.js | head -150Repository: usebruno/bruno
Length of output: 5797
🏁 Script executed:
# Check if there's a CSS file for EditableTable
find packages/bruno-app/src/components/EditableTable -type f \( -name "*.css" -o -name "*.scss" \)Repository: usebruno/bruno
Length of output: 40
🏁 Script executed:
# Check all the columns definitions in MultipartFormParams to see the full picture
cat -n packages/bruno-app/src/components/RequestPane/MultipartFormParams/index.js | sed -n '115,150p'Repository: usebruno/bruno
Length of output: 1441
🏁 Script executed:
# Find CSS files for EditableTable
find packages/bruno-app/src/components/EditableTable -name "*.css" -o -name "*.scss"Repository: usebruno/bruno
Length of output: 40
🏁 Script executed:
# Check StyledWrapper to see how widths are applied
cat packages/bruno-app/src/components/EditableTable/StyledWrapper.jsRepository: usebruno/bruno
Length of output: 4633
🏁 Script executed:
# Check where column widths are actually applied in the render
cat -n packages/bruno-app/src/components/EditableTable/index.js | sed -n '200,300p'Repository: usebruno/bruno
Length of output: 3903
🏁 Script executed:
# Check how EditableTable applies column widths in the rendering
cat -n packages/bruno-app/src/components/EditableTable/index.js | sed -n '300,350p'Repository: usebruno/bruno
Length of output: 2275
🏁 Script executed:
# Look for where column width is actually set on table cells
cat -n packages/bruno-app/src/components/EditableTable/index.js | sed -n '350,450p'Repository: usebruno/bruno
Length of output: 4431
🏁 Script executed:
# Verify that other similar table components use identical width patterns
for file in packages/bruno-app/src/components/RequestPane/*/index.js; do
echo "=== $(basename $(dirname $file)) ==="
rg -A 5 "width:.*%" "$file" | head -20
doneRepository: usebruno/bruno
Length of output: 2521
🏁 Script executed:
# Get complete column width definitions for all components with description columns
for file in FormUrlEncodedParams RequestHeaders QueryParams Vars MultipartFormParams; do
echo "=== $file ==="
rg -B 10 "width: '25%'" "packages/bruno-app/src/components/RequestPane/$file/index.js" | grep -E "(key:|width:)" | head -20
doneRepository: usebruno/bruno
Length of output: 396
🏁 Script executed:
# Check RequestHeaders full widths
rg -B 15 "descriptionColumn" packages/bruno-app/src/components/RequestPane/RequestHeaders/index.js | grep -E "(key:|width:)" | head -30Repository: usebruno/bruno
Length of output: 40
🏁 Script executed:
# Get full column definitions for RequestHeaders to see base columns + description
cat -n packages/bruno-app/src/components/RequestPane/RequestHeaders/index.js | sed -n '80,135p'Repository: usebruno/bruno
Length of output: 1733
🏁 Script executed:
# Get full column definitions for FormUrlEncodedParams
cat -n packages/bruno-app/src/components/RequestPane/FormUrlEncodedParams/index.js | sed -n '40,90p'Repository: usebruno/bruno
Length of output: 1561
Column widths create horizontal scrolling when Description column is shown.
MultipartFormParams sets explicit widths: 30% + 35% + 20% + 25% = 110%. With table-layout: fixed, this causes the table to overflow the container. The container's overflow: auto allows horizontal scrolling, but this may not be the intended UX.
Other similar components (RequestHeaders, FormUrlEncodedParams) avoid this by not setting explicit width on the Value column, allowing it to flex. Either reduce the widths of existing columns proportionally when showDescriptionColumn is true, or verify that horizontal scrolling is acceptable behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-app/src/components/RequestPane/MultipartFormParams/index.js`
around lines 193 - 216, MultipartFormParams is setting fixed column widths that
sum to 110% when showDescriptionColumn is true (resulting in horizontal
overflow); update the columns definition (the array that includes the column
with key 'description') to avoid hard-coding widths that force overflow—either
remove the explicit width on the 'value' column so it can flex (matching
RequestHeaders/FormUrlEncodedParams behavior) or reduce the widths of existing
columns proportionally when showDescriptionColumn is true; adjust the
render/column objects for keys like 'value' and 'description' in the
MultipartFormParams component so the table no longer exceeds 100% width.
| const getDescriptionSuffix = (item) => { | ||
| const desc = item && item.description && String(item.description).trim(); | ||
| if (!desc) return ''; | ||
| if (desc.includes('\'\'\'')) { | ||
| return ' @description("' + desc.replace(/\\/g, '\\\\').replace(/"/g, '\\"') + '")'; | ||
| } | ||
| return ' @description(\'\'\'' + desc.replace(/\\/g, '\\\\') + '\'\'\')'; | ||
| }; |
There was a problem hiding this comment.
Multiline descriptions will produce malformed BRU files.
When desc contains a literal newline (users can enter multiline text via the MultiLineEditor), getDescriptionSuffix returns a string with an embedded \n, e.g.:
Authorization: Bearer token `@description`('''Line 1
Line 2''')
The BRU grammar's singlelinevalue = valuechar* stops at the first newline (valuechar = ~(nl | tagend) any), so the parser captures only Bearer token @description('''Line 1 and then fails on Line 2'''). The file is silently corrupted or causes a hard parse error on next load.
A secondary issue: for variable items whose value is already a triple-quoted multiline block, getValueString returns '''\n content\n'''. Appending @description(...) after that produces:
varname: '''
content
''' `@description`("desc")
The pair grammar rule allows only st* after value, so @description(...) in this position is not recognized — the description is silently dropped.
Fix: Escape newlines in descriptions before embedding in the BRU line, and unescape them on parse:
🐛 Proposed fix for getDescriptionSuffix
const getDescriptionSuffix = (item) => {
const desc = item && item.description && String(item.description).trim();
if (!desc) return '';
+ // Escape newlines so the annotation stays on one line
+ const safeDesc = desc.replace(/\\/g, '\\\\').replace(/\n/g, '\\n').replace(/\r/g, '');
- if (desc.includes('\'\'\'')) {
- return ' `@description`("' + desc.replace(/\\/g, '\\\\').replace(/"/g, '\\"') + '")';
+ if (safeDesc.includes('\'\'\'')) {
+ return ' `@description`("' + safeDesc.replace(/"/g, '\\"') + '")';
}
- return ' `@description`(\'\'\'' + desc.replace(/\\/g, '\\\\') + '\'\'\')';
+ return ' `@description`(\'\'\'' + safeDesc + '\'\'\')';
};extractDescription must correspondingly unescape \\n → \n and \\\\ → \\ after capturing the group (also fixes the existing round-trip escaping mismatch).
The description-on-multiline-value case requires a grammar change (adding an optional descriptionannotation? to the pair rule, similarly to how contenttypeannotation was added to multilinetextblock).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bruno-lang/v2/src/jsonToCollectionBru.js` around lines 8 - 15,
getDescriptionSuffix currently embeds raw newlines and mismatched escapes into
BRU output causing parser breakage; update getDescriptionSuffix to escape
newlines (replace \n with \\n) and preserve backslashes (\\\\) when building the
`@description`(...) token, and update the parser by implementing an
extractDescription function that unescapes \\n → newline and \\\\ → \\ when
reading descriptions; additionally, adjust the grammar by adding an optional
descriptionannotation? to the pair rule (mirroring the contenttypeannotation
change on multilinetextblock) so description annotations are allowed after
multiline values, and ensure getValueString and the parser consistently
round-trip these escaped sequences.
1 participant
Description
Contribution Checklist:
Summary by CodeRabbit
New Features
Improvements