github-starred/yaak
2
0
Fork 0
mirror of https://github.com/mountain-loop/yaak.git synced 2026-06-09 12:45:58 -05:00
Code Issues 119 Packages Projects Releases 104 Wiki Activity

[PR #465] fix: Resolve : ambiguity in URL path placeholders #2635

New Issue
Open
opened 2026-05-29 06:44:01 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-05-29 06:44:01 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/mountain-loop/yaak/pull/465
Author: @simon446
Created: 5/21/2026
Status: 🔄 Open

Base: main ← Head: fix-openapi-colon-paths

📝 Commits (2)

  • 7b7f1be Resolve : ambiguity in URL path placeholders
  • 52ee7b6 Render path-only URLs with leading : placeholders as chips

📊 Changes

11 files changed (+133 additions, -24 deletions)

View changed files

📝 apps/yaak-client/components/HttpRequestPane.tsx (+2 -3)
📝 apps/yaak-client/components/WebsocketRequestPane.tsx (+2 -3)
📝 apps/yaak-client/components/core/Editor/twig/pathParameters.ts (+7 -2)
📝 apps/yaak-client/components/core/Editor/url/highlight.ts (+4 -1)
📝 apps/yaak-client/components/core/Editor/url/url.grammar (+10 -3)
📝 apps/yaak-client/components/core/Editor/url/url.terms.ts (+6 -6)
➕ apps/yaak-client/components/core/Editor/url/url.test.ts (+39 -0)
📝 apps/yaak-client/components/core/Editor/url/url.ts (+5 -5)
➕ apps/yaak-client/lib/pathPlaceholders.test.ts (+28 -0)
➕ apps/yaak-client/lib/pathPlaceholders.ts (+14 -0)
📝 crates/yaak-http/src/path_placeholders.rs (+16 -1)

📄 Description

Summary

:name path placeholders were ambiguous when followed by a literal : in the same path segment. The URL grammar matched the placeholder name greedily until /?#, and the Rust resolver's anchor required /?#$ after the name — so /users/:id:literal parsed as one big placeholder and never substituted. This PR tightens the grammar so the name excludes :, extends the resolver to accept : as a trailing boundary, and adjusts the editor (chip widget filter, highlight, param-list extraction) so the new boundaries are reflected visually.

Before / after

URL field rendering:

Before After
Before — :cancel rendered as a spurious chip with underline After — :cancel correctly rendered as literal text

Submission

  • This PR is a bug fix or small-scope improvement.
  • I have read and followed CONTRIBUTING.md.
  • I tested this change locally.
  • I added or updated tests when reasonable.

Approved feedback item (required if not a bug fix or small-scope improvement):

Related

:-suffixed path segments show up in the wild for AIP-136 custom methods (e.g. /tasks/{id}:cancel, /users/{id}:archive) and any other path style that uses :verb after a resource id. Before this PR, those URLs rendered with one greedy chip and didn't substitute when sent.

Details

Grammar (url.grammar): Placeholder { ":" ![/?#:]+ } excludes : from the name, and a /-segment now allows Placeholder PathSegment? so /:abc:def parses as Placeholder(:abc) + PathSegment(:def).

Resolver (crates/yaak-http/src/path_placeholders.rs): added : to the trailing-boundary regex set so :id in :id:literal substitutes correctly. One new unit test.

Editor:

  • pathParameters.ts: chip widgets only render for Placeholder nodes whose preceding document character is / — filters out the lexer's mid-segment over-emission and the long-standing env-var-followed-by-:abc quirk (${[var]}:abc no longer renders :abc as a spurious chip).
  • highlight.ts: dropped the Placeholder: t.emphasis style — chip widgets fully replace the placeholder text, so the underline was only ever visible on the spurious nodes we now skip.
  • pathPlaceholders.ts (new): shared extractPathPlaceholders helper with unit tests, used by both HttpRequestPane and WebsocketRequestPane to keep the URL → params sync consistent.
  • url.test.ts (new): grammar-level test documenting the lexer over-emission the chip filter relies on.

Test plan

  • cargo test -p yaak-http path_placeholder — 9/9 pass, includes new placeholder_followed_by_literal_colon
  • vp test --run against apps/yaak-client/lib/pathPlaceholders.test.ts and apps/yaak-client/components/core/Editor/url/url.test.ts — 8/8 pass
  • tsc --noEmit in apps/yaak-client — clean
  • npm test — 148 passing across JS workspaces, no failures
  • Manual smoke: built target/release/yaak-app-client and verified the editor visually for /:abc:def, ${[env]}:abc, and OpenAPI-imported requests (see Before / after above)

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/mountain-loop/yaak/pull/465 **Author:** [@simon446](https://github.com/simon446) **Created:** 5/21/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix-openapi-colon-paths` --- ### 📝 Commits (2) - [`7b7f1be`](https://github.com/mountain-loop/yaak/commit/7b7f1bea82a737527a07b9d26e08d58ae9274699) Resolve `:` ambiguity in URL path placeholders - [`52ee7b6`](https://github.com/mountain-loop/yaak/commit/52ee7b6fd44f4069fbac2d1912397a2f8a1ba967) Render path-only URLs with leading `:` placeholders as chips ### 📊 Changes **11 files changed** (+133 additions, -24 deletions) <details> <summary>View changed files</summary> 📝 `apps/yaak-client/components/HttpRequestPane.tsx` (+2 -3) 📝 `apps/yaak-client/components/WebsocketRequestPane.tsx` (+2 -3) 📝 `apps/yaak-client/components/core/Editor/twig/pathParameters.ts` (+7 -2) 📝 `apps/yaak-client/components/core/Editor/url/highlight.ts` (+4 -1) 📝 `apps/yaak-client/components/core/Editor/url/url.grammar` (+10 -3) 📝 `apps/yaak-client/components/core/Editor/url/url.terms.ts` (+6 -6) ➕ `apps/yaak-client/components/core/Editor/url/url.test.ts` (+39 -0) 📝 `apps/yaak-client/components/core/Editor/url/url.ts` (+5 -5) ➕ `apps/yaak-client/lib/pathPlaceholders.test.ts` (+28 -0) ➕ `apps/yaak-client/lib/pathPlaceholders.ts` (+14 -0) 📝 `crates/yaak-http/src/path_placeholders.rs` (+16 -1) </details> ### 📄 Description ## Summary `:name` path placeholders were ambiguous when followed by a literal `:` in the same path segment. The URL grammar matched the placeholder name greedily until `/?#`, and the Rust resolver's anchor required `/?#$` after the name — so `/users/:id:literal` parsed as one big placeholder and never substituted. This PR tightens the grammar so the name excludes `:`, extends the resolver to accept `:` as a trailing boundary, and adjusts the editor (chip widget filter, highlight, param-list extraction) so the new boundaries are reflected visually. ## Before / after URL field rendering: | Before | After | | --- | --- | | ![Before — `:cancel` rendered as a spurious chip with underline](https://github.com/user-attachments/assets/4e01957c-7797-491f-bc19-b8a805ce766a) | ![After — `:cancel` correctly rendered as literal text](https://github.com/user-attachments/assets/06f0d154-3586-480c-99ba-095aa3023d4f) | ## Submission - [x] This PR is a bug fix or small-scope improvement. - [x] I have read and followed [`CONTRIBUTING.md`](CONTRIBUTING.md). - [x] I tested this change locally. - [x] I added or updated tests when reasonable. Approved feedback item (required if not a bug fix or small-scope improvement): <!-- N/A — bug fix scope. --> ## Related `:`-suffixed path segments show up in the wild for [AIP-136](https://google.aip.dev/136) custom methods (e.g. `/tasks/{id}:cancel`, `/users/{id}:archive`) and any other path style that uses `:verb` after a resource id. Before this PR, those URLs rendered with one greedy chip and didn't substitute when sent. ## Details **Grammar** (`url.grammar`): `Placeholder { ":" ![/?#:]+ }` excludes `:` from the name, and a `/`-segment now allows `Placeholder PathSegment?` so `/:abc:def` parses as Placeholder(`:abc`) + PathSegment(`:def`). **Resolver** (`crates/yaak-http/src/path_placeholders.rs`): added `:` to the trailing-boundary regex set so `:id` in `:id:literal` substitutes correctly. One new unit test. **Editor**: - `pathParameters.ts`: chip widgets only render for `Placeholder` nodes whose preceding document character is `/` — filters out the lexer's mid-segment over-emission and the long-standing env-var-followed-by-`:abc` quirk (`${[var]}:abc` no longer renders `:abc` as a spurious chip). - `highlight.ts`: dropped the `Placeholder: t.emphasis` style — chip widgets fully replace the placeholder text, so the underline was only ever visible on the spurious nodes we now skip. - `pathPlaceholders.ts` (new): shared `extractPathPlaceholders` helper with unit tests, used by both `HttpRequestPane` and `WebsocketRequestPane` to keep the URL → params sync consistent. - `url.test.ts` (new): grammar-level test documenting the lexer over-emission the chip filter relies on. ## Test plan - [x] `cargo test -p yaak-http path_placeholder` — 9/9 pass, includes new `placeholder_followed_by_literal_colon` - [x] `vp test --run` against `apps/yaak-client/lib/pathPlaceholders.test.ts` and `apps/yaak-client/components/core/Editor/url/url.test.ts` — 8/8 pass - [x] `tsc --noEmit` in `apps/yaak-client` — clean - [x] `npm test` — 148 passing across JS workspaces, no failures - [x] Manual smoke: built `target/release/yaak-app-client` and verified the editor visually for `/:abc:def`, `${[env]}:abc`, and OpenAPI-imported requests (see *Before / after* above) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-05-29 06:44:01 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
pull-request
Mirrored from GitHub Pull Request
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/yaak#2635
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?