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

SJIS, EUC-JP, JISに変換できないときに該当の文字を無視するオプションを追加する #41

Merged
merged 4 commits into from
Jun 2, 2024
Merged

Conversation

syumai
Copy link
Contributor

@syumai syumai commented May 28, 2024

  • 別の文字コードへの変換時に変換不可能な文字が含まれる時、fallbackの表現への置き換えではなく、単にそれを無視するオプションが必要となったため追加させていただきたいです。
    • オプションの名前にはこだわりは無く、もし ignore が望ましくなければ、他のものでも問題ありません。
    • 実は、現行の実装でも、存在しないfallbackOptionを指定することでこの振る舞いを実現することが出来ますが、今後同じ挙動をすることが保証されないため、明示的に無視できるオプションを追加させていただきたいです。
  • このオプションがあれば、例えば 🍣寿司ビール🍺 (UNICODE) が ?寿司ビール? (Shift_JIS) に変換されていたものを 寿司ビール (Shift_JIS) に変換可能となります。
  • こちらのご対応をもし取り込んでいただけましたら、DefinitelyTypedの方にもPRを出させていただきます。
  • 関連PR: SJIS, EUC-JP, JISに変換できないときに数値文字参照に変換する #23

@polygonplanet
Copy link
Owner

polygonplanet commented Jun 2, 2024

@syumai
大変丁寧にPR出していただき、テストもREADMEも(日英訳も!)ありがとうございます!
動作も確認しました。ignore というオプション名で適切と思いましたので、このまま取り込ませていただきますね。

実は、現行の実装でも、存在しないfallbackOptionを指定することでこの振る舞いを実現することが出来ますが

これは考えてなかったです。なるほどと思いました💡

変換できない文字がある場合、fallback: 'ignore' をつけると、

  • 現行バージョン:ignoreオプションの仕様はないが処理の結果的に無視される
  • 次バージョン:ignoreオプションの仕様があり、無視されることが保証される

となります。次バージョンで本PR内容を反映させていただきます。

Copy link
Owner

@polygonplanet polygonplanet left a comment

Choose a reason for hiding this comment

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

LGTM!

@polygonplanet polygonplanet merged commit a9a4172 into polygonplanet:master Jun 2, 2024
@syumai syumai deleted the add-ignore-handle-fallback-option branch June 2, 2024 06:05
@syumai
Copy link
Contributor Author

syumai commented Jun 2, 2024

ご確認いただきありがとうございます!一件、ドキュメンテーションが誤っていたことについ先ほど気付いたため、別途PRを出させていただきます🙏

@syumai
Copy link
Contributor Author

syumai commented Jun 2, 2024

追ってDefinitely Typedの方にもPRを出します!

@polygonplanet
Copy link
Owner

@syumai
v2.2.0 としてリリースし、内容反映させていただきました。
今回の { fallback: 'ignore' } と一緒に、 { fallback: 'error' } も追加しました ( #44 )
v2.2.0でのAPI追加・変更はこのオプションのみです( 差分:2.1.0...2.2.0
ご報告まで、PRありがとうございました!

@syumai
Copy link
Contributor Author

syumai commented Jun 10, 2024

@polygonplanet 承知しました。リリース & ご連絡ありがとうございます!

@syumai
Copy link
Contributor Author

syumai commented Jul 3, 2024

本対応と合わせて、 #44 で導入された error fallback option の追加も行うPRをDefinitelyTypedに上げました
DefinitelyTyped/DefinitelyTyped#69982

@syumai
Copy link
Contributor Author

syumai commented Jul 4, 2024

DefinitelyTypedの方、マージされてリリース完了しました!
https://www.npmjs.com/package/@types/encoding-japanese/v/2.2.0

@polygonplanet
Copy link
Owner

型も更新いただいて、情報共有もいただき大変感謝です👍 ありがとうございました!

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