プロジェクト

全般

プロフィール

バグ #920

message[:geo]の中身が変わったため、一部プラグインがクラッシュする

Osamu Koga10ヶ月前に追加. 7ヶ月前に更新.

ステータス:
終了
優先度:
通常
担当者:
対象バージョン:
開始日:
2016-10-23
期日:
進捗率:

0%

プラグイン名:
再現手順:

説明

おそらく 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>'

関係しているリビジョン

リビジョン 18a1602e (差分)
toshi_a 初音9ヶ月前に追加

MessageのDeprecatedなフィールド geo を削除 refs #920

リビジョン 1e7ba8fd (差分)
toshi_a 初音7ヶ月前に追加

MessageのDeprecatedなフィールド geo を削除 refs #920

履歴

#1 Osamu Koga10ヶ月前に更新

なんか情報少ないなと思ったので追記します。

geoフィールド(今リファレンス https://dev.twitter.com/overview/api/tweets を見たらDeprecatedだからcoordinatesを使えと書かれてましたが、構造は同じっぽいです)はtype:Stringcoordinates:[Float]の2つのフィールドを持つようになっています。typePoint以外が入ってるのは見たことがないですが、調べた訳でもないので確信はないです。これがcastの影響でHash#to_sの結果をgeoに格納するようになっており、Hashを前提としたコードが壊れています。

geoはIDで識別されるような性質のものではないので、Retriever::Modelとしては適当ではないと思います。よって、geo(やcoordinates)のフィールド定義をやめるか、もしくは生のJSONをパースしただけのオブジェクトを持つというフィールド定義ヘルパを作成するか、どちらかの対策を取る必要があります。

個人的には、そもそもコード上で定義されていないフィールドにアクセスされると読みにくいので、フィールドの存在が明確になる後者のアプローチが好きです。

#2 toshi_a 初音9ヶ月前に更新

  • 担当者toshi_a 初音 から Osamu Koga に変更

coordinatesですけど、geolocationを示す汎用的なModelを作ってそれを入れておくのが美しいかなあと思います。
こういうModelを使うようにしておくと、geolocationのModelを引数にopenイベントを発生させたら、外部の地図サービス(Google Map)をブラウザで開くとか、Google Mapプラグインみたいなやばいものが出てきた時に綺麗に連携出来て良さそうだと思います。一方で、Hashは何の制約も持たないので乱用されてしまうと他のプラグインがそれに対してIntentを定義できなくなってしまうためあまり賛成できないと今のところ思っています。

ただ、現在のところ通常のmikutterではgeolocationは一切使わないのでModelを用意しても無駄になりますね。メモリや処理時間に関しては微々たるものだと思うんですが。
要するに無駄にしなければモチベーションが上がるので、実際にそのModelに対するIntentとしてGoogle Mapのページを開くみたいなのを実装するとベターかなあと思いました。

いずれにせよgeoの定義は消してしまったほうが良さそうですね。これに関しては私がすぐに対応して、coordinatesについては意見ください。俺も考えます

#3 Osamu Koga9ヶ月前に更新

上のコメントを書いた時点ではRetriever::Modelの要件をちゃんと分かってなかったんですが、Intentのやつとか見ると、findbyidとかは必ずしもサポートしてる必要はないんですね。それならModelで良いと思います(DataSourceからは拾ってこれないのでRetrieverという名前を含んでるのは微妙ですが)。

素のmikutterにcoordinates対応を入れるなら、Intent込みでgeocodeプラグインをきちんと書き直して、本体に統合するとかもアリですかね。というかよく見るとplaceという別のフィールドもあって、地名が最初から入ってるっぽいです。中身から見てもTwitterの色々なところで表示されてる位置情報はこれをベースにしてそうなので、coordinatesよりこっちを使うべきな気がします。

#4 toshi_a 初音9ヶ月前に更新

ほんとだplaceとかありますね。言われてみれば見たことがあったような…。

geocodeプラグイン取り込むの良いですね。ただ、subpartsはあまり需要なさそうなのと、 #872 みたいなことをやって今までのsubpartsがどうなるか微妙なので、geocodeプラグインが必要とするもので、他のプラグインでも使えそうなものをmikutterに実装するみたいなのが良いと思います。

#5 toshi_a 初音7ヶ月前に更新

  • ステータス新規 から 終了 に変更

適用しました

他の形式にエクスポート: Atom PDF