This repository was archived by the owner on Oct 21, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 382
revise 80 chars restriction #78
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
うーん、これはダメ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ヒアドキュメントのほうが高機能なので、両方の記法を許可するか、ヒアドキュメントのみに統一するべきだと思います。
その書き方のメリットが大きいのはどんなときでしょうか?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
可読性が高く(と個人的に思ってます)、ヒアドキュメントよりもコード量が少ないので、短い文章であればこの方が早い場面もあるかな、と。
特に、ヘルパーでちょっとしたHTMLタグを出力する時とか。
統一するのであればヒアドキュメントという意見には賛成です。
これを禁止するかどうか、という点ですね。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
上記のふたつのコードを比べると、後者のほうが出力される HTMLの構造が分かりやすいと思いますが、どうでしょうか。改行された HTML タグを出力したいときは、ほぼ間違いなくネストされた HTML タグを出力するときだと思います。
つまり、最初の行も含めて全体的にインデントが可読性に影響してくるものはヒアドキュメントが良いかと思います。
こういう場合なら、あまり大差ないと思いますが。
あと、長い文字列の後でメソッドチェインしたいときは前者しかありませんね。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(つまり、どちらについても、禁止すると不都合が出る場合がある、と。)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
あー、メソッドチェーンの視点は抜けてた。やりたい場面は少ないだろうけど、ありそうだね。
ってことで、ヒアドキュメント推奨として、
等の場合は文字列リテラルでOK、ではどうですか?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
はい、それで良いと思います。修正しておきます。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
メソッドチェーンできるんじゃないか?と思ったらできた。
ってことで複数行の文字列リテラルは無しにしても良いかも。
これ踏まえてどう?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
思うにメソッドチェーンしたいのって、
html_safe位かなーと思ってて、その時はJが言うようにヒアドキュメントだろうし。全面廃止にできない理由は無くなったので、廃止しても良いかも、と思えてきました。