Skip to content

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

Merged
polygonplanet merged 4 commits intopolygonplanet:masterfrom
basemachina:add-ignore-handle-fallback-option
Jun 2, 2024
Merged

SJIS, EUC-JP, JISに変換できないときに該当の文字を無視するオプションを追加する#41
polygonplanet merged 4 commits intopolygonplanet:masterfrom
basemachina:add-ignore-handle-fallback-option

Conversation

@syumai
Copy link
Copy Markdown
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
Copy Markdown
Owner

polygonplanet commented Jun 2, 2024 •
edited
Loading

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

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

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

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

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

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

polygonplanet
polygonplanet approved these changes Jun 2, 2024
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

syumai commented Jun 2, 2024

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

@syumai syumai mentioned this pull request Jun 2, 2024
Merged
@syumai
Copy link
Copy Markdown
Contributor Author

syumai commented Jun 2, 2024

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

@polygonplanet
Copy link
Copy Markdown
Owner

polygonplanet commented Jun 8, 2024

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

@syumai
Copy link
Copy Markdown
Contributor Author

syumai commented Jun 10, 2024

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

@syumai
Copy link
Copy Markdown
Contributor Author

syumai commented Jul 3, 2024

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

@syumai
Copy link
Copy Markdown
Contributor Author

syumai commented Jul 4, 2024

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

@polygonplanet
Copy link
Copy Markdown
Owner

polygonplanet commented Jul 4, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@polygonplanet polygonplanet polygonplanet approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@syumai @polygonplanet