Skip to content

docs: CLI documentation updates from v0.9.4 dogfooding (GH-209, GH-212)#215

Merged
arcaputo3 merged 1 commit intomainfrom
gh-209-212-docs
Feb 8, 2026
Merged

docs: CLI documentation updates from v0.9.4 dogfooding (GH-209, GH-212)#215
arcaputo3 merged 1 commit intomainfrom
gh-209-212-docs

Conversation

@arcaputo3
Copy link
Copy Markdown
Contributor

Summary

Addresses #209 and #212.

Test plan

  • ./mill __.compile passes
  • Review SKILL.md for consistency and accuracy
  • Verify shell quoting examples work as documented

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

- Update function count 81 → 82 across SKILL.md (4 occurrences) and CLAUDE.md
- Add shell quoting section for sheet names with spaces (#209)
- Add accounting format example (#,##0.0_);(#,##0.0))
- Clarify putf vs put (putf always interprets as formula)
- Add move-sheet cross-sheet reference warning
- Add HTML export limitations note
- Add deep eval chain caveat for --eval
- Add rasterizer install instructions (librsvg, cairosvg)
- Add custom format string examples (decimal, percent, date, multiples)
- Clarify batch JSON numFormat key (camelCase, not format)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude bot commented Feb 8, 2026

PR Review: Documentation Updates

Thank you for this thorough documentation update from dogfooding! The improvements address real usability gaps discovered during actual usage. Here's my review:

✅ Strengths

  1. Shell quoting section - Excellent addition addressing bug(cli): Cross-sheet formula refs to sheet names with spaces impossible due to shell quoting #209. The examples are clear and the JSON workaround is helpful.
  2. Clarifications - The putf vs put distinction, move-sheet warning, HTML limitations, and batch JSON numFormat key are all valuable additions.
  3. Custom format examples - The accounting format and custom format string examples (decimal, percent, date, multiples) are practical and useful.
  4. Rasterizer instructions - Good addition with platform-specific commands.
  5. Caveats documented - The deep eval chain caveat and cross-sheet reference warning for move-sheet are important to surface.

⚠️ Critical Issue: Function Count Discrepancy

Problem: The PR claims to update the function count from 81 → 82, but there's an inconsistency:

  1. CLAUDE.md:186 - Updated to say "82 functions" ✅
  2. CLAUDE.md:284 - The actual function list contains only 80 functions
  3. SKILL.md - Updated to say "82 functions" in 4 places ✅
  4. plugin/skills/xl-cli/reference/FORMULAS.md:3 - Still says "81 functions" (not updated in this PR) ❌

Action Required:

  • Count the actual functions in CLAUDE.md:284 - I get 80 when parsing the comma-separated list
  • Determine the correct count (80, 81, or 82?)
  • Update all locations consistently
  • If 82 is correct, add the 2 missing functions to the CLAUDE.md list
  • Update reference/FORMULAS.md:3 to match

📝 Minor Issues

  1. Issue bug(cli): Cross-sheet formula refs to sheet names with spaces impossible due to shell quoting #209 status: The PR description says "closes bug(cli): Cross-sheet formula refs to sheet names with spaces impossible due to shell quoting #209" but the shell quoting section documents a workaround, not a fix. The issue describes a parser/CLI limitation that still exists. Consider changing to "addresses bug(cli): Cross-sheet formula refs to sheet names with spaces impossible due to shell quoting #209" or "documents workaround for bug(cli): Cross-sheet formula refs to sheet names with spaces impossible due to shell quoting #209" to avoid auto-closing the issue if a proper fix is still desired.

  2. Consistency: Since you're updating function counts, consider updating reference/FORMULAS.md in this same PR to keep documentation in sync.

  3. Test plan: The PR description test plan shows:

    • ./mill __.compile passes ✅
    • Review SKILL.md for consistency and accuracy ⬜
    • Verify shell quoting examples work as documented ⬜

    Consider completing these checks before merge.

🎯 Recommendations

  1. Verify function count: Run xl functions to get the ground truth count from the actual codebase
  2. Update FORMULAS.md: Include it in this PR for consistency
  3. Test shell examples: Verify the shell quoting examples work as documented (especially the heredoc example)

Code Quality

  • ✅ Documentation-only changes, no code quality concerns
  • ✅ Follows project conventions in CLAUDE.md
  • ✅ Writing is clear and actionable
  • ✅ Examples are practical and realistic

Verdict

Request Changes - The function count discrepancy needs to be resolved before merge. Everything else looks excellent. Once the count is corrected across all files, this will be a solid documentation improvement.


How to fix:

# Get ground truth
xl functions | wc -l

# Update all locations with correct count:
# - CLAUDE.md (line 35, line 284 list)
# - SKILL.md (4 occurrences)  
# - reference/FORMULAS.md (line 3)

@arcaputo3 arcaputo3 merged commit 09a1747 into main Feb 8, 2026
2 checks passed
@arcaputo3 arcaputo3 deleted the gh-209-212-docs branch February 8, 2026 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(cli): Cross-sheet formula refs to sheet names with spaces impossible due to shell quoting

1 participant