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

リアクションを押して展開されたメッセージを消去する機能の追加 #24

Merged
merged 8 commits into from
Jun 20, 2021

Conversation

sizumita
Copy link
Contributor

リンク展開時に🗑リアクションをつけ、元メッセージの送信者とリンク送信者がリアクションをすると消せるようにしました。
メッセージリンクの引数を使って、外観を変えることなくデータを埋め込みました。

close #20

dispander/module.py Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@Huyu2239
Copy link
Contributor

使う側がDELETE_REACTION_EMOJIを指定できるようにできたらもっといいかと思います
✖リアクションにしたい場合とか

@sevenc-nanashi
Copy link
Member

sevenc-nanashi commented May 23, 2021

#22 実装するならここらへんになりそう

@sizumita
Copy link
Contributor Author

作ってみましたがどうですか

@Huyu2239
Copy link
Contributor

なるほど
LGTM.

@sizumita sizumita requested a review from sevenc-nanashi May 25, 2021 13:20
@sevenc-nanashi
Copy link
Member

sevenc-nanashi commented May 25, 2021

思ったんだけど環境変数に絵文字って入れられるの…?

@sizumita
Copy link
Contributor Author

sizumita commented May 25, 2021

試してから聞いて欲しかったんですが、Unicodeならなんでも入れられると思います...

export EMOJI_TEST="🇯🇵"echo $EMOJI_TEST
🇯🇵

*,
payload: Optional[discord.RawReactionActionEvent] = None,
reaction: Optional[discord.Reaction] = None,
user: Optional[discord.User] = None):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
user: Optional[discord.User] = None):
user: Optional[discord.User] = None) -> None:

引数に型ヒントがあって、返り値に型ヒントが無いのは何故?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

わかりにくかったところにつけてあるだけです
ライブラリの方針としてもつけてなさそうなのでいらないかなと

# when on_reaction_add event
if str(reaction.emoji) != DELETE_REACTION_EMOJI:
return
if user.id == bot.user.id:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

user が None だった時の検証は入れないんですか?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

それはエラーになるだけなのでいらないと思います

dispander/module.py Show resolved Hide resolved
dispander/module.py Show resolved Hide resolved
Copy link
Member

@1ntegrale9 1ntegrale9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

大変遅くなって申し訳ないですが、
動作確認まで済んでいるのでApproveします。

(ただしどこかでリファクタリングは必須)

@1ntegrale9 1ntegrale9 merged commit fc44365 into master Jun 20, 2021
@1ntegrale9 1ntegrale9 deleted the feature/20-remove-author branch June 20, 2021 15:03
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.

元メッセージ送信者とURL送信者が展開を削除できるように
6 participants