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

line end conversion does not work for old ByteTextConverters #4506

Open
Rydier opened this issue Sep 5, 2019 · 1 comment

Comments

@Rydier
Copy link

commented Sep 5, 2019

While these have been replaced by the new ZnCharacterEncoders, it might be nice to fix (some of?) the bugs that exist in the old TextConverters, for educational reasons and/or potential backport to Pharo 6.1 (... and maybe as a cautionary tale of the dangers of mixing too many responsibilities)

The issues are:

  • In installLineEndConvention:, the latin1Map is replaced by nonAsciiMap, meaning all non-ascii characters should be translated. This only works for utf8 - in the byte converters there are usually many entries with byte encoding equivalent to unicode code points.
    When the optimized nextPutByteString: searches for next index to convert, for most non-ascii, it will then encounter false hits, for which the latin1Encodings does not contain a set of bytes to translate to, because they shouldn't.
  • In the "slow" fallback for WideStrings, unicodeToByte: is used. This method precedes the latin1Encoding optimalization / lineEndConvension being added to Converter responsibilities, and thus assumes each code point corresponds to at most one byte. This is not true when cr can be converted to crlf.

How line end convention is supposed to work when decoding is anyone's guess, the above applies to the encoding process.

Another minor bug:

  • installLineEndConvention uses convertFromSystemString: when the correct call would be convertToSystemString:. Luckily, cr and lf are the same in all encodings, but it highlights the importance of method comments that state what one means by "system" string ;)
@welcome

This comment has been minimized.

Copy link

commented Sep 5, 2019

Thanks for opening your first issue! Please check the CONTRIBUTING documents for some tips about which information should be provided. You can find information of how to do a Pull Request here: https://github.com/pharo-project/pharo/wiki/Contribute-a-fix-to-Pharo

GitHub
Pharo is a dynamic reflective pure object-oriented language supporting live programming inspired by Smalltalk. - pharo-project/pharo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.