バグ #920
closed
message[:geo]の中身が変わったため、一部プラグインがクラッシュする
Description
おそらく 3ab6ff の影響で、message[:geo]の中身がHashからStringに変わってしまったため、mikutter-geocode がクラッシュします。
/home/osak/.mikutter/plugin/geocode/geocode.rb:61:in `[]': no implicit conversion of Symbol into Integer (TypeError)
from /home/osak/.mikutter/plugin/geocode/geocode.rb:61:in `locations'
from /home/osak/app/mikutter/core/lib/uithreadonly.rb:22:in `block (2 levels) in singleton class'
from /home/osak/.mikutter/plugin/geocode/geocode.rb:50:in `body'
from /home/osak/app/mikutter/core/lib/uithreadonly.rb:22:in `block (2 levels) in singleton class'
from /home/osak/.mikutter/plugin/geocode/geocode.rb:39:in `height'
from /home/osak/app/mikutter/core/lib/uithreadonly.rb:22:in `block (2 levels) in singleton class'
from /home/osak/.gem/gems/memoist-0.14.0/lib/memoist.rb:165:in `height'
from /home/osak/app/mikutter/core/mui/cairo_sub_parts_helper.rb:41:in `block in _subparts_height'
from /home/osak/app/mikutter/core/mui/cairo_sub_parts_helper.rb:41:in `each'
from /home/osak/app/mikutter/core/mui/cairo_sub_parts_helper.rb:41:in `inject'
from /home/osak/app/mikutter/core/mui/cairo_sub_parts_helper.rb:41:in `_subparts_height'
from /home/osak/app/mikutter/core/mui/cairo_sub_parts_helper.rb:34:in `subparts_height'
from /home/osak/app/mikutter/core/lib/uithreadonly.rb:22:in `block (2 levels) in singleton class'
from /home/osak/app/mikutter/core/mui/cairo_coordinate_module.rb:46:in `height'
from /home/osak/app/mikutter/core/lib/uithreadonly.rb:22:in `block (2 levels) in singleton class'
from /home/osak/app/mikutter/core/mui/cairo_cell_renderer_message.rb:145:in `render_message'
from /home/osak/app/mikutter/core/mui/cairo_cell_renderer_message.rb:128:in `uri='
from /home/osak/app/mikutter/core/plugin/gtk/mainloop.rb:10:in `main'
from /home/osak/app/mikutter/core/plugin/gtk/mainloop.rb:10:in `mainloop'
from mikutter.rb:63:in `boot!'
from mikutter.rb:92:in `<main>'
Updated by Osamu Koga over 8 years ago
なんか情報少ないなと思ったので追記します。
geoフィールド(今リファレンス https://dev.twitter.com/overview/api/tweets を見たらDeprecatedだからcoordinatesを使えと書かれてましたが、構造は同じっぽいです）はtype:Stringとcoordinates:[Float]の2つのフィールドを持つようになっています。typeはPoint以外が入ってるのは見たことがないですが、調べた訳でもないので確信はないです。これがcastの影響でHash#to_sの結果をgeoに格納するようになっており、Hashを前提としたコードが壊れています。
geoはIDで識別されるような性質のものではないので、Retriever::Modelとしては適当ではないと思います。よって、geo（やcoordinates）のフィールド定義をやめるか、もしくは生のJSONをパースしただけのオブジェクトを持つというフィールド定義ヘルパを作成するか、どちらかの対策を取る必要があります。
個人的には、そもそもコード上で定義されていないフィールドにアクセスされると読みにくいので、フィールドの存在が明確になる後者のアプローチが好きです。
Updated by toshi_a 初音 about 8 years ago
- Assignee changed from toshi_a 初音 to Osamu Koga
coordinatesですけど、geolocationを示す汎用的なModelを作ってそれを入れておくのが美しいかなあと思います。
こういうModelを使うようにしておくと、geolocationのModelを引数にopenイベントを発生させたら、外部の地図サービス（Google Map）をブラウザで開くとか、Google Mapプラグインみたいなやばいものが出てきた時に綺麗に連携出来て良さそうだと思います。一方で、Hashは何の制約も持たないので乱用されてしまうと他のプラグインがそれに対してIntentを定義できなくなってしまうためあまり賛成できないと今のところ思っています。
ただ、現在のところ通常のmikutterではgeolocationは一切使わないのでModelを用意しても無駄になりますね。メモリや処理時間に関しては微々たるものだと思うんですが。
要するに無駄にしなければモチベーションが上がるので、実際にそのModelに対するIntentとしてGoogle Mapのページを開くみたいなのを実装するとベターかなあと思いました。
いずれにせよgeoの定義は消してしまったほうが良さそうですね。これに関しては私がすぐに対応して、coordinatesについては意見ください。俺も考えます
Updated by Osamu Koga about 8 years ago
上のコメントを書いた時点ではRetriever::Modelの要件をちゃんと分かってなかったんですが、Intentのやつとか見ると、findbyidとかは必ずしもサポートしてる必要はないんですね。それならModelで良いと思います（DataSourceからは拾ってこれないのでRetrieverという名前を含んでるのは微妙ですが）。
素のmikutterにcoordinates対応を入れるなら、Intent込みでgeocodeプラグインをきちんと書き直して、本体に統合するとかもアリですかね。というかよく見るとplaceという別のフィールドもあって、地名が最初から入ってるっぽいです。中身から見てもTwitterの色々なところで表示されてる位置情報はこれをベースにしてそうなので、coordinatesよりこっちを使うべきな気がします。
Updated by toshi_a 初音 about 8 years ago
ほんとだplaceとかありますね。言われてみれば見たことがあったような…。
geocodeプラグイン取り込むの良いですね。ただ、subpartsはあまり需要なさそうなのと、 #872 みたいなことをやって今までのsubpartsがどうなるか微妙なので、geocodeプラグインが必要とするもので、他のプラグインでも使えそうなものをmikutterに実装するみたいなのが良いと思います。