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

Kconv.guess: 返り値から nil を削除 #2891

Merged
merged 1 commit into from
Jun 9, 2024
Merged

Conversation

tk0miya
Copy link
Contributor

@tk0miya tk0miya commented Jun 6, 2024

Kconv.guess の返り値は Encoding | nil と説明されていますが、nil を返すことはなさそうなので、説明から削除しました。

参考

Kconv.guess の返り値は Encoding | nil と説明されていますが、nil を返すことはなさそうなので、説明から削除しました。

参考

* Kconv.guess は NKF.guess を呼び出している 
 
 
 
 * https://github.com/ruby/nkf/blob/5c0f28e86600d9f8f77c4ede10d5ef9fcbba4612/lib/kconv.rb#L142
* NKF.guess の返り値は Encoding と説明されている
    * https://docs.ruby-lang.org/ja/latest/method/NKF/m/guess.html
* NKF.guess の実装では必ず Encoding オブジェクトを返している
    * https://github.com/ruby/nkf/blob/5c0f28e86600d9f8f77c4ede10d5ef9fcbba4612/ext/nkf/nkf.c#L191
* 英語版 Kconv.guess のドキュメントでも encoding を返すと説明されている
    * https://docs.ruby-lang.org/en/3.3/Kconv.html#method-i-guess
@ohai
Copy link
Member

ohai commented Jun 8, 2024

PRありがとうございます。

これは調べると

  • Kconv::UNKNOWN は nil で推定ができない場合には nil を返すことが想定されていた可能性がある
  • しかし現代では(日本語の範囲で)一意に推定不可能でも適当に何か返す実装になっているようなので nil を返すことはない
    • 明記はされていないがUNKNOWNを返すルートがソースコード上は見付からなかった

という感じのようです。
というわけでこの変更自体は間違っていませんが,他にも何箇所か変更すべき所があるようです。

ただこの変更自体は妥当そうなので取り込みます。

@ohai ohai merged commit 3351348 into rurema:master Jun 9, 2024
8 checks passed
@ohai
Copy link
Member

ohai commented Jun 9, 2024

例:

docker run -it --rm rubylang/all-ruby env ALL_RUBY_SINCE=1.2.0 ./all-ruby -e 'require "kconv"; p(Kconv.guess("\xB7\xC7") == Kconv::UNKNOWN)'
ruby-1.2.1            true                                                                                                                             
ruby-1.2.2            true
ruby-1.2.3            false
ruby-1.2.4            true
...
ruby-1.3              true
ruby-1.3.1-990215     /tmp/rb6nMHdi:1: Uninitialized constant Kconv::UNKNOWN (NameError)
                  exit 1
ruby-1.3.1-990224     true
...
ruby-1.3.2-990408     true
ruby-1.3.2-990413     /tmp/rbscGThW:1:in `require': No such file to load -- kconv (LoadError)
                      	from /tmp/rbscGThW:1
                  exit 1
ruby-1.3.3-990430     true
...
ruby-1.3.5            true
ruby-1.3.6            /tmp/rb32FGwG:1:in `require': No such file to load -- kconv (LoadError)
                      	from /tmp/rb32FGwG:1
                  exit 1
ruby-1.3.7            true
...
ruby-1.8.2-preview3   true
ruby-1.8.2-preview4   false
...
ruby-3.3.0-preview1   false
ruby-3.3.0-preview2   -e:1: warning: kconv is found in nkf which will be not part of the default gems since Ruby 3.4.0
                      false
ruby-3.3.0-preview3   false
...
ruby-3.4.0-preview1   false

@tk0miya tk0miya deleted the patch-1 branch June 12, 2024 17:00
@tk0miya
Copy link
Contributor Author

tk0miya commented Jun 12, 2024

なるほど、かつては nil が返ることがあったんですね。勉強になります。
フォローありがとうございました

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