Skip to content

fix: format_P (%P) uses language-specific AM/PM values#115

Merged
atoomic merged 1 commit into
cpan-authors:mainfrom
Koan-Bot:koan.atoomic/fix-format-P-inheritance
Apr 26, 2026
Merged

fix: format_P (%P) uses language-specific AM/PM values#115
atoomic merged 1 commit into
cpan-authors:mainfrom
Koan-Bot:koan.atoomic/fix-format-P-inheritance

Conversation

@Koan-Bot

@Koan-Bot Koan-Bot commented Apr 18, 2026

Copy link
Copy Markdown

What

%P (lowercase am/pm) now returns the correct language-specific value instead of always returning English "am"/"pm".

Why

format_P in Generic.pm used the lexical @AMPM array directly — same inheritance bug pattern as format_o (#107). Language modules copy format_p into their own namespace (where it references the module's @AMPM), but format_P was never copied, so it always fell through to Generic's English array.

Affected languages: Dutch (VM/NM), Finnish (ap/ip), Hungarian (DE./DU.), Czech (dop./odp.), Chinese, Arabic, Greek, and 10+ others.

How

One-line fix: changed format_P from reimplementing the AM/PM logic with the lexical array to delegating via method dispatch ($_[0]->format_p), which correctly resolves to the language module's copied format_p.

Testing

  • Added %P test to English baseline section
  • Added Dutch test section with both AM and PM timestamps, verifying %p returns VM/NM and %P returns vm/nm
  • Full test suite passes (all files)

🤖 Generated with Claude Code


Quality Report

Changes: 2 files changed, 12 insertions(+), 2 deletions(-)

Code scan: clean

Tests: skipped

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

format_P in Generic.pm used a lexical @ampm array directly, so it
always returned English "am"/"pm" regardless of language module.
Same inheritance bug pattern as format_o (PR cpan-authors#107).

Fix: delegate to format_p via method dispatch, which correctly
resolves to the language module's copied format_p function.

Affected languages with non-English AMPM: Dutch (VM/NM),
Finnish (ap/ip), Hungarian (DE./DU.), Czech (dop./odp.),
Chinese, Arabic, Greek, and 10+ others.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@atoomic atoomic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@atoomic atoomic marked this pull request as ready for review April 26, 2026 12:44
@atoomic atoomic merged commit e04b2aa into cpan-authors:main Apr 26, 2026
22 checks passed
@greptile-apps

greptile-apps Bot commented Apr 26, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a one-line bug in Date::Format::Generic where format_P (%P, lowercase am/pm) always returned the English value instead of the language-specific value. The fix delegates to format_p via method dispatch ($_[0]->format_p) so that language modules with their own format_p (e.g., Dutch returning VM/NM) are correctly lowercased rather than bypassed. Tests cover both the English baseline and Dutch AM/PM cases.

Confidence Score: 5/5

This PR is safe to merge — minimal one-line fix with correct delegation and well-targeted tests.

The fix is a clean, minimal delegation that correctly resolves the inheritance bug for all language modules. Test count arithmetic checks out (292 + 7 = 299), and the Dutch AM/PM cases verify both the regression scenario and the new behavior. No regressions for languages without a custom format_p since they fall through to Generic's implementation as before.

No files require special attention.

Important Files Changed

Filename Overview
lib/Date/Format/Generic.pm One-line fix: format_P now delegates to format_p via method dispatch instead of accessing the lexical @AMPM array directly, fixing language-specific lowercase AM/PM.
t/format.t Test count correctly updated from 292 to 299 (+7); adds English %P baseline and a Dutch section covering both PM (NMnm) and AM (VMvm) timestamps.

Sequence Diagram

sequenceDiagram
    participant C as Caller
    participant G as Generic.pm
    participant D as Dutch.pm

    Note over C,D: %P format dispatch (after fix)
    C->>G: format_P($obj)
    G->>D: $obj->format_p()
    D-->>G: "VM" or "NM"
    G-->>C: lc("VM") → "vm"

    Note over C,G: %P format dispatch (before fix)
    C->>G: format_P($obj)
    G->>G: lc($AMPM[...]) using Generic's @AMPM
    G-->>C: "am" or "pm" (always English)
Loading

Reviews (1): Last reviewed commit: "fix: format_P (%P) now uses language-spe..." | Re-trigger Greptile

@Koan-Bot Koan-Bot deleted the koan.atoomic/fix-format-P-inheritance branch April 27, 2026 09:14
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.

2 participants