Skip to content

Commit e760466

Browse files
tharindulakclaude
andcommitted
Address CodeRabbit and Copilot review feedback on the Hurl Client PR
- Do not discard report entries whose line cannot be resolved once every cell boundary is claimed; attach them to the last boundary so an executed request never disappears from the notebook, and cover it with a test. - Distinguish a comments-only cell from one holding content that parsed to no request. A malformed request previously ended its cell successfully, masking the error; it now fails with a "NOT PARSED" output. - Drop the leftover deprecated onCommand activation event. Contributed commands are auto-activated from VS Code 1.74 and this extension targets ^1.100.0, so neither importHurlString entry is needed. - Document that hurl.vars is resolved from hurl-client.fileRoot (defaulting to the notebook folder), and tag the variables-file example as ini. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 419d689 commit e760466

5 files changed

Lines changed: 73 additions & 17 deletions

File tree

workspaces/api-tryit/hurl-runner/src/report-parser.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -750,14 +750,20 @@ export function mapFileResultToCellOutcomes(fileResult: HurlFileResult, boundari
750750
}
751751

752752
let cursor = 0;
753-
for (const entry of unplaced) {
753+
for (let i = 0; i < unplaced.length; i++) {
754754
while (cursor < outcomes.length && outcomes[cursor].entries.length > 0) {
755755
cursor++;
756756
}
757757
if (cursor >= outcomes.length) {
758+
// Every boundary is already claimed. Rather than discard the
759+
// remainder - which would hide requests that really executed -
760+
// attach them to the last boundary so they still surface.
761+
if (outcomes.length > 0) {
762+
outcomes[outcomes.length - 1].entries.push(...unplaced.slice(i));
763+
}
758764
break;
759765
}
760-
outcomes[cursor].entries.push(entry);
766+
outcomes[cursor].entries.push(unplaced[i]);
761767
cursor++;
762768
}
763769

workspaces/api-tryit/hurl-runner/tests/report-parser.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,24 @@ describe('mapFileResultToCellOutcomes', () => {
587587
expect(outcomes[1].entries.map(e => e.name)).toEqual(['Lineless entry']);
588588
});
589589

590+
it('never discards a lineless entry when every boundary is already claimed', () => {
591+
// Both boundaries are taken by line-matched entries, so there is no
592+
// free slot left - the extra entry must still surface somewhere
593+
// rather than vanishing from the notebook entirely.
594+
const fileResult = makeFileResult([1, 5]);
595+
fileResult.entries.push({ name: 'Lineless entry', status: 'passed' });
596+
const boundaries = [
597+
{ startLine: 1, endLine: 3 },
598+
{ startLine: 5, endLine: 7 }
599+
];
600+
601+
const outcomes = mapFileResultToCellOutcomes(fileResult, boundaries);
602+
603+
const rendered = outcomes.flatMap(o => o.entries.map(e => e.name));
604+
expect(rendered).toContain('Lineless entry');
605+
expect(rendered).toHaveLength(3);
606+
});
607+
590608
it('does not let a lineless entry displace a boundary a line-matched entry already claimed', () => {
591609
const fileResult = makeFileResult([5]); // matches the second boundary
592610
fileResult.entries.push({ name: 'Lineless entry', status: 'passed' });

workspaces/hurl-client/hurl-client-extension/README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,14 @@ Select both cells above and run them together — `{{token}}` only resolves beca
5858

5959
## Variables
6060

61-
To set your own variables (like a base URL or API key), create a file named `hurl.vars` in the same folder as your `.hurl` files:
61+
To set your own variables (like a base URL or API key), create a file named `hurl.vars`:
6262

63-
```
63+
```ini
6464
base_url=https://api.example.com
6565
api_key=your-api-key
6666
```
6767

68-
Hurl Client picks this up automatically — no extra setup needed. Reference the values as `{{base_url}}`, `{{api_key}}`, etc. in your requests.
68+
Hurl Client looks for it in the folder set by `hurl-client.fileRoot`, which defaults to the notebook's own folder — so if you haven't changed that setting, put `hurl.vars` next to your `.hurl` files and it is picked up automatically. Reference the values as `{{base_url}}`, `{{api_key}}`, etc. in your requests.
6969

7070
Need a different value for just one file? Add `<filename>.hurl.vars` (e.g. `requests.hurl.vars` for `requests.hurl`) — it overrides the shared file for the values it defines.
7171

workspaces/hurl-client/hurl-client-extension/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@
2121
"Notebooks"
2222
],
2323
"activationEvents": [
24-
"onNotebook:HurlClient",
25-
"onCommand:HTTPClient.importHurlString"
24+
"onNotebook:HurlClient"
2625
],
2726
"icon": "images/icon.png",
2827
"main": "./dist/extension.js",

workspaces/hurl-client/hurl-client-extension/src/notebook/HurlNotebookController.ts

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -180,12 +180,13 @@ export class HurlNotebookController {
180180
const entries = outcomes[index].entries;
181181

182182
if (entries.length === 0) {
183-
const hasRequest = cellHasRequest(content);
183+
const kind = classifyCellWithoutResult(content);
184184
const laterEntryExists = outcomes.slice(index + 1).some(o => o.entries.length > 0);
185-
await execution.appendOutput([this.buildSkippedOutput(fileResult, hasRequest, laterEntryExists)]);
186-
// Nothing ran for this cell, so report no elapsed time
187-
// rather than the whole batch's.
188-
execution.end(!hasRequest, startedAt);
185+
await execution.appendOutput([this.buildSkippedOutput(fileResult, kind, laterEntryExists)]);
186+
// Only a comments-only cell is a success - a cell that
187+
// holds a request, or content hurl could not parse, failed.
188+
// Report no elapsed time either way, since nothing ran.
189+
execution.end(kind === 'comments', startedAt);
189190
endedCount = index + 1;
190191
continue;
191192
}
@@ -340,14 +341,27 @@ export class HurlNotebookController {
340341
return isLastEntry ? extractResponseBody(fileResult.stdout) : undefined;
341342
}
342343

343-
private buildSkippedOutput(fileResult: HurlFileResult, hasRequest: boolean, laterEntryExists: boolean): vscode.NotebookCellOutput {
344-
if (!hasRequest) {
344+
private buildSkippedOutput(
345+
fileResult: HurlFileResult,
346+
kind: CellWithoutResultKind,
347+
laterEntryExists: boolean
348+
): vscode.NotebookCellOutput {
349+
if (kind === 'comments') {
345350
const md = '##### ℹ️ NO REQUEST\n\nThis cell has no request to run.';
346351
return new vscode.NotebookCellOutput([
347352
vscode.NotebookCellOutputItem.text(md, 'text/markdown')
348353
]);
349354
}
350355

356+
if (kind === 'unparsed') {
357+
const detail = fileResult.errorMessage || fileResult.stderr;
358+
const detailBlock = detail ? `\n\n\`\`\`\n${detail}\n\`\`\`` : '';
359+
const md = `##### ❌ NOT PARSED\n\nNo runnable request was found in this cell. If it is meant to be a request, check its syntax - a malformed request also stops the rest of the run.${detailBlock}`;
360+
return new vscode.NotebookCellOutput([
361+
vscode.NotebookCellOutputItem.text(md, 'text/markdown')
362+
]);
363+
}
364+
351365
if (laterEntryExists) {
352366
// A later cell in the same run did produce an entry, so the run
353367
// didn't stop - hurl reached this request and skipped or failed
@@ -501,9 +515,28 @@ async function pathExists(filePath: string): Promise<boolean> {
501515
}
502516
}
503517

504-
/** Does this cell's own text contain an actual hurl request, or is it just comments/notes? */
505-
function cellHasRequest(content: string): boolean {
506-
return parseHurlDocument(content).blocks.length > 0;
518+
/**
519+
* Why a cell produced no result:
520+
* - `request` it does contain a request, so hurl either never reached it
521+
* or refused to run it
522+
* - `comments` nothing but comments/blank lines - genuinely nothing to run
523+
* - `unparsed` it has real content that the parser found no request in,
524+
* i.e. it is most likely malformed
525+
*
526+
* `comments` and `unparsed` must stay distinct: reporting a malformed request
527+
* as "nothing to run" would end the cell successfully and hide the error.
528+
*/
529+
type CellWithoutResultKind = 'request' | 'comments' | 'unparsed';
530+
531+
function classifyCellWithoutResult(content: string): CellWithoutResultKind {
532+
if (parseHurlDocument(content).blocks.length > 0) {
533+
return 'request';
534+
}
535+
const meaningfulLines = content
536+
.split('\n')
537+
.map(line => line.trim())
538+
.filter(line => line.length > 0 && !line.startsWith('#'));
539+
return meaningfulLines.length === 0 ? 'comments' : 'unparsed';
507540
}
508541

509542
/**

0 commit comments

Comments
 (0)