Skip to content

Conversation

qryxip
Copy link
Member

@qryxip qryxip commented May 5, 2025

内容

撲滅可能なUUOC (Useless use of cat)を撲滅する。以下のcatの利用は放置。

  • cat <<EOF | while read -r line; do; …
    • catを抜こうとすると、ヒアドキュメントを後ろの方に持って来なければならないため。
    • arrayに格納してからforループにしたい気持ちがあるが、とりあえずこのまま
  • <(cat <<< "$variable")

CIによるチェックは入れない。というより簡単にできるかがわからない。VOICEVOX organizationにおける"uuoc"の検索結果に引っ掛かるよう、このPRをマージするにせよクローズするにせよ判例を作るのが目的。

関連 Issue

その他

@qryxip qryxip requested a review from Hiroshiba May 5, 2025 04:02
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

LGTM!!

なのですが、一点だけ確認まで!
bashに不慣れなコントリビューターにとって読みづらい(=貢献しづらい)コードになりますが、その辺りの方針は考えていたりしますか? 👀

個人的には、この変更は理想なshellに近づく一方で、不慣れな人にわかりづらくなるという一長一短かもと感じました。
記号はググれないので、何をしているのか調べることすらできないこともありえるかなと。

ここのトレードオフをどう捉えるか、コアのメンテナとして方針、もしくはその途中の考えを言語化しておくと良いのかなーと!
(たぶん正解はなさそう。)

@qryxip
Copy link
Member Author

qryxip commented May 5, 2025

理由を挙げるとするならこんな感じでしょうか。

  • <, <<, <<<に対して統一性が取れる (e.g. do-something < ./input.txt > ./output.txt)
  • 「コマンド」を介さないため、認知負荷の軽減をはかれる(これについては大いに人によりそう)
  • 検索エンジンは基本的に記号を受け付けないが、今はLLMがある。ChatGPT 4oでもおそらくは適切に回答可能

まあただcatにする強い理由があればその限りではないと思います。SC2002にしてもすべてのケースで適切ではないと認めてはいますし。

@Hiroshiba
Copy link
Member

Hiroshiba commented May 5, 2025

あ、言語化しておいた方がいいのは利点ではなく、利点と欠点のバランスをどうするのかというトレードオフの観点かなと・・・!

それとも欠点はないという感じですかね・・・?

あるいは、そもそもこのPRによって何を解決したいのかがわからないという話なのかもです!

追記:もしかしてこれは課題提起・・・?(Issue・・・?)

@qryxip
Copy link
Member Author

qryxip commented May 8, 2025

あ、言語化しておいた方がいいのは利点ではなく、利点と欠点のバランスをどうするのかというトレードオフの観点かなと・・・!

それとも欠点はないという感じですかね・・・?

Bashを学んだ方法によってはcat, >, >>, |しか知らないこともありえて、そういった人が$(< file)という式を目の前にして何を思うか、というのはありそうです。ただそういった人でも、(LLMがあるとは言いましたが他にも)次の手段があると思います。

  1. Google検索では実は$(<は検索可能なので、Bashで$(cat foo.txt)していませんか?$(< foo.txt)しよう - Lambdaカクテルといった記事に辿り着ける
  2. man bashを見る。"Command Substitution"の節で$(< file)について書かれているので、そこに辿り着ければ判明する。「正しい」アプローチはこれなはずで、「ググるよりも前にmanを見ろ」という教育を受けた人であればこの選択肢が取れるはず。

追記:もしかしてこれは課題提起・・・?(Issue・・・?)

ですね。ただissueと比べてPRの場合、強い理由が無い限り棄却しにくいというのも確かにあると思います。今回はその視点が抜けていました。

課題定義の理由としては、いつかどこかのPRで$(< file)という式が登場した際に「わかりにくいので$(cat file)にしませんか」「いえシェルスクリプトの一般的な慣習だとUUOCは避けるべきでは」という議論になることがありうるかなと予感したからですね。ちょうど VOICEVOX/voicevox_engine#1646 みたいな話がありましたし、せっかくなので単体の話として議論しておくのも悪くないかなと思った次第です。

@Hiroshiba
Copy link
Member

Hiroshiba commented May 8, 2025

なるほどです! とりあえずどうすべきかは置いといて、意見をここに書いておくと良さそうですかね。

それとも欠点はないという感じですかね・・・?

↑これはどうでしょう?
結局物事にはよほどのことがない限り利点と欠点があるはずで、 @qryxip さんの意見として「なんとかなりうる」というのは分かるのですが、そもそも欠点にどういうものがあるのか考えると良いのかなと思っています!
欠点が思いつかないとかであれば僕から考えてみようと思います。

あとは利点を考えて、そのバランスで決めるのが良いと思います!
利点と欠点のどちらが大きいかが自明じゃないのが難しいポイントですねぇ。。。
もしかしたらこの観点だけでは決められなくて、「CI用のスクリプトはどうあるべきか」を考えて、そこから答えを導くことになるのかもしれない。(大変だけど有意義)

@qryxip
Copy link
Member Author

qryxip commented May 9, 2025

Bashの<を知らない人がいるというのが欠点の一つで、それについての所感を述べるというのが私の意図でした。言い方が悪かったのですが、LLMもGoogle検索もmanも90%程度は「なんとかなりうる」けど別に100%だとも思っていませんでした。

欠点が思いつかないとかであれば僕から考えてみようと思います。

上記以外、つまりBashの<をちゃんと知っている人にとってはどうかという観点だと思い付かないので、お願いできたらと思っています。

もしかしたらこの観点だけでは決められなくて、「CI用のスクリプトはどうあるべきか」を考えて、そこから答えを導くことになるのかもしれない。(大変だけど有意義)

たぶんそうで、議論の場を移すとしたら VOICEVOX/voicevox_project#70 の一部として議論することになりそうかなと思っています。

ただまあ、私としてはぶっちゃけ最悪$(cat file)$(< file)を混在させるという「結論」に至ってもよいかなと思っています。「$(cat file)にしましょう」「$(< file)にしましょう」といったレビューは行わないよう心掛け、このPRのようなリファクタ提案も原則拒否するような形で。

@Hiroshiba
Copy link
Member

欠点はBashの<を知らない人がいるから導かれること一式、って感じかなーと!
例えばこんな感じ?

  • 書いてることがわからないので意図を読み間違う・誤認する
  • 書いていることがわからないのでコミットを諦める
  • 正直分かりづらい概念なので、調べたところで使い方を誤りうる

あと調べても<は標準入力だとわかるかもだけど、$(< ファイル名)が何かはさらに分かりづらいかも。
結局bashがややこしいという話になりそうですが。

まあbashの<に限らず、|だとか$()だとか、github actionの${{}}だとか、ややこしいのはいっぱいあってそのうちの1つかなぁ。


たぶんそうで、議論の場を移すとしたら VOICEVOX/voicevox_project#70 の一部として議論することになりそうかなと思っています。

話していて思いましたが、このややこしさはどちらかというとシェルスクリプトをどう扱うかに起因していると感じました!
もちろんGithub Actions内のシェルスクリプトをどう書くのかにも関わりますが、主軸はシェルスクリプトの方かなみたいな。

ちょこちょこと考えていく必要がある気はするから、issue作るのはアリ・・・かも・・・?

私としてはぶっちゃけ最悪$(cat file)$(< file)を混在させるという「結論」に至ってもよいかなと思っています

リポジトリメンテナの判断で、片方に寄せた方が良さそうだとはを感じます!
レビューで言わないのは賛成で、メンテナ側が勝手に変えちゃうぐらいしちゃっても良いかな~とか。(コミッター側は楽なはず。)

@qryxip
Copy link
Member Author

qryxip commented May 10, 2025

話していて思いましたが、このややこしさはどちらかというとシェルスクリプトをどう扱うかに起因していると感じました!
もちろんGithub Actions内のシェルスクリプトをどう書くのかにも関わりますが、主軸はシェルスクリプトの方かなみたいな。

シェルスクリプトについては話し合った方がよいことが他にも結構色々ありそうなので、主軸をそれにするのはありだと思います。

例えば、環境変数じゃない変数は全部小文字にしてSC2154とかの恩恵を受けようみたいな話とかも前々からしてみたいと思っていました。
(ちなみにこれについてはtLDPのBash Guide for BeginnersGoogleのShell Style Guideが論拠になりそう)

VOICEVOX/voicevox_project#70 のsub-issueとして「シェルスクリプトをどうするか」を作り、そのsub-issueとしてUUoCをどうするか(今このPRで話している内容)とかシェル変数の大文字小文字をどうするかみたいな話題をぶら下げるというのはどうでしょうか?

[追記] VOICEVOX/voicevox_project#70 はそんなにシェルスクリプトに焦点を当ててないので、親子関係であるべきかというと確かに微妙な気がしますね。ただシェルスクリプトはほぼほぼGHAから呼ばれるので、GHAのworkflowの一部とも言えなくもないような気もします。

@Hiroshiba
Copy link
Member

VOICEVOX/voicevox_project#70 のsub-issueとして「シェルスクリプトをどうするか」を作り

独立させた方が良いかなと思いました!
例えばworkflowファイルの取り決めが決まっても、まだシェルスクリプトの取り決めが決まってない場合、issueがcloseできないのは変かなーと思いました。
シェルスクリプトの話をする時にworkflowのことを考慮するのは大事だと思うけど、親子関係というわけではなさそう?

そのsub-issueとしてUUoCをどうするか(今このPRで話している内容)とかシェル変数の大文字小文字をどうするかみたいな話題をぶら下げるというのはどうでしょうか?

普通ならそれでも良さそうなんですけど、sub-issueの1つ1つが他のvoicevox_projectissueの粒度とずれてしまい、プロジェクトを網羅的に見れなくなっちゃって不便かもです。。。 🙇
代替案は・・・・・Github Discussionですかね。。。それか1つのissueで頑張るか。。。

ま~ギリギリ1つのissueでなんとかなるかも!!Discussion使ってみるのもありかもです。どちらでも!!
議題それぞれに固有の番号やタグとかを振れると楽なんですけどねぇ・・・。

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.

2 participants