Skip to content

chore(docs): fix example path parameter #625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+10 −9
Open

chore(docs): fix example path parameter #625

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7ed80ce
chore(docs): fix example path parameter
Liumingxun May 26, 2025
53cca69
Update server-requests-and-responses.topic
Liumingxun May 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions topics/server-requests-and-responses.topic
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,9 +371,10 @@
</list>
<p>
The route you would add is
<code>/tasks/byPriority/{priority}</code>
where <code>{priority}</code> represents a query parameter that you will need to extract at runtime. The
query parameter can have any name you like, but <code>priority</code> seems the obvious choice.
<code>/tasks/byPriority/{priority?}</code>
where <code>{priority?}</code> represents a query parameter that you will need to extract at runtime,
and the question mark is used to indicate that a parameter is optional. The query parameter can have
any name you like, but <code>priority</code> seems the obvious choice.
Comment on lines +374 to +377
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Fix terminology and resolve implementation inconsistency.

There are several issues with this documentation update:

  1. Incorrect terminology: The text refers to {priority?} as a "query parameter", but this is actually a path parameter. Query parameters are key-value pairs that appear after ? in URLs (e.g., ?key=value).

  2. Implementation mismatch: The documentation shows the route as /tasks/byPriority/{priority?} (optional), but the actual implementation on line 404 defines it as /tasks/byPriority/{priority} (required).

  3. Logical inconsistency: If the parameter is truly optional, the implementation should handle missing parameters differently. Currently, the code returns 400 Bad Request when the parameter is null (lines 406-409), which suggests it's required.

Apply this diff to fix the terminology:

-where <code>{priority?}</code> represents a query parameter that you will need to extract at runtime,
-and the question mark is used to indicate that a parameter is optional. The query parameter can have
+where <code>{priority?}</code> represents a path parameter that you will need to extract at runtime,
+and the question mark is used to indicate that a parameter is optional. The path parameter can have

Additional action needed: Either update the route implementation to match the optional syntax, or revert the documentation to show the required parameter syntax {priority} to match the current implementation.

Let me verify if there are other route definitions that need to be updated consistently:

🏁 Script executed:

#!/bin/bash
# Description: Check for route definitions and parameter handling patterns in the documentation

# Search for route patterns with optional parameters
rg -A 3 -B 3 '\{[^}]*\?\}' 

# Search for route definitions in code blocks
rg -A 5 -B 5 'get\("/[^"]*\{[^}]*\}[^"]*"\)'

Length of output: 39761

Fix terminology and align documentation with implementation

The update introduces two issues that must be addressed:

• Incorrect terminology:
The text calls {priority?} a “query parameter” when it’s actually a path parameter.
• Docs/implementation mismatch:
Documentation shows /tasks/byPriority/{priority?} (optional) but the sample code still defines and handles it as a required parameter (get("/tasks/byPriority/{priority}")), returning 400 on null.

Please update the docs or code so they match:

  1. In topics/server-requests-and-responses.topic (lines 374–377), change “query parameter” → “path parameter” and decide on optional vs. required syntax. For example, if you keep it required, revert to {priority}:
- <code>/tasks/byPriority/{priority?}</code>
+ <code>/tasks/byPriority/{priority}</code>

- where <code>{priority?}</code> represents a query parameter …
- and the question mark is used to indicate that a parameter is optional.
+ where <code>{priority}</code> represents a path parameter that you will extract at runtime.
  1. If you truly want an optional parameter ({priority?}), update the Kotlin routing code (around line 404) to use get("/tasks/byPriority/{priority?}") and handle the missing case (e.g., treat null as “all priorities” instead of BadRequest).
📝 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.

Suggested change
<code>/tasks/byPriority/{priority?}</code>
where <code>{priority?}</code> represents a query parameter that you will need to extract at runtime,
and the question mark is used to indicate that a parameter is optional. The query parameter can have
any name you like, but <code>priority</code> seems the obvious choice.
<code>/tasks/byPriority/{priority?}</code>
- where <code>{priority?}</code> represents a query parameter that you will need to extract at runtime,
- and the question mark is used to indicate that a parameter is optional. The query parameter can have
+ where <code>{priority?}</code> represents a path parameter that you will need to extract at runtime,
+ and the question mark is used to indicate that a parameter is optional. The path parameter can have
any name you like, but <code>priority</code> seems the obvious choice.
🤖 Prompt for AI Agents 
In topics/server-requests-and-responses.topic around lines 374 to 377, the
documentation incorrectly calls `{priority?}` a query parameter when it is a
path parameter, and it shows the parameter as optional while the implementation
treats it as required. Fix this by changing "query parameter" to "path
parameter" and either revert the route in the docs to
`/tasks/byPriority/{priority}` to match the required parameter in the code or
update the Kotlin routing code near line 404 to define the route as
`/tasks/byPriority/{priority?}` and handle the null case gracefully instead of
returning 400.

</p>
<p>
The process to handle the request can be summarized as follows:
Expand All @@ -400,7 +401,7 @@
get("/tasks") { ... }

//add the following route
get("/tasks/byPriority/{priority}") {
get("/tasks/byPriority/{priority?}") {
val priorityAsText = call.parameters["priority"]
if (priorityAsText == null) {
call.respond(HttpStatusCode.BadRequest)
Expand Down Expand Up @@ -428,7 +429,7 @@
</code-block>
<p>
As summarized above, you have written a handler for the URL
<code>/tasks/byPriority/{priority}</code>
<code>/tasks/byPriority/{priority?}</code>
. The symbol
<code>priority</code>
represents the query parameter that the user has added. Unfortunately, on the server there's
Expand Down Expand Up @@ -709,7 +710,7 @@

get("/tasks") { ... }

get("/tasks/byPriority/{priority}") { … }
get("/tasks/byPriority/{priority?}") { … }
}
}
</code-block>
Expand Down Expand Up @@ -897,7 +898,7 @@
//Code remains the same
}

get("/byPriority/{priority}") {
get("/byPriority/{priority?}") {
//Code remains the same
}

Expand Down Expand Up @@ -943,7 +944,7 @@
)
}

get("/byPriority/{priority}") {
get("/byPriority/{priority?}") {
//Code remains the same
}

Expand All @@ -965,4 +966,4 @@
RESTful API for your task manager that generates JSON files.
</p>
</chapter>
</topic>
</topic>