Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

MACRO : setdlgposの位置指定方法エンハンス (TeraTermウインドウを基準とした位置の指定等 #234 関連) #279

Closed
hkanou opened this issue Aug 20, 2024 · 22 comments
Labels
enhancement New feature or request
Milestone

Comments

@hkanou
Copy link
Contributor

hkanou commented Aug 20, 2024

TeraTermウインドウを基準とした位置の指定等が可能なよう、setdlgposによるダイアログの位置指定方法をエンハンスする。

#234 関連

画面に整列した、複数のTeraTermのウインドウで、個々のマクロがポップアップを出しても
どれがどのポップアップなのか全然わからないので、
ポップアップ(messagebox 等)を出力する方法に画面基準ではなく、
自分のウインドウ基準でポップアップを出力する方法はありませんか?(setdlgpos)

#234 (comment) を元に 仕様案(仮) で実装中。(2024/8/20時点)

@hkanou hkanou changed the title MACRO : setdlgposの MACRO : setdlgposの位置指定方法エンハンス (TeraTermウインドウを基準とした位置の指定等 https://github.com/TeraTermProject/teraterm/issues/234 関連) Aug 20, 2024
@hkanou hkanou changed the title MACRO : setdlgposの位置指定方法エンハンス (TeraTermウインドウを基準とした位置の指定等 https://github.com/TeraTermProject/teraterm/issues/234 関連) MACRO : setdlgposの位置指定方法エンハンス (TeraTermウインドウを基準とした位置の指定等 (#234 関連) Aug 20, 2024
@hkanou hkanou changed the title MACRO : setdlgposの位置指定方法エンハンス (TeraTermウインドウを基準とした位置の指定等 (#234 関連) MACRO : setdlgposの位置指定方法エンハンス (TeraTermウインドウを基準とした位置の指定等 #234 関連) Aug 20, 2024
@hkanou
Copy link
Contributor Author

hkanou commented Aug 20, 2024

Tera Term 5.3のリリースには間に合わない見込みです。

@hkanou
Copy link
Contributor Author

hkanou commented Sep 6, 2024

下記のPRを出させて頂きました。

  • 7d2ac0c setdlgpos のダイアログの位置指定オプション追加
  • 142e78c エラーダイアログのクローズボタン押下時の動作が Continue になっていたため、マクロ終了確認ダイアログを表示するよう修正
  • cab18aa CCtrlWindow::OnDpiChanged()の引数の脱字の修正

hkanou added a commit to hkanou/teratermproject_teraterm that referenced this issue Sep 23, 2024
hkanou added a commit to hkanou/teratermproject_teraterm that referenced this issue Sep 23, 2024
hkanou added a commit to hkanou/teratermproject_teraterm that referenced this issue Sep 23, 2024
hkanou added a commit to hkanou/teratermproject_teraterm that referenced this issue Sep 23, 2024
hkanou added a commit to hkanou/teratermproject_teraterm that referenced this issue Sep 23, 2024
hkanou added a commit to hkanou/teratermproject_teraterm that referenced this issue Sep 23, 2024
@nmaya
Copy link
Member

nmaya commented Sep 26, 2024

issue #277 の修正である PR #350 をマージしましたので、こちらの作業にかかれる状態になったでしょうか。
よろしくお願いいたします。

@hkanou
Copy link
Contributor Author

hkanou commented Sep 27, 2024

issue #277 の修正である PR #350 をマージしましたので、こちらの作業にかかれる状態になったでしょうか。

はい、こちらの PR の作成に着手いたします。
PR #350をマージ頂きありがとうございます。

@hkanou
Copy link
Contributor Author

hkanou commented Sep 27, 2024

本 issue「MACRO: setdlgposの位置指定方法のエンハンス #279 」の PR #356 をお送りさせて頂きました。

@nmaya
Copy link
Member

nmaya commented Sep 27, 2024

確認用にバイナリのビルドをかけました
https://ci.appveyor.com/project/teraterm/github-snapshot/builds/50692583

@hkanou
Copy link
Contributor Author

hkanou commented Sep 28, 2024

確認用にバイナリのビルドをかけました

ありがとうございます。
ビルド頂いたバイナリで動作に問題がないことを確認いたしました。

@nmaya
Copy link
Member

nmaya commented Sep 29, 2024

変更履歴

MACRO: setdlgpos ダイアログボックスの表示位置をディスプレイ/VT ウインドウ相対で指定できるようにした。

一言で説明するには多い変更内容だと思います。

  • MACRO: setdlgpos コマンドの表示位置の指定方法を拡張した。
    • 座標の基準として、ディスプレイの四隅と中央、VT ウインドウの四隅と中央を指定できるようにした。
    • 表示位置を補正するパラメータを指定できるようにした。

こんな感じでどうでしょうか?

setdlgpos コマンド

2つ目の例は今回の拡張を全部網羅するテストスクリプトのように見えます。実際そうでしょうし、ユーザもこれを動かすと動作を理解できると思います。
でもそれだけでなく1つめの例のほうに、今回拡張したときの代表的な例(「VTウィンドウの左上を基準に表示する」「ディスプレイの右上を基準に、下に10px、左に10px」のような、ユーザが使いたそうな具体的もの)を追加してもらえると、「拡張されたことでこんなことができるようになった」というのがわかりやすいと思います。

setdlgpos 0 0 6
messagebox 'top-left' 'VTWindow'

setdlgpos 0 0 2 (-10) 10
messagebox 'top-right offset 10' 'Display'

hkanou added a commit to hkanou/teratermproject_teraterm that referenced this issue Sep 29, 2024
@hkanou
Copy link
Contributor Author

hkanou commented Sep 29, 2024

不備に対し修正案まで記載頂き、大変ありがとうございます。
f7f4588 で修正させて頂きました。

変更履歴
一言で説明するには多い変更内容だと思います。
こんな感じでどうでしょうか?

はい、記載頂いた内容に修正いたしました。

setdlgpos コマンド

利用方法が伝わりやすくなりますね。
こちらも、1つめの例のほうに追加いたしました。

sempreff pushed a commit to sempreff/teraterm that referenced this issue Oct 10, 2024
@zmatsuo
Copy link
Member

zmatsuo commented Nov 13, 2024

listbox で追加したキーワード引き数で

base=vtwin
base=primary_monitor (デフォルト)

という案はどうでしょう。

(すみません、仕様のことで遅いレスですみません)

@zmatsuo
Copy link
Member

zmatsuo commented Nov 13, 2024

cab18aa の引数がないのは意図したもので、

仮引数を付けて
Visual Studio で警告レベルを上げる(/W4)と次の警告が出るようになります。

warning C4100: 'lp': 引数は関数の本体部で 1 度も参照されません。

コンパイラの警告を消す(引き数を使わないことを承知してることを知らせる)には
次の方法があります(もっとあるかも)。

  1. lp を消す(修正前にする, C++的な感じがします)
  2. 関数内で (void)lp; としてlpを参照する(特に何も作用がないコードなので最適化されて消える)

@nmaya
Copy link
Member

nmaya commented Nov 13, 2024

base=vtwin

このキーワード引数を追加すると、position 引数を10個から5個に減らせるというわけですか?

position 引数の後ろに、省略可能な offset x と offset y 引数があります。
キーワード引数は必ず位置引数の後ろになりますので、VTの中央に表示するにはこうなります。書くのが長くなりませんか?

; 今のPR場合
setdlgpos 0 0 10
x 無視される
y 無視される
position center of VT

; 提案の場合
setdlgpos 0 0 5 0 0 base=vtwin
x 無視される
y 無視される
position center
offset x 0
offset y 0
base vtwin

@nmaya
Copy link
Member

nmaya commented Nov 13, 2024

cab18aa の引数がないのは意図したもので、

わかりました。
このような誤解が生まれますから、変わったことをする場合は fall through のようにコメントを書いて他の人にわかるようにしていただけるでしょうか。

@zmatsuo
Copy link
Member

zmatsuo commented Nov 16, 2024

マジックナンバーが減らせると思ったのですがいまいちでした。
positionの10種類は全て起点ですね。

例えばこんな感じでしょうか

base=primary_monitor_top_left
base=primary_monitor_top_right
base=primary_monitor_bottom_left
base=primary_monitor_bottom_right
base=primary_monitor_center
base=vtwin_top_left
:
base=vtwin_bottom_center

結局base=positionですね。

書くのが長くなりませんか?

とても長くなってしまい今一つです。

PR よさそうです。

@zmatsuo
Copy link
Member

zmatsuo commented Nov 16, 2024

関数の定義時に仮引数の変数名を省略する書き方は普段しないですね。
こういう書き方があるという解説などないかなと探してみましたが
デフォルト引数、可変長引数ばかりでてきて見つけることができませんでした。

どこで見かけるかと考えると、やはりWindowsのメッセージ処理な気はします。

Windowsのメッセージを処理する関数なので、
WPARAM,LPARAMを引数、戻り値をLRESULTにしていますが、
変更したほうが普通な感じで扱えそうです。
添付のような修正を入れるはどうでしょうか?

OnDpiChanged.diff.txt

@hkanou
Copy link
Contributor Author

hkanou commented Nov 17, 2024

cab18aa の警告メッセージ気づいておりませんでした。
ご指摘頂きありがとうございます。

warning C4100: 'lp': 引数は関数の本体部で 1 度も参照されません。
コンパイラの警告を消す(引き数を使わないことを承知してることを知らせる)には
次の方法があります(もっとあるかも)。

C++では、使用しない仮引数の変数名を省略することが出来るのですね。
http://proger.blog10.fc2.com/blog-entry-50.html
http://blog.kmckk.com/archives/5711736.html

Google C++ Style Guideに下記の記載がありました。
Unused parameters that are obvious from context may be omitted:
Unused parameters that might not be obvious should comment out the variable name in the function definition:

どこで見かけるかと考えると、やはりWindowsのメッセージ処理な気はします。

DlgProcコールバック等では、仮引数を使用しないケースがありそうです。

Windowsのメッセージを処理する関数なので、
WPARAM,LPARAMを引数、戻り値をLRESULTにしていますが、
変更したほうが普通な感じで扱えそうです。
添付のような修正を入れるはどうでしょうか?

ありがとうございます、現在の処理に合わせると添付頂いた修正が良さそうです。
将来、OnDpiChangedでLPARAMを使用できるよう拡張性を持たせてコメントアウトしておくのも良いように思われました。

teraterm/ttpmacro/ttmmain.cpp 修正例
   703	LRESULT CCtrlWindow::OnDpiChanged(WPARAM wp, LPARAM /*lp*/) // 仮引数lpは未使用のためコメントアウト
   704	{
   705		const UINT new_dpi = LOWORD(wp);
   706		TTSetIcon(m_hInst, m_hWnd, MAKEINTRESOURCEW(IDI_TTMACRO), new_dpi);
   707		return TRUE;
   708	}

@nmaya
Copy link
Member

nmaya commented Nov 18, 2024

Windowsのメッセージを処理する関数なので、
WPARAM,LPARAMを引数、戻り値をLRESULTにしていますが、

このほうがそろっていてよさそうに思いましたが、ttpmacro\ttmmain.h を見るとそうでない関数もあるんですね。

コメントアウトしておくのも良いように思われました。

WPARAM,LPARAM のままで、使わないので警告が出るのを対策するなら、私もこのイメージでした。

@nmaya
Copy link
Member

nmaya commented Nov 18, 2024

macro/command/setdlgpos.html についてです。

setdlgpos 0 0 10
x 無視される
y 無視される

と書きましたが、「第3引数以降があると x, y が無視される」という認識であっているでしょうか。でしたら、x, y の引数の所にそのことが書かれていると分かりやすいかもしれません。
また、「第4引数がある場合は第5引数は省略不可」でしょうか?そうなら、それも書いて頂けるとわかりやすいかもしれません。

@hkanou
Copy link
Contributor Author

hkanou commented Nov 21, 2024

macro/command/setdlgpos.html についてです。

ご指摘頂きありがとうございます。

「第3引数以降があると x, y が無視される」という認識であっているでしょうか。

はい、第3引数で1~5(ディスプレイ基準)を指定した場合、x, yは使用されません。
第3引数で6~10(VT ウィンドウ基準)を指定し、MACRO が Tera Term とリンクされていない、もしくは、Tera Term が最小化状態または非表示状態の場合は x, y が使用されます。
第3引数で1~5(ディスプレイ基準)を指定した場合に x, yが使用されない旨、マニュアルを修正致します。

また、「第4引数がある場合は第5引数は省略不可」でしょうか?

第4引数と第5引数はセットで指定する必要があります。
こちらもマニュアルに加筆致します。

関数定義の仮引数の変数名の省略につきましては、混乱しないよう zmatsuo さん案で修正させて頂こうと思います。
後ほど、PRの修正を出させて頂きます。

hkanou added a commit to hkanou/teratermproject_teraterm that referenced this issue Nov 21, 2024
hkanou added a commit to hkanou/teratermproject_teraterm that referenced this issue Nov 21, 2024
…ermProject#348 の修正ミス対応 (仮引数で LPARAM lp を渡さないよう見直し TeraTermProject#279)
@hkanou
Copy link
Contributor Author

hkanou commented Nov 21, 2024

後ほど、PRの修正を出させて頂きます。

PR #356 を修正させて頂きました。

  1. setdlgposのマニュアル修正 8244313
  2. CCtrlWindow::OnDpiChanged() 仮引数 LPARAM lp を使用しないよう修正 8a4351a

zmatsuo added a commit that referenced this issue Nov 23, 2024
MACRO: setdlgposの位置指定方法のエンハンス #279 (TeraTermウインドウを基準とした位置の指定等 #234 関連)
@zmatsuo
Copy link
Member

zmatsuo commented Nov 23, 2024

ありがとうございます。先ほどマージしました。
ビルドしました。
https://ci.appveyor.com/project/teraterm/github-main/builds/51044083/artifacts

@hkanou
Copy link
Contributor Author

hkanou commented Nov 25, 2024

マージ頂き大変ありがとうございます。
ビルド頂いたバイナリで想定どおりの動作をすることを確認いたしました。
本issueをクローズさせて頂きます。

@hkanou hkanou closed this as completed Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants