t-sin / inquisitor

Encoding/end-of-line detection and external-format abstraction for Common Lisp
34 stars 3 forks source link

Guess to the end #43

Closed cxxxr closed 7 years ago

cxxxr commented 7 years ago

guess-*でstreamを指定してバッファの最後まで走査した後に read-sequenceで走査を続けられるようにしました

streamがnilの場合はread-sequenceが出来ないのでそこで終わるようにしています

なぜstreamがnilの場合を用意したかというとdetect-external-formatでは streamは無しでベクタだけを渡せるからです

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-24.8%) to 64.475% when pulling 1cbaa2eb52cdd063e725a379b3f630c54f3fab54 on cxxxr:guess-to-the-end into 756c1d021f726539d2bfc8098082f5641cf9e321 on t-sin:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-3.04%) to 86.275% when pulling ab3b4a88c718af5bbf5e743cb6e3b6de7e297a18 on cxxxr:guess-to-the-end into 756c1d021f726539d2bfc8098082f5641cf9e321 on t-sin:master.

t-sin commented 7 years ago

レビュー機能なるものを利用していたら途中でsubmitしてしまった…。

全体的にナイスなんですが、ces-guess-from-vectorの引数については相談したく。

cxxxr commented 7 years ago

たしかにces-guess-from-vectorの引数はあまりよくありませんね vectorは渡さなくてもいいかもしれないです

t-sin commented 7 years ago

たとえばなんですが、vector側に寄せる場合、「文字コード判定の状態を多値で返しておいて、それをstreamからread-sequenceできなくなるまで使い続ける」という実装が考えられると思います。 (それをやろうと思っていたけど、気力がわかなかったです… :bowing_man:)

この後どうしましょうね。このPRはいったんmergeしてしまい、ぼくのほうでstreamかvectorかに寄せる実装をするという手もあります。ただし、気合い充填までに時間を要するかもですが…。

cxxxr commented 7 years ago

使う側としてはファイルの先頭*detecting-buffer-size*分だけしか見ないのは 間違った結果になる場合が多々あって不便なので、固定長のバッファでファイルを最後まで guessしてほしくてこのPRを出しました

ces-guess-from-vectorの引数が増えてしまったのはよくなかったですが 一応動くし裏側に隠されてるのでまあいいかという気持もあります

t_sinさんのvector側に寄せる案は素晴しいと思うのですがそれをうまく書くのは 僕は自身が無いです(主にiso-2022 escape sequenceがバッファ境界にある部分)

このPRをどうするかはどちらでもいいのですが、*detecting-buffer-size*の辺りは どうにかならないかなという気持ちです

cxxxr commented 7 years ago

iso-2022を特別扱いしてるところを無くせばvectorだけで処理できそうです define-dfaを見るとiso-2022-jpは定義されてますね

t-sin commented 7 years ago

使う側としてはファイルの先頭detecting-buffer-size分だけしか見ないのは間違った結果になる場合が多々あって不便なので

了解です。利用する側で不便なことは理解しました。vectorに寄せるほうでいろいろ試してみます。

ついでにもうひとつ聞かせてください。利用する際はやっぱりstreamよりvectorのほうがよかったりしますか?

t-sin commented 7 years ago

vector側に寄せる案は素晴しいと思うのですが

ぼくもコードを眺めて改めて自信ないなと感じてしまいましたが、ちょっと試行錯誤してみます。このプルリクは一旦寝かせさせてください。

cxxxr commented 7 years ago

利用する際はやっぱりstreamよりvectorのほうがよかったりしますか?

ファイルにしか使ってないので今の所はpathnameだけです

t-sin commented 7 years ago

@cxxxr こんな感じ https://github.com/t-sin/inquisitor/pull/44*detecting-buffer-size*は潰しました。

cxxxr commented 7 years ago

見ました。 guessの重複するコードが気になっていたんですが、この変更ですっきりしたのがとても良いです。

気になるところとしてiso-2022 escape sequenceがbufferの境界にあるときに判定できなさそうですが 大丈夫なんですかね

t-sin commented 7 years ago

ISO-2022系が判定できないの、大丈夫じゃないですね。これもちょっといじり回してみます。

cxxxr commented 7 years ago

とりあえずこのpullreqの範囲は終わったみたいなので閉じます ありがとうございました