feat(sheets): add text wrapping, alignment, and font formatting to format_sheet_range - #416
Conversation
…rmat_sheet_range Extends format_sheet_range with new parameters: - wrap_strategy: WRAP, CLIP, OVERFLOW_CELL - horizontal_alignment: LEFT, CENTER, RIGHT - vertical_alignment: TOP, MIDDLE, BOTTOM - bold: boolean for bold text - italic: boolean for italic text - font_size: integer for font size in points All parameters work alongside existing color and number format options. Includes 21 unit tests covering all new functionality. Added to extended tier in tool_tiers.yaml. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fixed formatting issues in: - gcalendar/calendar_tools.py - gsheets/sheets_tools.py - tests/gsheets/test_format_sheet_range.py Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR extends the format_sheet_range function with comprehensive text formatting capabilities, moving it from the Complete tier to the Extended tier. The enhancement adds text wrapping strategies, horizontal and vertical alignment options, and font styling (bold, italic, and size).
Changes:
- Added 6 new optional parameters to
format_sheet_range(wrap_strategy, horizontal_alignment, vertical_alignment, bold, italic, font_size) - Refactored implementation into a testable internal function
_format_sheet_range_impl - Added comprehensive test coverage with 21 unit tests covering all new parameters and edge cases
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| gsheets/sheets_tools.py | Extracted implementation to _format_sheet_range_impl and added validation/normalization for new text formatting parameters |
| tests/gsheets/test_format_sheet_range.py | Added 21 unit tests covering wrap strategies, alignments, font styling, parameter validation, and case-insensitive handling |
| core/tool_tiers.yaml | Moved format_sheet_range from complete to extended tier |
| README_NEW.md | Updated tool tier documentation and enhanced feature description |
| README.md | Added format_sheet_range entry to extended tier tools table |
| gcalendar/calendar_tools.py | Reformatted lambda expressions for consistent multi-line parenthesization |
| # Build confirmation message | ||
| applied_parts = [] | ||
| if bg_color_parsed: | ||
| applied_parts.append(f"background {background_color}") |
There was a problem hiding this comment.
The confirmation message uses inconsistent terminology. Line 577 says 'background {background_color}' while this line says 'text color {text_color}'. For consistency, consider using either 'background color' or just 'background' and 'text' for both.
| applied_parts.append(f"background {background_color}") | |
| applied_parts.append(f"background color {background_color}") |
| .get(calendarId=calendar_id, eventId=event_id) | ||
| .execute() | ||
| lambda: ( | ||
| service.events().get(calendarId=calendar_id, eventId=event_id).execute() |
There was a problem hiding this comment.
This formatting change appears unrelated to the PR's stated purpose of adding text formatting to format_sheet_range. These lambda reformatting changes in calendar_tools.py should be in a separate PR focused on code style consistency.
…into feature/enhanced-sheets-formatting
|
Looks good, thanks! |
| ) | ||
|
|
||
| error_msg = str(exc_info.value).lower() | ||
| assert "wrap_strategy" in error_msg or "wrap" in error_msg |
There was a problem hiding this comment.
The error message assertion is overly permissive. The condition or 'wrap' in error_msg could match unintended error messages that happen to contain the word 'wrap' in a different context. Consider tightening the assertion to only check for 'wrap_strategy' or verify the exact error message format.
| assert "wrap_strategy" in error_msg or "wrap" in error_msg | |
| assert "wrap_strategy" in error_msg |
| ) | ||
|
|
||
| error_msg = str(exc_info.value).lower() | ||
| assert "horizontal" in error_msg or "left" in error_msg |
There was a problem hiding this comment.
The error message assertion is overly permissive. Checking for 'left' could match unintended messages. Consider checking only for 'horizontal' or 'horizontal_alignment' to ensure the correct error is raised.
| assert "horizontal" in error_msg or "left" in error_msg | |
| assert "horizontal" in error_msg or "horizontal_alignment" in error_msg |
| ) | ||
|
|
||
| error_msg = str(exc_info.value).lower() | ||
| assert "vertical" in error_msg or "top" in error_msg |
There was a problem hiding this comment.
The error message assertion is overly permissive. Checking for 'top' could match unintended messages. Consider checking only for 'vertical' or 'vertical_alignment' to ensure the correct error is raised.
| assert "vertical" in error_msg or "top" in error_msg | |
| assert "vertical" in error_msg or "vertical_alignment" in error_msg |
- Use consistent terminology in confirmation message ("background color"
to match "text color")
- Tighten test assertions for invalid parameter validation to check
parameter name and all allowed values instead of loose substring matches
- Remove unrelated calendar_tools.py changes (lambda reformatting, guest
permissions, query_freebusy) that belong in separate PRs
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This reverts commit e5a746d.
Summary
Extends
format_sheet_rangewith comprehensive text formatting capabilities:wrap_strategyparameter (WRAP, CLIP, OVERFLOW_CELL)horizontal_alignmentparameter (LEFT, CENTER, RIGHT)vertical_alignmentparameter (TOP, MIDDLE, BOTTOM)boldanditalicboolean parametersfont_sizeinteger parameterAll new parameters work alongside existing color and number format options.
Changes
gsheets/sheets_tools.py: Extendedformat_sheet_rangewith new parameterscore/tool_tiers.yaml: Addedformat_sheet_rangeto extended tierREADME_NEW.md: Updated documentation for extended tier and enhanced descriptiontests/gsheets/test_format_sheet_range.py: Added 21 unit testsTest Plan
Notes
Following feedback from PR #390:
tool_tiers.yamlentry