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

[WIP] リンクテキストの処理に SysLink を使う #570

Closed
wants to merge 4 commits into from

Conversation

m-tmatma
Copy link
Member

#563 (comment)

で SysLink という component が参照されていたので、SysLink を使って URL のリンクを
実装してみました。

制限事項

  • 不要なコードの削除はしていません。
  • WM_NOTIFY での処理 で hwndFrom はチェックしていません。

参考情報

@m-tmatma
Copy link
Member Author

SysLink という簡単に使えるコンポーネントがあるので既存のコードを
メンテするのではなく置き換えたらいいのではないかと思いました。

実装したので使ってみて感想お願いします。

@m-tmatma m-tmatma mentioned this pull request Oct 20, 2018
@berryzplus
Copy link
Contributor

これはこれで、今後使いたい人が使えるように、の目的でやっといたほうがいいと思ってます。
バージョン情報ダイアログじゃなくてもURLリンク付けたい需要はあると思うので。

で、バージョン情報ダイアログの CUrlWnd を置き換えたいかというと、いまのところ NO です。
なんか問題があったとしても、改善できるとこを保守改善していったらいいような気がしています。

あと、Sys Link使うならコモンコントロールの初期化関数に「sys link使うよ宣言」をしてやるのが作法なはずです。初期化しなかった場合に動くかどうかはやったことないです。
https://docs.microsoft.com/en-us/windows/desktop/api/commctrl/nf-commctrl-initcommoncontrolsex

@m-tmatma
Copy link
Member Author

で、バージョン情報ダイアログの CUrlWnd を置き換えたいかというと、いまのところ NO です。

なぜ?

@ds14050
Copy link
Contributor

ds14050 commented Oct 20, 2018

昨日は見つけられなかった SysLink の要件を見つけました。

アップデートされた Windows Server 2003 か Windows Vista 以上が必要なんでしょうか。バージョン要件が「6.0 or later」なのかどうかも不確かです。

@berryzplus
Copy link
Contributor

で、バージョン情報ダイアログの CUrlWnd を置き換えたいかというと、いまのところ NO です。

なぜ?

#563 に書いたとおりです。

理屈で考えれば廃止して置き換えるのが当然なんです。
でも、このコントロールって目立った不具合もなく10年近くちゃんと動いてますよね?

ユーザ設定をガン無視して描画されるリンクテキストとか、
根拠不明なハイライト背景色とか、
独自コントロールなのにクリック時のイベント制御が親ウインドウとか、
中身を見れば突っ込みどころはたくさんあるんですが、ちゃんと動いてる。

害がないなら「味」として残してもいいんじゃないかな、と最近は思うようになりました。

正しい/正しくないの判断だけで置換をすすめていくとしたら、
真っ先に対応すべきなクラスは CNativeW です。
CNativeW の影響範囲はとてつもなく広いので、引き合いに出すのは変かも知れません。
でも、害がないならできるだけ残していきたい、という考えは伝わるかと思います。

@m-tmatma
Copy link
Member Author

でも、このコントロールって目立った不具合もなく10年近くちゃんと動いてますよね?

単にあまり使われていなかっただけのようにも思います。

害がないなら「味」として残してもいいんじゃないかな、と最近は思うようになりました。

今回の件がなければ害がないといえます。
すくなくともメンテナンスしにくいという弊害はあると思います。

@beru
Copy link
Contributor

beru commented Oct 20, 2018

昨日は見つけられなかった SysLink の要件を見つけました。

これに関していうと既に下記の記載があるので問題無いと思います。

#if defined _M_IX86
#pragma comment(linker,"/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='x86' publicKeyToken='6595b64144ccf1df' language='*'\"")
#elif defined _M_IA64
#pragma comment(linker,"/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='ia64' publicKeyToken='6595b64144ccf1df' language='*'\"")
#elif defined _M_X64
#pragma comment(linker,"/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='amd64' publicKeyToken='6595b64144ccf1df' language='*'\"")
#else
#pragma comment(linker,"/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='*' publicKeyToken='6595b64144ccf1df' language='*'\"")
#endif
#endif

アップデートされた Windows Server 2003 か Windows Vista 以上が必要なんでしょうか。バージョン要件が「6.0 or later」なのかどうかも不確かです。

Windows Vista, Windows Server 2008, and Windows 7 には Version 6.10 の ComCtl32.dll が付いてきているようなので問題無いと思います。

Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

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

リンク機能はありますがマウスカーソルを合わせてもURLが黄色に変化しないですね。
色変化が必要かというと微妙ですが…。

Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

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

言語設定が English だと何故か表示がおかしくなります。
image

@berryzplus
Copy link
Contributor

なんでそうなるか分かりませんでした・・・。
っていうか、英語ダイアログ文字デカいっすね。
前にそういう別件issueがあった気がしますけれども。

@AppVeyorBot
Copy link

Build sakura 1.0.2664 completed (commit b4036270c7 by @m-tmatma)

@m-tmatma
Copy link
Member Author

言語設定が English だと何故か表示がおかしくなります。

修正しました。

image

@berryzplus
Copy link
Contributor

サクラエディタの WM_NOTIFY ハンドラの実装は「正しくない」と思っております。

WM_NOTIFY というのは「システムからの通知メッセージ」の1種です。
Windowsには数百種類のシステムメッセージが定義されていて、基本的なメッセージの仕様は公開されています。

WM_NOTIFYメッセージをハンドルするには、マニュアルに従ってメッセージパラメータを解析して必要なデータを取得する必要があります。

マニュアル ⇒ https://docs.microsoft.com/en-us/windows/win32/controls/wm-notify

  • 第1パラメータ wparam にはゴミが入っている。(正確には「イベントを送信したウインドウのID(=識別番号)が入ることになっているがIDの一意性は保証されないのでlparamを使え」と書いてある。)
  • 第2パラメータ lparam には通知詳細(NMHDR 構造体) を指すポインタが入っている。

サクラエディタの WM_NOTIFY ハンドラ実装である CDialog::OnNotify のシグニチャは、BOOL CDialog::OnNotify( WPARAM wParam, LPARAM lParam ) となっています。

マニュアルをマジメに読んでいたら BOOL CDialog::OnNotify( NMHDR* pNMHDR ) とかになりそうですが、そうなってはいません。

「メンテしやすくする」という観点からすると以下のようなコードがコピペで増えまくるのはどうなんだろうと思うわけです。

    NMHDR* pNMHDR = (NMHDR*) lParam;

・・・やばい。結論がない:cry:

ま、いいか・・・。

@berryzplus
Copy link
Contributor

まったりと検討継続中・・・な認識です。

@berryzplus
Copy link
Contributor

放置期間が1年を超えました。必要であれば「再開するときに」再オープンしてください。

@berryzplus berryzplus closed this Oct 9, 2021
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.

5 participants