+
Skip to content

Conversation

naoNao89
Copy link
Contributor

@naoNao89 naoNao89 commented Oct 6, 2025

The ls command didn't support locale-aware filename quoting, making output less readable for non-English users.

Solution: Added --quoting-style=locale option that reads LC_ALL/LC_CTYPE/LANG and applies culturally appropriate quotation marks:

  • French/Spanish/Russian: « file »
  • German/Czech: „ file "
  • Japanese: 「 file 」
  • Chinese/Korean: " file "
  • English/default: " file "

Changes:

  • New Quotes::Locale variant with locale detection module supporting 20+ languages
  • Enhanced CQuoter to handle multi-byte UTF-8 quote characters
  • CLI integration and localization (English + French)

Partially addresses #1872.

Implement locale-aware quoting for the ls command, allowing filenames
to be quoted using locale-specific quotation marks based on the
current LC_CTYPE setting.

Features:
- Add Quotes::Locale variant for locale-aware quoting
- Create locale_quotes module with comprehensive locale mappings
- Support 20+ languages with appropriate quote characters:
  * Romance languages (French, Spanish, etc.): guillemets
  * Germanic languages (German, Czech, etc.): low-9 and high quotes
  * Japanese: corner brackets
  * Chinese/Korean: CJK curly quotes
  * English/default: ASCII double quotes
- Proper UTF-8 handling for multi-byte quote characters
- Environment variable precedence: LC_ALL > LC_CTYPE > LANG
- Localized help text in English and French

Implementation:
- Enhanced CQuoter to dynamically detect and apply locale quotes
- Added --quoting-style=locale CLI option
- Follows C-style quoting semantics (always-quote behavior)
- Safe fallback to ASCII double quotes for unknown locales
- Added spell-checker:ignore comments for technical terms

Testing:
- Verified with multiple locales (en_US, fr_FR, de_DE, ja_JP, zh_CN)
- All existing tests pass
- Help text properly localized
@naoNao89 naoNao89 force-pushed the feat/ls-quoting-style-locale branch from 149b5e8 to c25f9cd Compare October 6, 2025 00:53
Copy link

github-actions bot commented Oct 6, 2025

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

Copy link

github-actions bot commented Oct 6, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)

@naoNao89 naoNao89 force-pushed the feat/ls-quoting-style-locale branch from 532b9f4 to c25f9cd Compare October 6, 2025 02:18
Copy link

github-actions bot commented Oct 6, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@sylvestre
Copy link
Contributor

same comment, please write the summary yourself

@sylvestre
Copy link
Contributor

and needs tests

Add test coverage for locale-aware quoting functionality:
- Tests 10 different locales with appropriate quotation marks
- Verifies English, French, German, Japanese, Chinese, Russian, Spanish, Polish, C, and POSIX locales
- Tests escape sequence handling with locale quoting (newline character)
- Validates UTF-8 encoding of multi-byte quote characters
Copy link

github-actions bot commented Oct 8, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

Copy link

codspeed-hq bot commented Oct 8, 2025

CodSpeed Performance Report

Merging #8825 will improve performances by 3.03%

Comparing naoNao89:feat/ls-quoting-style-locale (21def45) with main (daa74e7)

Summary

⚡ 1 improvement
✅ 104 untouched
⏩ 73 skipped1

Benchmarks breakdown

Benchmark BASE HEAD Change
ls_recursive_long_all_wide_tree[(15000, 1500)] 149.8 ms 145.4 ms +3.03%

Footnotes

  1. 73 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@sylvestre
Copy link
Contributor

many jobs are failing

Fix test failures by limiting locale tests to those available in the CI environment.
CI only generates en_US.UTF-8, fr_FR.UTF-8, es_ES.UTF-8, and sv_SE.UTF-8.

Removed tests for: de_DE, ja_JP, zh_CN, pl_PL, ru_RU.UTF-8
- These locales are not generated in .github/workflows/GnuTests.yml
- The locale_quotes module unit tests still validate these quote types
- This keeps the integration test CI-friendly while maintaining comprehensive coverage
Copy link

github-actions bot commented Oct 8, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

Add missing Quotes::Locale test case in test_quotes_display() to ensure
the Display trait implementation is tested for all quote variants.

Add comprehensive unit tests for locale_quotes module:
- Test locale environment variable precedence (LC_ALL > LC_CTYPE > LANG)
- Validate quote character mappings for all supported locales
- Ensure Romance, Germanic, Slavic, and Asian language quotes work correctly
- Test locale string parsing with encoding and modifiers
- Verify fallback behavior for unknown locales

This adds 17 unit tests covering the locale detection and quote mapping
functionality that was previously untested.
@naoNao89 naoNao89 force-pushed the feat/ls-quoting-style-locale branch from 2279a7a to 51bcdfe Compare October 9, 2025 07:48
…ariables

Add test_ls_quoting_style_locale_env_vars to verify that --quoting-style=locale
correctly responds to different locale environment variables (LC_ALL).

Tests verify:
- French locale (fr_FR.UTF-8) uses guillemets (« »)
- Spanish locale (es_ES.UTF-8) uses guillemets (« »)
- Swedish locale (sv_SE.UTF-8) uses ASCII quotes
- C locale uses ASCII quotes

Note: Environment variable precedence testing (LC_ALL > LC_CTYPE > LANG) is
already comprehensively covered in unit tests (locale_quotes.rs). These
integration tests focus on end-to-end functionality.
Copy link

github-actions bot commented Oct 9, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

- Add locale/typography terms (CTYPE, Guillemets, guillemets) to spell checker dictionary
- Skip newline filename test on Windows where such filenames are invalid
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@naoNao89 naoNao89 marked this pull request as ready for review October 11, 2025 19:28
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

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载