taka-tuos / nanotodon

CLI mastodon client
MIT License
35 stars 6 forks source link

Display avatar icon and screen names etc. as one line header. #66

Open tsutsui opened 3 weeks ago

tsutsui commented 3 weeks ago

rc3 の後に入れる変更じゃないという話もありますが、雑に書いたら動いているっぽいのでとりあえず投げておきます。

要件

実装設計

要検討項目

いずれもAPI変更になるのでここではいじってません。

あと mltermmlterm-wsconsxterm -ti 382 しかテストしてないので、もう少しテストしたほうが良さそう。

taka-tuos commented 3 weeks ago

rc3 の後に入れる変更じゃないという話もありますが

0.4.0に及ぶ範囲ではないので全然大丈夫です(?) というのはさておき:


要件に関して

アバターアイコンサイズを「フォントの高さの2倍以上3倍未満でsixel単位の6ドットの倍数」にする (sayaka準拠)

これなんですが、自分もそんな感じの仕様(フォントサイズベースで可変)なのは設計・実装時点で認識していて、じゃあなんでやってなかったかといえば定数にして最適化をしたかったのというのがありまして(nanotodonはsayakaより速くあるべき、みたいな謎のライバル意識がある) まあmul指定できるようになっている時点で崩れているといえばそうなんですが、今は「ナイーブな実装」扱いなので、ゆくゆくヘッダに分離してSIXEL_MUL(仮称)をdefineしてからincludeして2種類の関数を定義して、とかにするのを考えていたのがあり。 "sixelの幅に関して" 動的計算するのはどうしようかな~みたいなのはあります(=ずらす文字数の方を計算するのはあり)

アバターアイコン表示の後、アイコンの次の行ではなくアイコンの右側に screen name, display name, date/time を表示する

ということ(?)なのでこっちに関しては賛成です(今の仕様はやるのをめんどくさがってそうしてるだけなので)

要検討項目に関して

print_picture() の mul 引数の定義がもともと複数の意味を一つに詰め込んでいて読んで解釈しづらい

ここはどうするにせよ(sixel幅を可変にする、しない、どちらにせよ)「アイコンと画像で別の wrapper関数にする」といった感じの実装にしたいと思っています

「アイコン幅相当文字数」を算出側の sixel.c から参照側の nanotodon.c にグローバル変数で渡しているのがダサい

前項を踏まえて「返り値で右移動量を出す」実装がいいかなあと思います

sayakaさん同様で「フォントサイズ指定オプション」もあったほうがよい?

あったところで特に困ることはないのでゆくゆくは実装したい(1.0.0に進みだしてからでよい)

sayakaさん同様で、そもそも「端末がsixel対応しているか」のチェックを入れたほうが良い?

(同上)

tsutsui commented 2 weeks ago

本業がアレだと趣味レビューするガッツが出ねえ、みたいな日々で遅くなりましたが

tsutsui commented 2 weeks ago

あー。雑にコミットしたら余分な差分も入ってしまいました

-    indent_icon = ((6 * mul_icon) + fontwidth - 1) / fontwidth;
+    indent_icon = ((6 * SIXEL_MUL_ICO) + fontwidth) / fontwidth;

これは「フォント幅が6の倍数だった場合フォントとアイコンが接触してしまう」ので「最低1〜6ドットは離す」の修正です。(元は 0〜5ドット離す、になっていた) で、こんなこと書かずに

indent_icon = (6 * SIXEL_MUL_ICO)  / fontwidth + 1;

でいい、という話があります。