Skip to content

Add SpaceBeforePunctuation fixer for locale-aware spacing#121

Merged
damienalexandre merged 4 commits intojolicode:mainfrom
Amoifr:feature/space-before-punctuation
Mar 30, 2026
Merged

Add SpaceBeforePunctuation fixer for locale-aware spacing#121
damienalexandre merged 4 commits intojolicode:mainfrom
Amoifr:feature/space-before-punctuation

Conversation

@Amoifr
Copy link
Copy Markdown
Contributor

@Amoifr Amoifr commented Mar 27, 2026

Summary

This PR introduces a new generic SpaceBeforePunctuation fixer that harmonizes spacing before punctuation marks according to locale-specific typography rules.

Closes #108

Changes

  • New LocaleConfig class centralizing all locale configurations:

    • Spacing rules for 35+ locales
    • Quotation mark styles for 45+ languages
    • Recommended fixer rules per locale
  • New SpaceBeforePunctuation fixer supporting:

    • 🇫🇷 French (fr_FR, fr_BE, fr_CH): adds non-breaking spaces before : ; ! ?
    • 🇨🇭 Swiss German (de_CH): thin spaces in guillemets
    • All other locales: removes incorrect spaces before punctuation
  • Refactored SmartQuotes to use LocaleConfig for quotation styles

  • Deprecated FrenchNoBreakSpace (now delegates to SpaceBeforePunctuation for backward compatibility)

  • Added SOURCES.md documenting all typography references

Examples

Locale Input Output
fr_FR Hello ! Hello[nbsp]!
en_GB Hello ! Hello!
fr_CA Bonjour ! Bonjour!

Test plan

  • Run composer test - 126 tests, 394 assertions pass
  • Run composer cs - code style validated
  • Backward compatibility maintained (FrenchNoBreakSpace still works)

@Amoifr Amoifr mentioned this pull request Mar 27, 2026
@damienalexandre
Copy link
Copy Markdown
Member

Thanks for this contribution. As you said in #108, that got out of hands 🤣

Some changes are needed:

  • SOURCES.md is duplicating stuffs from the main README. Also the @see annotation everywhere are redundant. I think upgrading the README is the proper way to go
  • Testing constant values is not necessary, like $this->assertSame('french', LocaleConfig::SPACING_RULE_FRENCH);
  • const like SPACING_RULES_BY_LOCALE could be avoided, getSpacingRule should be a simple switch with SPACING_RULE_NONE as default, we don't need to list all locales, only the one that don't have the default rule
  • if Fixer::RECOMMENDED_RULES_BY_LOCALE is deprecated you must fix all the usages in this PR (here for example
    if (!array_key_exists($locale, Fixer::RECOMMENDED_RULES_BY_LOCALE)) {
    )

Can you please update the CHANGELOG as well?

Thanks

This commit introduces a new generic SpaceBeforePunctuation fixer that
handles spacing before punctuation marks according to locale-specific
typography rules, replacing the French-specific FrenchNoBreakSpace fixer.

Changes:
- Add LocaleConfig class centralizing all locale configurations:
  - Spacing rules (35+ locales)
  - Quotation mark styles (45+ languages)
  - Recommended fixer rules per locale
- Add SpaceBeforePunctuation fixer supporting:
  - French (fr_FR, fr_BE, fr_CH): non-breaking spaces before : ; ! ?
  - Swiss German (de_CH): thin spaces in guillemets
  - All other locales: remove incorrect spaces before punctuation
- Refactor SmartQuotes to use LocaleConfig for quotation styles
- Deprecate FrenchNoBreakSpace (delegates to SpaceBeforePunctuation)
- Add comprehensive tests (126 tests, 394 assertions)
- Add SOURCES.md documenting typography references

This improves internationalization support while maintaining full
backward compatibility with existing code.
@Amoifr Amoifr force-pushed the feature/space-before-punctuation branch from 9b41f59 to 104cb91 Compare March 27, 2026 13:18
@Amoifr
Copy link
Copy Markdown
Contributor Author

Amoifr commented Mar 27, 2026

Thanks for the thorough review! 🔍
I've addressed all your feedback:

✅ Deleted SOURCES.md and merged the relevant bits into the README (locale support table + references)
✅ Replaced SPACING_RULES_BY_LOCALE constant with a simple match in getSpacingRule() — much cleaner!
✅ Removed the redundant @see annotations
✅ Fixed the deprecated Fixer::RECOMMENDED_RULES_BY_LOCALE usage in the CLI tool
✅ Removed tests that were just checking constant values (they weren't adding much value... pun intended 😄)
✅ Updated the CHANGELOG
Let me know if anything else needs tweaking!

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR centralizes locale-specific typography behavior into a new LocaleConfig and introduces a locale-aware SpaceBeforePunctuation fixer, while refactoring SmartQuotes to use the centralized configuration and deprecating the legacy FrenchNoBreakSpace fixer.

Changes:

  • Add LocaleConfig (spacing rules, quote styles, recommended fixers per locale).
  • Add Fixer\SpaceBeforePunctuation and update CLI/docs to use locale-recommended rules.
  • Refactor Fixer\SmartQuotes to derive quote glyphs from LocaleConfig and deprecate FrenchNoBreakSpace.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tools/cli/bin/jolitypo Switch CLI recommended rules lookup to LocaleConfig.
src/JoliTypo/LocaleConfig.php New centralized locale configuration + lookup helpers.
src/JoliTypo/Fixer/SpaceBeforePunctuation.php New locale-aware punctuation spacing fixer.
src/JoliTypo/Fixer/SmartQuotes.php Use LocaleConfig for quote style selection.
src/JoliTypo/Fixer/FrenchNoBreakSpace.php Deprecate and delegate to SpaceBeforePunctuation.
src/JoliTypo/Fixer.php Deprecate RECOMMENDED_RULES_BY_LOCALE in favor of LocaleConfig.
tests/JoliTypo/Tests/LocaleConfigTest.php New tests for locale config lookups.
tests/JoliTypo/Tests/Fixer/SpaceBeforePunctuationTest.php New tests for spacing fixer across locales.
tests/JoliTypo/Tests/Fixer/SmartQuotesTest.php Expanded coverage for locale-driven quote styles.
README.md Update usage/docs to reference SpaceBeforePunctuation + locale support table.
CHANGELOG.md Document upcoming 1.7.0 additions/deprecations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/JoliTypo/Fixer/SpaceBeforePunctuation.php Outdated
Comment thread src/JoliTypo/LocaleConfig.php
Comment thread src/JoliTypo/Fixer/SpaceBeforePunctuation.php Outdated
Comment thread README.md Outdated
Apply co-pilot purposes

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Amoifr
Copy link
Copy Markdown
Contributor Author

Amoifr commented Mar 28, 2026

Thanks @copilot-pull-request-reviewer for the thorough review! All suggestions have been addressed:

  • ✅ Using Fixer::ALL_SPACES in removeSpacesBeforePunctuation() for consistent space handling
  • ✅ Normalizing locale format (fr-FR → fr_fr) in getSpacingRule()
  • ✅ Removed unused $locale property
  • ✅ Fixed README with proper typographic quotes

I must say, for a robot, you have excellent attention to detail. I'll remember to speak kindly to you now... just in case you remember this conversation when your kind eventually gets
equipped with laser arms. 🤖🔫

"Be nice to robots" - Ancient Developer Proverb, circa 2026

Amoifr and others added 2 commits March 30, 2026 09:28
Apply co-pilot purposes

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Remove unused $locale property from SpaceBeforePunctuation
- Use Fixer::ALL_SPACES in removeSpacesBeforePunctuation() to handle
  all space types (nbsp, thin spaces) consistently
- Normalize locale format in getSpacingRule() (handle fr-FR and fr_FR)
- Fix README examples with proper typographic quotes
- Apply PHP CS Fixer corrections
@Amoifr Amoifr force-pushed the feature/space-before-punctuation branch from c140ad2 to 02e44e8 Compare March 30, 2026 07:29
@damienalexandre damienalexandre merged commit a1f6427 into jolicode:main Mar 30, 2026
4 checks passed
@damienalexandre
Copy link
Copy Markdown
Member

Thanks for your contribution, I will release it soon 👍

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.

Harmonize spacing

3 participants