Skip to content

refactor(checkbox): share regex literals via exported storage vars#37

Merged
bborbe merged 1 commit into
masterfrom
feat/checkbox-shared-regex
Jun 28, 2026
Merged

refactor(checkbox): share regex literals via exported storage vars#37
bborbe merged 1 commit into
masterfrom
feat/checkbox-shared-regex

Conversation

@bborbe

@bborbe bborbe commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Summary

PR #36 broadened the checkbox regex to accept * list markers, but the pattern (and its two replacement variants) were duplicated across 7 inline sites in pkg/storage and pkg/ops. This refactor centralizes them as three exported package-level vars in pkg/storage:

  • storage.CheckboxRegex — detection (^(\s*)[-*] \[([ x/])\] (.+)$)
  • storage.CheckboxCompleteRegex — rewrite [ ] / [/][x] preserving marker
  • storage.CheckboxUncompleteRegex — rewrite [x][ ] preserving marker

pkg/ops/{complete,update,workon}.go drop their regexp imports entirely; pkg/ops/defer.go keeps it for regexp.MatchString on an unrelated path.

Per go-package-layout-guide.md, no new sub-package — promoted in place in pkg/storage (none of the 5 split triggers fire). The 3 new exported names are the only public API delta.

pkg/ops/lint.go lines 460 and 771 intentionally untouched — they use a different state class ([ xX]) by design.

Test plan

  • go test ./pkg/storage/... ./pkg/ops/... exits 0 (existing PR fix(parser): accept asterisk list-marker on checkbox regex #36 coverage exercises both dash and asterisk forms)
  • git grep -nE 'storage\.CheckboxRegex' pkg/ops/ returns 4 matches; storage.CheckboxCompleteRegex and storage.CheckboxUncompleteRegex each 1
  • No regexp.MustCompile.*\[-\*\] remaining in pkg/ops/
  • make precommit exits 0
  • git diff origin/master..HEAD -- pkg/ops/lint.go is empty

@bborbe bborbe marked this pull request as ready for review June 28, 2026 13:18

@ben-s-pull-request-reviewer ben-s-pull-request-reviewer 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.

Based on my analysis, here's my review:

Verification of all concerns from the plan:

Concern Status
pkg/storage/base.go - three exported vars added ✅ Verified: CheckboxRegex, CheckboxCompleteRegex, CheckboxUncompleteRegex at lines 30-39
pkg/ops/complete.go - regexp import removed ✅ Verified: no regexp references, uses storage.CheckboxRegex, storage.CheckboxCompleteRegex, storage.CheckboxUncompleteRegex
pkg/ops/defer.go - regexp import retained at line 87 ✅ Verified: import present, regexp.MatchString used for +Nd shorthand (unrelated to checkbox)
pkg/ops/update.go - regexp import removed ✅ Verified: no regexp references, uses storage.CheckboxRegex at line 151
pkg/ops/workon.go - regexp import removed ✅ Verified: no regexp references, uses storage.CheckboxRegex at line 244
pkg/ops/lint.go - untouched, different [ xX] regex ✅ Verified: lines 460/771 use (?m)^[\s]*[-*]\s+\[([ xX])\] (intentionally different - matches uppercase X)
CHANGELOG.md - updated ✅ Verified: line 13 documents the refactor accurately
prompts/completed/148-...md exists ✅ Verified: file exists with 260 lines

No issues found. The refactor is clean:

  • Regex literals are byte-for-byte identical to the replaced inline patterns
  • Import hygiene is correct across all files
  • lint.go's intentionally different [ xX] state-class regexes are preserved
  • The changelog accurately describes the scope
{
  "verdict": "approve",
  "summary": "Refactor is clean and correct. Three exported regex vars in pkg/storage/base.go correctly replace seven inline regexp.MustCompile call sites across pkg/ops/{complete,defer,update,workon}.go. Import hygiene verified: complete/update/workon correctly drop regexp import; defer retains it only for the +Nd shorthand at line 87. lint.go's intentionally different [ xX] regex shape at lines 460 and 771 is correctly untouched. CHANGELOG entry is accurate.",
  "comments": [],
  "concerns_addressed": [
    "correctness: pkg/storage/base.go exports CheckboxRegex, CheckboxCompleteRegex, CheckboxUncompleteRegex - byte-for-byte identical to replaced inline patterns",
    "correctness: pkg/ops/complete.go regexp import removed - no other regexp usage",
    "correctness: pkg/ops/defer.go regexp import retained at line 87 for +Nd shorthand",
    "correctness: pkg/ops/update.go regexp import removed - no other regexp usage",
    "correctness: pkg/ops/workon.go regexp import removed - no other regexp usage",
    "correctness: pkg/ops/lint.go untouched - intentionally different [ xX] regex at lines 460 and 771 preserved",
    "tests: existing test suites cover both dash and asterisk forms (PR #36)"
  ]
}

@bborbe bborbe merged commit ba3d5c7 into master Jun 28, 2026
1 check passed
@bborbe bborbe deleted the feat/checkbox-shared-regex branch June 28, 2026 13:23
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.

1 participant