Skip to content

fix(docs): clarify replace_all selection errors#954

Merged
fangshuyu-768 merged 1 commit into
larksuite:mainfrom
afengzi:codex/docs-replace-all-selection-hint
May 19, 2026
Merged

fix(docs): clarify replace_all selection errors#954
fangshuyu-768 merged 1 commit into
larksuite:mainfrom
afengzi:codex/docs-replace-all-selection-hint

Conversation

@afengzi
Copy link
Copy Markdown
Contributor

@afengzi afengzi commented May 18, 2026

Summary

  • keep replace_all selection validation, but add a targeted hint for full-document replacement
  • suggest --mode overwrite when users run --mode replace_all without a required selection
  • add tests so the overwrite hint only appears for replace_all

Validation

  • go test -count=1 ./shortcuts/doc
  • go test -race -gcflags="all=-N -l" -count=1 ./shortcuts/doc
  • go vet ./shortcuts/doc
  • make build

Refs #809

Summary by CodeRabbit

  • Bug Fixes

    • Improved error messages during update validation to provide mode-specific guidance. When using replace_all mode without required selection flags, users now receive suggestions to consider using overwrite mode as an alternative option.
  • Tests

    • Added test cases to validate mode-specific error message generation and ensure appropriate suggestions appear based on the selected mode.

Review Change Stack

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 18, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 13003e90-8f03-428f-8e0c-73a05000a3ca

📥 Commits

Reviewing files that changed from the base of the PR and between 315e0ab and bc75e89.

📒 Files selected for processing (2)
  • shortcuts/doc/docs_update.go
  • shortcuts/doc/docs_update_test.go

📝 Walkthrough

Walkthrough

The v1 update validation now generates mode-specific error messages when a selection locator is missing. A new helper function returns selection requirement guidance and, for replace_all mode, additionally suggests using --mode overwrite.

Changes

Mode-Specific Selection Error Message

Layer / File(s) Summary
Mode-specific error message implementation
shortcuts/doc/docs_update.go
The new selectionRequiredMessageV1(mode string) helper builds the missing-selection guidance message and appends an instruction for "replace_all" mode to suggest --mode overwrite. The validateUpdateV1 function is updated to call this helper instead of using a static message.
Validation tests
shortcuts/doc/docs_update_test.go
Two new test functions verify that selectionRequiredMessageV1 produces correct guidance: for "replace_all" mode, the message includes selection/overwrite guidance; for other modes like "replace_range", it omits the overwrite suggestion while still requiring selection flags.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Suggested labels

size/M, domain/ccm

Suggested reviewers

  • fangshuyu-768

Poem

🐰 A little helper hops along the way,
Suggesting overwrite when replace_all won't play,
When selection's missing from the command line,
Now users see guidance, a helpful design.
Tests verify each mode's own special way!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding clarity to error messages for replace_all selection validation by suggesting --mode overwrite as an alternative.
Description check ✅ Passed The description covers the motivation and changes, but lacks a formal Test Plan section matching the template structure with checkboxes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added domain/ccm PR touches the ccm domain size/M Single-domain feat or fix with limited business impact labels May 18, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2026

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 66.79%. Comparing base (315e0ab) to head (bc75e89).

Files with missing lines Patch % Lines
shortcuts/doc/docs_update.go 83.33% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #954   +/-   ##
=======================================
  Coverage   66.78%   66.79%           
=======================================
  Files         564      564           
  Lines       52441    52446    +5     
=======================================
+ Hits        35024    35029    +5     
  Misses      14516    14516           
  Partials     2901     2901           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@bc75e8996937a332db7bb9b2131393dd0fc9ff0b

🧩 Skill update

npx skills add afengzi/cli#codex/docs-replace-all-selection-hint -y -g

@fangshuyu-768 fangshuyu-768 merged commit 583349e into larksuite:main May 19, 2026
18 checks passed
@liangshuo-1 liangshuo-1 mentioned this pull request May 19, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain/ccm PR touches the ccm domain size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants