fix: correct URL formatting in login --no-wait output#169
fix: correct URL formatting in login --no-wait output#169JackZhao10086 merged 4 commits intomainfrom
Conversation
… --no-wait" command contains \u0026
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughReplaced direct json.Marshal/json.MarshalIndent usage with streaming json.NewEncoder output (SetEscapeHTML(false)). Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR fixes HTML-escaping of special characters (e.g. Key changes:
Outstanding gap: The Confidence Score: 5/5Safe to merge; the core fix is correct and the only remaining issue is a P2 consistency gap in two secondary JSON emit paths. All P1+ concerns from the previous review thread have been addressed (error handling added for new encoder calls, root.go newline behavior preserved). The single remaining finding is P2: two JSON emit paths in the same function were not updated, which is a minor inconsistency rather than a blocking defect. cmd/auth/login.go — lines 269–274 and 322–328 still use the old json.Marshal pattern without SetEscapeHTML(false). Important Files Changed
Reviews (4): Last reviewed commit: "docs(cmd/auth): add comment for authLogi..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
cmd/auth/login.go (1)
264-268: Consider aligning JSON output style for consistency.This
authorization_failedevent (andauthorization_completeat lines 317-323) still usesjson.Marshal, while the other paths now usejson.NewEncoderwithSetEscapeHTML(false). While these payloads don't contain URLs, aligning the approach improves maintainability and ensures consistent output formatting (e.g., trailing newline behavior).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/auth/login.go` around lines 264 - 268, Replace the json.Marshal + fmt.Fprintln pattern used for the "authorization_failed" payload (and the similar "authorization_complete" payload at lines ~317-323) with a json.Encoder that sets SetEscapeHTML(false) and calls Encode(...) to write the object directly to f.IOStreams.Out; specifically, locate the map[string]interface{} creation for the "event" and "error" fields and swap the Marshal+Fprintln sequence for: enc := json.NewEncoder(f.IOStreams.Out); enc.SetEscapeHTML(false); enc.Encode(<the map>), doing the same replacement for the authorization_complete block to keep JSON output style and newline/escaping behavior consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/auth/login.go`:
- Around line 249-251: The call to encoder.Encode(data) is currently ignoring
its returned error; update the code in the login flow to capture and handle that
error from json.NewEncoder(...).Encode(data) (the encoder variable and Encode
method) — e.g., check the error, return it or log/propagate an appropriate
wrapped error that includes context (writing JSON to f.IOStreams.Out failed) so
I/O failures are surfaced consistently with other handlers.
- Around line 234-236: The call to encoder.Encode(data) in cmd/auth/login.go
ignores its returned error; update the code to capture and handle that error
(e.g., err := encoder.Encode(data)); if non-nil, log or write a clear message to
the command's error stream (f.IOStreams.Err or process logger) and propagate the
error from the command (return fmt.Errorf(...) or appropriate error return) so
I/O failures like broken pipes are not silently ignored; locate the encoder
usage and the surrounding command handler (the login command Run/RunE function)
and add the check and return/log accordingly.
---
Nitpick comments:
In `@cmd/auth/login.go`:
- Around line 264-268: Replace the json.Marshal + fmt.Fprintln pattern used for
the "authorization_failed" payload (and the similar "authorization_complete"
payload at lines ~317-323) with a json.Encoder that sets SetEscapeHTML(false)
and calls Encode(...) to write the object directly to f.IOStreams.Out;
specifically, locate the map[string]interface{} creation for the "event" and
"error" fields and swap the Marshal+Fprintln sequence for: enc :=
json.NewEncoder(f.IOStreams.Out); enc.SetEscapeHTML(false); enc.Encode(<the
map>), doing the same replacement for the authorization_complete block to keep
JSON output style and newline/escaping behavior consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8ef5d904-673d-41b3-bfdd-23e3e45dd46a
📒 Files selected for processing (2)
cmd/auth/login.gocmd/root.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@0ddf1a52dcc7cd91958ccaf934aec4c83ac455cf🧩 Skill updatenpx skills add larksuite/cli#fix/login_no_wait -y -g |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cmd/auth/login.go (2)
320-327: Inconsistent:authorization_completeJSON also usesjson.Marshal.Same issue as above—this should use
json.NewEncoderwithSetEscapeHTML(false)for consistency.Proposed fix
if opts.JSON { - b, _ := json.Marshal(map[string]interface{}{ + encoder := json.NewEncoder(f.IOStreams.Out) + encoder.SetEscapeHTML(false) + data := map[string]interface{}{ "event": "authorization_complete", "user_open_id": openId, "user_name": userName, "scope": result.Token.Scope, - }) - fmt.Fprintln(f.IOStreams.Out, string(b)) + } + if err := encoder.Encode(data); err != nil { + fmt.Fprintf(f.IOStreams.ErrOut, "error: failed to write JSON output: %v\n", err) + } } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/auth/login.go` around lines 320 - 327, The JSON branch in the authorization completion path uses json.Marshal and fmt.Fprintln (checking opts.JSON and building the map with keys "event","user_open_id","user_name","scope"), which is inconsistent with other places; replace the json.Marshal + fmt.Fprintln usage with a json.NewEncoder targeted at f.IOStreams.Out, call SetEscapeHTML(false) on the encoder and then Encode the same map (handle or return any encode error), removing the fmt.Fprintln call so output is written via the encoder.
267-272: Inconsistent: still usingjson.Marshalwith default HTML escaping.This code path and the one at lines 320-327 still use
json.Marshal, which HTML-escapes special characters. For consistency with the PR objective ("disable HTML escaping in JSON outputs for authentication commands"), consider using the samejson.NewEncoderpattern here.Proposed fix for consistency
if !result.OK { if opts.JSON { - b, _ := json.Marshal(map[string]interface{}{ + encoder := json.NewEncoder(f.IOStreams.Out) + encoder.SetEscapeHTML(false) + data := map[string]interface{}{ "event": "authorization_failed", "error": result.Message, - }) - fmt.Fprintln(f.IOStreams.Out, string(b)) + } + if err := encoder.Encode(data); err != nil { + fmt.Fprintf(f.IOStreams.ErrOut, "error: failed to write JSON output: %v\n", err) + } return output.ErrBare(output.ExitAuth) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/auth/login.go` around lines 267 - 272, Replace the json.Marshal usage in the JSON error output branch (the block that checks opts.JSON and writes the authorization_failed payload) with a json.NewEncoder writing to f.IOStreams.Out and call SetEscapeHTML(false) on the encoder to disable HTML escaping; build the same map or struct used now (event, error/result.Message) and encode it via encoder.Encode(...) instead of fmt.Fprintln(...), keeping the same output stream and handling any encode error via the existing logging/error flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/auth/login.go`:
- Around line 320-327: The JSON branch in the authorization completion path uses
json.Marshal and fmt.Fprintln (checking opts.JSON and building the map with keys
"event","user_open_id","user_name","scope"), which is inconsistent with other
places; replace the json.Marshal + fmt.Fprintln usage with a json.NewEncoder
targeted at f.IOStreams.Out, call SetEscapeHTML(false) on the encoder and then
Encode the same map (handle or return any encode error), removing the
fmt.Fprintln call so output is written via the encoder.
- Around line 267-272: Replace the json.Marshal usage in the JSON error output
branch (the block that checks opts.JSON and writes the authorization_failed
payload) with a json.NewEncoder writing to f.IOStreams.Out and call
SetEscapeHTML(false) on the encoder to disable HTML escaping; build the same map
or struct used now (event, error/result.Message) and encode it via
encoder.Encode(...) instead of fmt.Fprintln(...), keeping the same output stream
and handling any encode error via the existing logging/error flow.
* fix: Fix the issue where the URL returned by the "lark-cli auth login --no-wait" command contains \u0026 * style: fix indentation and whitespace in error handling code * fix(auth): handle JSON encoding errors in login output * docs(cmd/auth): add comment for authLoginRun function
3 participants
Summary
Fixed an issue where special characters in URLs were incorrectly HTML-escaped in JSON outputs. This ensures that the generated URLs from the
login --no-waitcommand and other JSON responses are correctly formatted and directly usable.Changes
Test Plan
lark-cli auth login --no-waitreturns the correct unescaped URLlark-cli auth login --jsonreturns the correct unescaped URLRelated Issues
Summary by CodeRabbit