Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: URLに関する置換処理を作成 #31

Merged
merged 26 commits into from
Oct 12, 2023

Conversation

book000
Copy link
Member

@book000 book000 commented Oct 9, 2023

以下の変更を含みます。
Kotlinなんもわからんなので、より良い書き方とかあればガンガンレビューください。

textreplacers ディレクトリの名前が微妙な気がしますが、単語の区切り文字を下手に入れるとダメそうな気がして…。

  • voicetext/TextProcessor.kt にあった置換処理を voicetext/textreplacers/*.kt に切り出し
    • 置換処理の基底クラスとして voicetext/textreplacers/BaseReplacer.kt を作成
  • URLに関する置換処理 voicetext/textreplacers/UrlReplacer.kt を新規作成
    • メッセージURL、チャンネルURL、イベントへのダイレクトURL、イベント招待リンク、ツイートURLを特殊サポートします
    • それ以外のURLについては、title要素を取得できたらその値を、そうでなければ拡張子があればそれを、なければ「Webページへのリンク」と読みます

@book000 book000 requested a review from yuuahp October 9, 2023 14:38
@book000 book000 marked this pull request as ready for review October 9, 2023 15:19
@book000 book000 marked this pull request as draft October 9, 2023 15:19
@book000 book000 marked this pull request as ready for review October 9, 2023 15:23
@book000
Copy link
Member Author

book000 commented Oct 9, 2023

CIが落ちていますが、これは #32 によるものです。

Copy link
Member

@yuuahp yuuahp left a comment

Choose a reason for hiding this comment

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

コード以外

  • models/response 以下は discord/ twitter/ などのパッケージに分かれているのに、models/original でなっていないのはなぜですか?
  • TwitterOembed じゃなくて TwitterOEmbed ではないでしょうか

Copy link
Member

@yuuahp yuuahp left a comment

Choose a reason for hiding this comment

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

追加

@book000
Copy link
Member Author

book000 commented Oct 10, 2023

models/response 以下は discord/ twitter/ などのパッケージに分かれているのに、models/original でなっていないのはなぜですか?

たしかに...。models/responses/*.kt とするか、discord/ twitter/ でパッケージを分けるか、どちらがお好みですか?

TwitterOembed じゃなくて TwitterOEmbed ではないでしょうか

oEmbed が正式名で、単語の一文字目を大文字にする...という方針で TwitterOEmbed にしましょうか。

book000 and others added 17 commits October 10, 2023 22:14
…rlReplacer.kt

Co-authored-by: yuuaHP <identity@yuua.dev>
…rlReplacer.kt

Co-authored-by: yuuaHP <identity@yuua.dev>
…rlReplacer.kt

Co-authored-by: yuuaHP <identity@yuua.dev>
…rlReplacer.kt

Co-authored-by: yuuaHP <identity@yuua.dev>
…rlReplacer.kt

Co-authored-by: yuuaHP <identity@yuua.dev>
…rlReplacer.kt

Co-authored-by: yuuaHP <identity@yuua.dev>
…rlReplacer.kt

Co-authored-by: yuuaHP <identity@yuua.dev>
…rlReplacer.kt

Co-authored-by: yuuaHP <identity@yuua.dev>
…rlReplacer.kt

Co-authored-by: yuuaHP <identity@yuua.dev>
Co-authored-by: yuuaHP <identity@yuua.dev>
Co-authored-by: yuuaHP <identity@yuua.dev>
Co-authored-by: yuuaHP <identity@yuua.dev>
…rlReplacer.kt

Co-authored-by: yuuaHP <identity@yuua.dev>
…rlReplacer.kt

Co-authored-by: yuuaHP <identity@yuua.dev>
@book000
Copy link
Member Author

book000 commented Oct 10, 2023

クソ大量にcommitしてしまったのでマージするときはsquashでお願いします(デフォルトでsquashでもいいと思うけど)

@book000 book000 requested a review from yuuahp October 10, 2023 13:27
@book000
Copy link
Member Author

book000 commented Oct 10, 2023

できれば、なにかしらフォーマッタできる設定が入ると嬉しいわね…。IDEAのフォーマット設定とか

@yuuahp
Copy link
Member

yuuahp commented Oct 11, 2023

できれば、なにかしらフォーマッタできる設定が入ると嬉しいわね…。IDEAのフォーマット設定とか

どういう?
EditorConfigみたいな?

@yuuahp
Copy link
Member

yuuahp commented Oct 11, 2023

models/response 以下は discord/ twitter/ などのパッケージに分かれているのに、models/original でなっていないのはなぜですか?

たしかに...。models/responses/*.kt とするか、discord/ twitter/ でパッケージを分けるか、どちらがお好みですか?

パッケージ名の方が密度低いと思うので分けたいです

@book000
Copy link
Member Author

book000 commented Oct 11, 2023

フォーマット設定はこの辺を想像しています
ja: https://pleiades.io/help/idea/configuring-code-style.html
en(official): https://www.jetbrains.com/help/idea/configuring-code-style.html

@book000
Copy link
Member Author

book000 commented Oct 11, 2023

気になってるところとして、この辺がunused argumentになるのが気になるんですよねえ。いつもコミット時に怒られる
image

@book000 book000 requested a review from yuuahp October 11, 2023 14:14
@yuuahp
Copy link
Member

yuuahp commented Oct 11, 2023

フォーマット設定はこの辺を想像しています ja: https://pleiades.io/help/idea/configuring-code-style.html en(official): https://www.jetbrains.com/help/idea/configuring-code-style.html

デフォルトじゃ足りないところがある感じですか?

@book000
Copy link
Member Author

book000 commented Oct 11, 2023

たとえぱ、指摘いただいたRegexの改行とかってIDEAの自動フォーマットをかけた上でアレなんですよね(デフォルトは一定文字数を漏れた行だけが改行される仕様だったはず)
なのでお好みな形に設定してもらえればレビュー負荷下がるかなあと

@yuuahp
Copy link
Member

yuuahp commented Oct 11, 2023

気になってるところとして、この辺がunused argumentになるのが気になるんですよねえ。いつもコミット時に怒られる image

諦めて処理を2種類に分けるか、アノテーションで無視するかだと思います

@yuuahp
Copy link
Member

yuuahp commented Oct 11, 2023

たとえぱ、指摘いただいたRegexの改行とかってIDEAの自動フォーマットをかけた上でアレなんですよね(デフォルトは一定文字数を漏れた行だけが改行される仕様だったはず) なのでお好みな形に設定してもらえればレビュー負荷下がるかなあと

やってみます

@book000
Copy link
Member Author

book000 commented Oct 11, 2023

unused argumentのところ、引数1つと2つ両方受け付けますよみたいな書き方ないですかね?

@book000
Copy link
Member Author

book000 commented Oct 11, 2023

アノテーションかけるとそのメソッド全体でかかる気がしててなんかちょっとですよねえ

@yuuahp
Copy link
Member

yuuahp commented Oct 11, 2023

この前 reflection で無理やり入れられないかと思って試してみましたが無理でした

@book000
Copy link
Member Author

book000 commented Oct 12, 2023

一応、guildIdのunused対応はこのPRの対象外とさせてください。

Copy link
Member

@yuuahp yuuahp left a comment

Choose a reason for hiding this comment

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

じゃあマージします

@yuuahp yuuahp merged commit aaeb1be into jaoafa:main Oct 12, 2023
@book000 book000 deleted the feat/webpage-read branch October 13, 2023 12:23
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WebページのURLが貼り付けられた際、そのページのタイトルを取得し読み上げる
2 participants