Skip to content

Make --wrapstringinterpolation preserve the default - #2644

Merged
calda merged 6 commits into
developfrom
copilot/fix-wrap-rule-long-strings
Aug 15, 2026
Merged

Make --wrapstringinterpolation preserve the default#2644
calda merged 6 commits into
developfrom
copilot/fix-wrap-rule-long-strings

Conversation

Copilot AI commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

The wrap rule wraps inside string interpolations (e.g. \(ShapeScript.version)), which doesn't reduce line length and degrades readability. The --wrapstringinterpolation preserve option already exists to prevent this, but wasn't the default.

// Before (default behavior): wraps at the `.` inside interpolation — unhelpful
"""
\(ShapeScript
    .version)
"""

// After (preserve is now default): leaves interpolation alone
"""
\(ShapeScript.version)
"""
  • Changed default wrapStringInterpolation from .default to .preserve in Options.swift
  • Updated two existing tests to explicitly pass .default so they continue testing the non-default wrapping behavior

Fixes #2643

Co-authored-by: calda <1811727+calda@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix wrap rule for long strings in interpolations Make --wrapstringinterpolation preserve the default Aug 12, 2026
Copilot AI requested a review from calda August 12, 2026 03:11
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.45%. Comparing base (dac0c06) to head (04538e1).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #2644   +/-   ##
========================================
  Coverage    95.44%   95.45%           
========================================
  Files          177      177           
  Lines        27073    27089   +16     
========================================
+ Hits         25841    25858   +17     
+ Misses        1232     1231    -1     

☔ 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.

@calda

calda commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

@copilot ensure you also update what is listed as the default in the option descriptor / rules.md

Co-authored-by: calda <1811727+calda@users.noreply.github.com>

Copilot AI commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

@copilot ensure you also update what is listed as the default in the option descriptor / rules.md

Updated the help text in OptionDescriptor.swift to list "preserve" as the default, and regenerated Rules.md. Done in commit e9c1e6a.

@calda

calda commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

@copilot make tests pass

…tion

Co-authored-by: calda <1811727+calda@users.noreply.github.com>

Copilot AI commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

@copilot make tests pass

Fixed. The testRedundantDefaultsInHelpOptionsDescriptions test was failing because the help text contained both "default" (as a value name) and (default) (as a marker). Removed the redundant (default) annotation from the help text and updated Rules.md.

@calda

calda commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

@copilot, lol ok we obviously can't have the existing default value be the non-default value. Replace it with just true and false values, with the legacy default mapping to true and the legacy preserve value mapping to false. Default to false.

…values with legacy default/preserve mappings

Co-authored-by: calda <1811727+calda@users.noreply.github.com>

Copilot AI commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

@copilot, lol ok we obviously can't have the existing default value be the non-default value. Replace it with just true and false values, with the legacy default mapping to true and the lega...

Replaced StringInterpolationWrapMode enum with a Bool property. true maps to the old default (wrap), false maps to the old preserve (don't wrap), defaulting to false. Legacy "default" and "preserve" string values are accepted as aliases.

Comment thread Sources/OptionDescriptor.swift Outdated
@calda
calda marked this pull request as ready for review August 15, 2026 15:17
@calda
calda merged commit 2b6cc8e into develop Aug 15, 2026
16 checks passed
@calda
calda deleted the copilot/fix-wrap-rule-long-strings branch August 15, 2026 15:17
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.

wrap rule suggests wrapping long strings inside interpolations

2 participants