Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

more PHPTs for multi-byte support #2

Open
wants to merge 4 commits into
base: win_mb_path_w_only
Choose a base branch
from

Conversation

mattficken
Copy link

most of these PHPTs are for the ANSI fallback behavior.

added some PHPTs for PHAR extension.

@weltling
Copy link
Owner

weltling commented Jun 13, 2016

Thanks for the PR, Matt. I've checked and found several issues, all seem to be of the test bug nature

  • several tests are missing data, ext/tidy/tests/005_charset-mb.phpt and some others
  • some tests have data in wrong charset, fe same ext/tidy/tests/005_charset-mb.phpt has the hardcoded UTF-8 content but sets default_charset=cp1251. This will fail, the hardcoded content needs to be encoded as default_charset/internal_encoding tell
  • the outputs in some tests don't match (probably c/p mistake), like ext\phar\tests\tar\tar_001-mb.phpt
  • some codepages like 932 can show some incompatibilities with western codepages, so the tests involving them should be verified carefully on machines with cp 932 and some european like 1252, at least
  • the function get_basename_with_cp() checks the files created by PHP using PS and expects codepage. In ext\standard\tests\file\windows_mb_path\bug54977_charset.phpt the default_charset is SJIS (thus the path returned by readdir as well), but get_base_name_with_cp() is passed 65001 instead of 932. But even passing it correct, it might be that on a non 932 cp PS would fails to accept cp 932, so a test like this might still need cp932 by default (or another ways to check).

Please check this points.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants