Project

General

Profile

バグ #1358

抽出条件「本文」がMessage Modelのto_sを使用している

Added by Shibafu Midorino 3 months ago. Updated about 1 month ago.

Status:
終了
Priority:
通常
Target version:
プラグイン名:
extract
ブランチ:
topic/1358-extract-description
クラッシュする:
No

Description

抽出プラグインの標準抽出条件「本文」では、以下のS式が使用されています。

(,compare (to_s message) ,(car args))

Message Modelの #to_s を使用しているように見えるのですが、これは現在のDiva Modelの規約とは合っていないのではないでしょうか?
たとえば Plugin::Twitter::Message では、互換実装が発動して140文字にトリムされた本文との比較になってしまう可能性があるかと思います。

(description message) のほうが良いのではないかと思いますが、いかがでしょうか。


Files

0001-description.patch (1.32 KB) 0001-description.patch Shibafu Midorino, 2019-07-15 19:35

Related issues

Related to バグ #1109: ツイートの詳細タブでURLリンクが多数ある場合に表示が切れてしまう終了Actions

Associated revisions

Revision 04e01613 (diff)
Added by Shibafu Midorino about 2 months ago

本文の抽出で description を参照する

refs #1358

History

#1

Updated by Izumi Tsutsui 3 months ago

  • Related to バグ #1109: ツイートの詳細タブでURLリンクが多数ある場合に表示が切れてしまう added
#2

Updated by toshi_a 初音 3 months ago

  • Status changed from 分類待ち to 実装待ち
  • Target version set to 3.9

これは確かに

#3

Updated by Shibafu Midorino 2 months ago

言いだしっぺがやるべきだなーと思ったのでパッチを作りました。

#4

Updated by toshi_a 初音 about 2 months ago

  • Status changed from パッチ適用待ち to レビュー待ち
  • Assignee set to Shibafu Midorino
  • ブランチ set to topic/1358-extract-description

3.9.1にmergeしたので、一応動作確認お願いします

#5

Updated by Shibafu Midorino about 2 months ago

自分の日常使用の範囲では、問題なく動作していることが確認できました。

マージ待ちにする前の心残りなのですが、抽出プラグインが扱うModelでdescriptionを実装していないことって、おそらく無いですよね...?

#6

Updated by toshi_a 初音 about 2 months ago

マージ待ちにする前の心残りなのですが、抽出プラグインが扱うModelでdescriptionを実装していないことって、おそらく無いですよね...?

descriptionがなければMessage Modelの要件を満たしておらずScoreが動かないので、必ず実装されています。

#7

Updated by Shibafu Midorino about 2 months ago

  • Status changed from レビュー待ち to マージ待ち

toshi_a 初音 さんは書きました:

descriptionがなければMessage Modelの要件を満たしておらずScoreが動かないので、必ず実装されています。

認識が合っていることが確認できて良かったです。ありがとうございます。
それでは、お手すきの際にマージをお願いします。

#8

Updated by toshi_a 初音 about 1 month ago

  • Status changed from マージ待ち to 終了

merged

Also available in: Atom PDF