TeraTermProject / teraterm

Other
392 stars 33 forks source link

MACRO: listboxで表示されるダイアログのダブルクリック対応他 #154

Open hkanou opened 4 months ago

hkanou commented 4 months ago

ttpmacroのlistboxで表示されるダイアログの下記の要望につきまして、サンプル実装を紹介させていただきます。

パッチ https://github.com/hkanou/ttpmacro/tree/main/ttpmacro

参考にして頂ければ幸いです。

nmaya commented 3 months ago

ご提案ありがとうございます。

ダブルクリックによる項目選択に対応

よいと思います。

最小化/最大化ボタンの追加

私の環境では最小化せず、タスクバーを除いたデスクトップ範囲の左下端に「タイトルバーだけ」のような状態で表示されました。 154

→この拡張で予想される、さらなるリクエスト

アイコン画像の差し替え→listboxダイアログにアイコンをセット

このダイアログだけ?ほかのbox(messagebox, yesnobox, statusbox, inputbox, passwordbox)はどうするの?と思いました。全体として挙動が統一されていたほうがよいです。 また、"M" のアイコンかファイルのアイコンが考えられますが、私の感覚では "M" な気がします。マクロのメインウィンドウが "M" だからです。

ダイアログのサイズの拡大→ダイアログのサイズをマウス操作で広げられるようになった

最初のサイズより縮小できないようになっていて、よいと思います。

→この拡張で予想される、さらなるリクエスト

ダイアログのサイズの拡大→デフォルトのダイアログのサイズの拡大

選択肢の数や長さが短く、今までどおりのサイズでコンパクトに使いたい人にとっては、これは嬉しくありません。

hkanou commented 3 months ago

ご回答いただきありがとうございます。

最小化/最大化ボタンの追加 私の環境では最小化せず、タスクバーを除いたデスクトップ範囲の左下端に「タイトルバーだけ」のような状態で表示されました。

失礼いたしました、WS_EX_APPWINDOWを付け忘れていました。修正しました。

→この拡張で予想される、さらなるリクエスト 最大化状態でlistboxを出したい 最小化状態でlistboxを出したい

オプションで指定できるよう修正しました。

アイコン画像の差し替え→listboxダイアログにアイコンをセット このダイアログだけ?ほかのbox(messagebox, yesnobox, statusbox, inputbox, passwordbox)はどうするの?と思いました。全体として挙動が統一されていたほうがよいです。

他のダイアログについてもアイコン画像の差し替えを行いました。

また、"M" のアイコンかファイルのアイコンが考えられますが、私の感覚では "M" な気がします。マクロのメインウィンドウが "M" だからです。

メインウィンドウのアイコン"M"に揃えました。

→この拡張で予想される、さらなるリクエスト 寸法を指定してlistboxを出したい ダイアログのサイズの拡大→デフォルトのダイアログのサイズの拡大 選択肢の数や長さが短く、今までどおりのサイズでコンパクトに使いたい人にとっては、これは嬉しくありません。

ダイアログのサイズをパラメタで指定できるよう修正しました。

修正版の格納先は以下のとおりです。 使用方法:http://htmlpreview.github.io/?https://github.com/hkanou/ttpmacro/blob/main/ttpmacro/doc/listbox.html パッチ、修正ソース:https://github.com/hkanou/ttpmacro/tree/main/ttpmacro バイナリ:https://github.com/hkanou/ttpmacro/tree/main/ttpmacro/Release

zmatsuo commented 1 month ago

もしよければ Pull Request にしていただけないでしょうか。@hkanou

いただいた改良がリポジトリに残りますし、 修正の確認(diff)などがgitを使っていると比較的簡単にできます。

まずは、setpassword、getpassword暗号化機能部分などどうでしょうか https://github.com/hkanou/ttpmacro/tree/main/ttpmacro2

hkanou commented 1 month ago

ありがとうございます。 setpassword、getpassword暗号化機能をPull Requestさせていただきました。

nmaya commented 1 month ago

タイトルと異なる内容のPRなので、別のissueにしていただけるでしょうか?

hkanou commented 1 month ago

考慮が足りておりませんでした、別issueで起票させて頂きます。 https://github.com/TeraTermProject/teraterm/issues/213#issue-2293160167

hkanou commented 2 days ago

ソースをマージ頂きまして大変ありがとうございます。 しばらく様子を見て、特に問題が無いようでしたらクローズさせて頂こうと思います。

nmaya commented 2 days ago

(1) マクロコマンドの引数について 今までの「追加コマンドは引数を順番に追加し、何番目の引数は固定の何かを表す」という型から外れています。 このコマンドは新規のコマンドではなく、selected までが既存の引数に、それ以降の引数が新しい形になっています。

167 で最終結論が出ているように思いませんが、マージされたということは「新しいマクロコマンドについて、新しい引数の受け取り方がプロジェクトとして受け入れられた」「既存のマクロコマンドに新旧の引数の受け取り方が混在していても問題ない(既存の引数を変えては既存のマクロが動かないので、既存のマクロコマンドは今までの受け取り方のみにするか、途中からは許して混在を許可するかの2択)」「マクロユーザに対して、この変化を十分に説明するドキュメントになっている」と判断されたということですか?>@zmatsuo

(2) (1)が問題ないという前提で、「dclick以降の引数だけが順不同である」ということがわかるようにドキュメントを書いて頂けるでしょうか。

(3) (1)が問題ないという前提で、マクロのドキュメントで「固定文字列」と言う言葉は使いません。文字列変数でも文字列リテラルでもよい場合は「文字列」と書きます。

(4) ドキュメントに省略時の動作を書いて頂けるでしょうか。私のように全くマクロを使わないユーザだと、ドキュメントだけでは何が普通の動きなのかわかりません。

(5) "dclick" という綴りに違和感があります。VBA では OnDblClick ですし、HTMLでも dblclick イベントになっているので、"dblclick" としてはどうでしょうか?

hkanou commented 2 days ago

混乱を招いてしまい大変申し訳ございません。

今までの「追加コマンドは引数を順番に追加し、何番目の引数は固定の何かを表す」という型から外れています。 このコマンドは新規のコマンドではなく、selected までが既存の引数に、それ以降の引数が新しい形になっています。

ご指摘のとおりです。 引数の指定方法を変更しており、マクロユーザに対して従来とは異なる旨説明が必要でした。 本提案は "既存のマクロコマンドに新旧の引数の受け取り方が混在していても問題ない(途中からは許して混在を許可する)"になります。既存のマクロへの影響が無く新オプションの指定もし易いと思っておりますが、考慮漏れ等ございましたら修正させていただきます。 (2)~(5)につきましても対応させて頂きますので、どうぞよろしくお願いいたします。

zmatsuo commented 1 day ago

リリースに向けてプルリクエストを処理しなきゃと思っていました。

167 で最終結論が出ているように思いません

そうですね。 失礼しました。

このissueについて、 各々のついて私のコメントです。

新しいマクロコマンドについて、新しい引数の受け取り方がプロジェクトとして受け入れられた

わかりやすいかなと思います。

たとえば、logopen の引き数をみると、すぐにわかりにくい感じです。

私が分かりやすい/にくいと思うことと、 プロジェクトとして受け入れられたとすることは別ですね。

私もマクロを使っていないのですが拡張をしたい内容は理解できますし、 listboxの機能拡張,引き数の追加はとてもよさそうだと思います。

引き数の仕様についてもうすこし考えたほうがよさそうでしょうか?

既存のマクロコマンドに新旧の引数の受け取り方が混在していても問題ない(既存の引数を変えては既存のマクロが動かないので、既存のマクロコマンドは今までの受け取り方のみにするか、途中からは許して混在を許可するかの2択)

手もとでテストしてみて従来との互換性があって、拡張/仕様変更も容易そうです。 私は良いとおもいました。

マクロユーザに対して、この変化を十分に説明するドキュメントになっている

私はPRのドキュメントをみて使い方がわかりました。 ソースも見ているからでしょうね。

引き数の仕様の変化は分かりにくいでしょうか。 従来の引数に対して「名前付き引き数(仮)」と名前を付けて、

listbox <message> <title> <string array> [named paraeters]

 :
named paraeters

  minimize=on

みたいな感じにすれば分かりやすいでしょうか・・。

5.3devで、このissueの拡張の前にlistboxをリサイズ可能にしています。 デフォルトはリサイズ不可で resizeable=on でリサイズ可能としたほうが よいかな思いました。

minmaxbutton=on という書き方は、 Python(という言語です)の関数呼び出しの引数のような感じで C(言語ですね:-)の関数の引数順序が固定しているより 自由度があってよさそうです。 scanf() でsize=<N1xN2>のサイズを取ってきているところがありますが、 今後もっと複雑なパースがあってもoniguruma使えば何とかなりそうです。 (いや width=w height=h でシンプルさを保ったほうがよいか?)

hkanou commented 11 hours ago

5.3devで、このissueの拡張の前にlistboxをリサイズ可能にしています。 デフォルトはリサイズ不可で resizeable=on でリサイズ可能としたほうが よいかな思いました。

5.3devのダイアログのリサイズ可能対応は、私には違和感は無くresizeable=onがデフォルトで良いように感じています。(リサイズしなければ従来の挙動と変わりがないため)

scanf() でsize=のサイズを取ってきているところがありますが、 今後もっと複雑なパースがあってもoniguruma使えば何とかなりそうです。 (いや width=w height=h でシンプルさを保ったほうがよいか?)

無意識にsize=としていました。 key=value形式での直交性という意味では、width=w height=h の方が良さそうです...