Skip to content

fix(slides): clarify xml-text-overlap-lint error for positional argument #1986

Merged
BD-ZERO merged 2 commits into
mainfrom
fix/clarify_lint_flag
Jul 21, 2026
Merged

fix(slides): clarify xml-text-overlap-lint error for positional argument #1986
BD-ZERO merged 2 commits into
mainfrom
fix/clarify_lint_flag

Conversation

@BD-ZERO

@BD-ZERO BD-ZERO commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Description:

Summary

  • When a positional argument is passed instead of --input <path>, the CLI error message now explicitly hints ,need --input, making the required flag obvious instead of a bare "unexpected argument" message.
  • Add regression test covering the CLI's stderr/exit-code behavior for this case, and fix an indentation bug in the test file (the new test method and its body were over-indented relative to the surrounding test
    methods).

Test plan

  • python3 -m unittest xml_text_overlap_lint_test -v — 53 tests pass

Summary by CodeRabbit

  • Bug Fixes

    • Improved command-line error messaging when a positional argument is provided, now advising to use the required --input option.
    • Maintained clear, deterministic failure behavior for invalid CLI usage.
  • Tests

    • Added a unit test to verify the CLI exits with code 1, outputs no stdout, and reports the updated “unexpected argument” error with the --input suggestion.

@BD-ZERO
BD-ZERO requested a review from ethan-zhx July 21, 2026 11:52
@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label Jul 21, 2026
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9c1900e8-d443-4360-9835-7405f574efea

📥 Commits

Reviewing files that changed from the base of the PR and between 4138025 and df2b9d3.

📒 Files selected for processing (1)
  • skills/lark-slides/scripts/xml_text_overlap_lint.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • skills/lark-slides/scripts/xml_text_overlap_lint.py

📝 Walkthrough

Walkthrough

The XML text overlap lint CLI now advises using --input when passed a positional argument, with a regression test covering exit status and output streams.

Changes

CLI argument guidance

Layer / File(s) Summary
Argument error and regression test
skills/lark-slides/scripts/xml_text_overlap_lint.py, skills/lark-slides/scripts/xml_text_overlap_lint_test.py
The parser appends , need --input to unexpected-argument errors, and the CLI test verifies the failure status, empty stdout, and stderr guidance.

Estimated code review effort: 2 (Simple) | ~5 minutes

Suggested reviewers: ethan-zhx

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 clearly and concisely summarizes the main change: improving the xml-text-overlap-lint error for positional arguments.
Description check ✅ Passed The description covers the summary and test plan, with only the Changes and Related Issues sections omitted from the template.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/clarify_lint_flag

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.

@BD-ZERO
BD-ZERO requested a review from fangshuyu-768 July 21, 2026 11:52

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@skills/lark-slides/scripts/xml_text_overlap_lint.py`:
- Line 69: Replace the fullwidth comma in the unexpected-argument error emitted
by fail with a standard comma followed by a space. Update the matching assertion
in skills/lark-slides/scripts/xml_text_overlap_lint_test.py lines 50-53 to
expect the exact revised message.
🪄 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: bdab7a2e-17ad-4e2e-ac97-427fd3a0706e

📥 Commits

Reviewing files that changed from the base of the PR and between daaacb4 and 4138025.

📒 Files selected for processing (2)
  • skills/lark-slides/scripts/xml_text_overlap_lint.py
  • skills/lark-slides/scripts/xml_text_overlap_lint_test.py

Comment thread skills/lark-slides/scripts/xml_text_overlap_lint.py Outdated
fangshuyu-768
fangshuyu-768 previously approved these changes Jul 21, 2026
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.05%. Comparing base (daaacb4) to head (df2b9d3).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1986   +/-   ##
=======================================
  Coverage   75.05%   75.05%           
=======================================
  Files         901      901           
  Lines       95391    95391           
=======================================
  Hits        71591    71591           
  Misses      18307    18307           
  Partials     5493     5493           

☔ View full report in Codecov by Harness.
📢 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

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add larksuite/cli#fix/clarify_lint_flag -y -g

@BD-ZERO
BD-ZERO merged commit 12ca42c into main Jul 21, 2026
28 of 41 checks passed
@BD-ZERO
BD-ZERO deleted the fix/clarify_lint_flag branch July 21, 2026 12:29
@liangshuo-1 liangshuo-1 mentioned this pull request Jul 21, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants