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

設定画面の数値入力用のエディット コントロールにフォーカス時にIMEを使わないように設定 #1212

Merged
merged 1 commit into from
Mar 8, 2020

Conversation

beru
Copy link
Contributor

@beru beru commented Mar 7, 2020

PR の目的

キーボード入力での設定を行いやすくするのが目的です。

従来の動作

サクラエディタのエディタ画面でIMEを有効にした状態から、設定画面を開いて数値の入力項目にフォーカスを合わせた後にキーボード入力を行うと、IMEで入力する動作になります。

この状態だとエンターキーを押したりしないと入力が確定されないので入力がしにくいと思います。数値入力ならばIMEを使う必要はないと考えています。

このPRの動作

このPRでは設定画面の数値入力用のエディット コントロールにフォーカスが合った際にIMEの入力をオープンではない状態(OFF)にする事で設定がしやすいようにしました。

フォーカス消失時はIMEのオープン状態を元に戻す処理を入れる事で、ほかの場所に影響が出ないようにしています。

カテゴリ

  • 仕様変更

PR のメリット

キーボード入力での設定が少し行いやすくなります。

PR のデメリット (トレードオフとかあれば)

ユーザーによってはIMEが自動的にオフになる事で使いにくくなったと思うかもしれません。
日常的にIMEの入力を変換する事で特定の数字を入れるようにするなどの変則的な使い方をしていた場合等です。ただおそらくそういうユーザーはいないだろうと考えています…。というかそんな使い方は良くない…。

参考資料

https://docs.microsoft.com/en-us/windows/win32/api/imm/nf-imm-immgetcontext
https://docs.microsoft.com/en-us/windows/win32/api/imm/nf-imm-immreleasecontext

https://docs.microsoft.com/en-us/windows/win32/api/imm/nf-imm-immgetopenstatus
https://docs.microsoft.com/en-us/windows/win32/api/imm/nf-imm-immsetopenstatus

https://docs.microsoft.com/en-us/windows/win32/menurc/wm-command

https://docs.microsoft.com/en-us/windows/win32/controls/en-setfocus
https://docs.microsoft.com/en-us/windows/win32/controls/en-killfocus

関連項目のリソースID一覧
IDD_JUMP
	IDC_EDIT_LINENUM
	IDC_EDIT_PLSQL_E1

IDD_PRINTSETTING
	IDC_EDIT_FONTHEIGHT
	IDC_EDIT_LINESPACE
	IDC_EDIT_DANSUU
	IDC_EDIT_DANSPACE
	IDC_EDIT_MARGINTY
	IDC_EDIT_MARGINBY
	IDC_EDIT_MARGINLX
	IDC_EDIT_MARGINRX
	
IDD_PROP_COLOR
	IDC_EDIT_LINECOMMENTPOS
	IDC_EDIT_LINECOMMENTPOS2
	IDC_EDIT_LINECOMMENTPOS3
	
IDD_PROP_FILE
	IDC_EDIT_AUTOLOAD_DELAY
	IDC_EDIT_nDropFileNumMax
	IDC_EDIT_ALERT_FILESIZE
	IDC_EDIT_AUTOBACKUP_INTERVAL
	
IDD_PROP_FNAME
	IDC_EDIT_SHORTMAXWIDTH
	
IDD_PROP_GENERAL
	IDC_EDIT_REPEATEDSCROLLLINENUM
	IDD_PROP_GENERAL
	IDC_EDIT_MAX_MRU_FILE
	IDC_EDIT_MAX_MRU_FOLDER
	
IDD_PROP_MACRO
	IDC_MACROCANCELTIMER

IDD_PROP_SCREEN
	IDC_EDIT_MAXLINELEN
	IDC_EDIT_CHARSPACE
	IDC_EDIT_LINESPACE
	IDC_EDIT_TABSPACE
	
IDD_PROP_WIN
	IDC_EDIT_nRulerHeight
	IDC_EDIT_nRulerBottomSpace
	IDC_EDIT_nLineNumberRightSpace
	IDC_EDIT_FUNCKEYWND_GROUPNUM
	
IDD_PROP_WINDOW
	IDC_EDIT_LINENUMWIDTH
	IDC_EDIT_BACKIMG_OFFSET_X
	IDC_EDIT_BACKIMG_OFFSET_Y
	IDC_EDIT_BACKIMG_TRANSPARENCY
	
IDD_WINPOSSIZE
	IDC_EDIT_WX
	IDC_EDIT_WY
	IDC_EDIT_SX
	IDC_EDIT_SY
	

@beru beru added the specification change ■仕様変更 label Mar 7, 2020
@beru beru changed the title 設定画面の数値入力用のエディット コントロールにフォーカス時にIMEをオープンしないように設定 設定画面の数値入力用のエディット コントロールにフォーカス時にIMEを使わないように設定 Mar 7, 2020
@AppVeyorBot
Copy link

Build sakura 1.0.2647 completed (commit f8c0be1916 by @beru)

Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

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

PRありがとうございます。

PRの趣旨には同意で、実現方法も大筋で合意です 😃

プログラム設計の観点から些細なツッコミを何件か入れました。(笹井って誰だよっ!
修正するとなると修正量は多いのですが、より分かりやすくで考えるとたぶんやったほうがいい気がしています。

なので、このままでも問題はないんだけど一旦commentにしてみた感じです。

@@ -184,6 +185,35 @@ BOOL CDlgJump::OnBnClicked( int wID )
return CDialog::OnBnClicked( wID );
}

// IMEのオープン状態復帰用
static BOOL s_isImmOpenBkup;
Copy link
Contributor

Choose a reason for hiding this comment

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

BOOL・・・?

おいらの気が確かなら typedef int BOOL; だったはず。
c++なのに bool を使わない理由が何かありますでしょうか?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

WindowsAPIで取得した値の復帰用途なので元の値の型のままにしています。

C++の組み込み型の bool を使った方が違和感が無いのはそうなんですが、WindowsAPIの手続きを隠蔽する意図は無いのでそのままでいいかなと判断しました。

Copy link
Contributor

@berryzplus berryzplus Mar 8, 2020

Choose a reason for hiding this comment

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

隠ぺいする意図はない・・・元の値に何が入っていようと不具合が起きたらwindowsの所為だ!、みたいなw

一旦それで了解です。

しいて言うと Imm系関数 はwindows8以降では obsolete 扱いになっているはずで、どっかで tsf への移行を検討せにゃならんのです。まぁ、「せにゃならん」は正確には、ぼくがやりたいだけですけども。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

いちおうWindowsAPIの戻り値でエラーは見てますが、基本的に自分が書くエラー処理はザルなので問題が見つかるまではしらばっくれます。

Imm系の関数はGDI系の関数と同様に legacy ですが obsolete や deprecated 扱いではないという認識です。

https://docs.microsoft.com/en-us/windows/win32/api/imm/nf-imm-immsetopenstatus

の記述確認しましたが obsolete とは書かれてないです。まだまだ使って大丈夫なはず。多数のユーザーが使っている過去の資産をかなぐりすてるのはAppleがやる事らしいです。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

隠蔽する意図はない、っていうのはQt等のマルチプラットフォーム対応のフレームワークを使って書かれてるわけではないしWindowsAPIべったりの実装が全体的に蔓延してるのであえて抽象化層を整備する気はしないという事です。

Copy link
Contributor

Choose a reason for hiding this comment

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

処理の詳細を隠ぺいしておくメリットとして、インターフェース関数の抽象化が十分であれば、実装がいつの間にか Imm から tsf に切り替わっていても利用側の対処は不要だってことが挙げられます。

極東アジア地域向け以外のwindowsには imm32.dll が同梱されとらんはずなので、外せるものなら外したほうがより良いような気がしています。これはグローバリゼーションの話なのでこことは全然関係ないんですけどw

Copy link
Contributor Author

@beru beru Mar 8, 2020

Choose a reason for hiding this comment

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

Imm系が廃止はまずないでしょう。内部実装が変わって単なるラッパーに成り果ててても、てょまど嬢が頂点 or ミドルエンドの会社の中の人もしくは外注が宜しく保守してくれると思います。

Imm32.dll は既に暗黙的なダイナミックリンクしてるし入ってないPCはサクラエディタ起動出来ないんじゃないかな、知らんけど…。そんなPC使うことになったらサクラエディタ使わないでVS Code使えば桶です。

Copy link
Contributor

@berryzplus berryzplus Mar 8, 2020

Choose a reason for hiding this comment

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

Imm32.dll は既に暗黙的なダイナミックリンクしてるし入ってないPCはサクラエディタ起動出来ないんじゃないかな、知らんけど…。そんなPC使うことになったらサクラエディタ使わないでVS Code使えば桶です。

話通じてると思いますが、その状況をなんとかしたいっていうグローバリゼーションです。つまり、動作環境説明にある「日本語版のwindowsが必要」から「日本語版の」を外したいっていうことを言っとります 👃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

日本沈没してお互いキノコったら考えましょう。

Copy link
Contributor

@berryzplus berryzplus Mar 8, 2020

Choose a reason for hiding this comment

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

い抜かれた...orz

sakura_core/dlg/CDlgJump.cpp Show resolved Hide resolved
BOOL CDlgJump::OnEnKillFocus(HWND hwndCtl, int wID)
{
if (isImeUndesirable(wID))
ImeSetOpen(hwndCtl, s_isImmOpenBkup, nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

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

上の話で進めるとなるとここは

    if( isImeUndesirable(wID) && s_isImmOpenBkup) {
        EnableIme( hwndCtl, &s_isImmOpenBkup);    //IMEを活性に戻す
    }

となるイメージです。

Copy link
Contributor Author

@beru beru Mar 8, 2020

Choose a reason for hiding this comment

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

EnableIME の第2引数はポインタになってますがどうしてでしょうか?フォーカスを失う際はIME開閉状態の復帰の設定だけを行うのでポインタを使う必要はないです。

あと EnableIME と DisableIME で関数を分けると ONにする場合とOFFにする場合で呼び出す関数を別にしないといけないので好ましくないと思います。

自分が util/os.cpp に追加した ImeSetOpen 関数は実は開閉状態の設定だけではなく復帰用の値取得も第3引数のポインタ経由で行っています。それなら名前を ImeGetSetOpen とかにした方が良いかもですが…。この関数を追加したのは WindowsAPI の Imm 系の関数を呼び出す手続きの記述を繰り返し複数ファイルに重複して書くのを避けるためです。

Copy link
Contributor

Choose a reason for hiding this comment

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

ポインタになっていますがどうしてでしょうか?

単に誤記です...orz

あと EnableIME と DisableIME で関数を分けると ONにする場合とOFFにする場合で呼び出す関数を別にしないといけないので好ましくないと思います。

フォーカスを受けるとき: IMEを閉じる or 何もしない
フォーカスを抜けるとき: IMEを開く or 何もしない

呼び分けが必要になって困る理由はなさそうに思います。

この関数を追加したのは WindowsAPI の Imm 系の関数を呼び出す手続きの記述を繰り返し複数ファイルに重複して書くのを避けるためです。

この目的の関数実装の説明をぼくがすると、
Imm系関数の 手続きの詳細を隠ぺい しています。
になります。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

フォーカス抜けるときに以前のオープン状態を復帰するので、オンとオフで関数を分けるとそこで別々に呼び出しを分ける必要性が生じてしまいます。というかWindowsAPI側で別れてないです。

ぱっと見で意味が分かりやすいからといって真偽のそれぞれで関数を分けるより単一の関数の引数指定で対応する方が変数で制御できるので柔軟で良いと思います。

Copy link
Contributor

Choose a reason for hiding this comment

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

フォーカス抜けるときに以前のオープン状態を復帰するので、オンとオフで関数を分けるとそこで別々に呼び出しを分ける必要性が生じてしまいます。というかWindowsAPI側で別れてないです。

もしかして、フォーカスを抜けるとき、OFFに戻すときも関数を呼び出す、と言ってます?

フォーカスを受けるとき: IMEを閉じる or 何もしない
フォーカスを抜けるとき: IMEを開く or 何もしない

という認識なので、フォーカスを抜けるときの処理は、
・focus-in 時に true であれば true に戻す処理を呼び出す。
・focus-in 時に false であれば 何もしない(=関数呼出は不要)。
となると思っていました。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

対象のコントロールはリソースIDでしぼってはいますが、フォーカス時はクローズに設定します。仮にオープンされていなくてもです。

そしてフォーカスを抜けるときは(対象のコントロールはリソースIDでしぼってはいますg…ry)フォーカス時に取得した元のオープン状態(BOOLなのでオープンのTRUEもしくはクローズのFALSE)を設定します。仮に既に元のオープン状態と一致していても、です。

フォーカス時もフォーカスを抜けるときも使っている関数は同じものです。

仮にフォーカス時にオープン状態を取得するのに失敗したら、フォーカスを抜ける際に再設定すべきではないですが、今のとこ失敗するケースが無いっぽいのでエラーチェックしてません。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

今まではテキストエディタ上でIMEをオープンしていたら設定画面の数値設定コントロール上でもIMEのオープン状態が引き継がれていたので、そこを切った感じですね。

ただ従来は、仮にテキストエディタ上でIMEをオープンしていない状態で設定画面を開いて数値設定コントロールにフォーカスしてもIMEはオープンしていない状態ですが、おそらく従来はそこでIMEをオープンした後に設定画面を閉じてテキストエディタ上に戻ってもIMEはオープン状態のままだと思います。このPRの変更でその動作も変わります。連続性がなくなったのをヨシとするかアシとするかの白黒は難しいですがまっくろくろすけだとは思いません。

Copy link
Contributor

Choose a reason for hiding this comment

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

現状の実装は理解してます。意外と 😄

話していたのは「あるべき」の話です。

imm系関数もシステムコールの一種なので、必要ないなら呼ばないのが正しい気がします。

仮にフォーカス時にオープン状態を取得するのに失敗したら、フォーカスを抜ける際に再設定すべきではないですが、今のとこ失敗するケースが無いっぽいのでエラーチェックしてません。

GetImmContext は hwnd に関連付けられた IME が存在しない状態では失敗した気がします。IMEを無効にしたらこの状態になるはずですが、Editコントロールの標準実装が細かい調整をしてくれるはずなので再現は難しかったような気がします。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

呼ぶ必要ない場合は呼ばない方が確かによいですね。
短時間に大量に繰り返し呼ばれないのでコストを考慮しにくいですけど。

コントロールのプロパティにime-modeがあるような環境の実装ではエラーチェックしっかり書かれてるかもしれないですね。それでもたまにIMEがおかしくなりますが Office 365 へのお布施が必要なのかもしれません。

static BOOL s_isImmOpenBkup;

// IMEを使用したくないコントロールのID判定
static bool isImeUndesirable(int id)
Copy link
Contributor

Choose a reason for hiding this comment

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

この関数名、もっと分かりやすくなりませんかね・・・(ただの感想です。

desire という単語を知らないと関数の意味が分からんのが「ん~」と思います。
中森明菜の名曲「DESIRE」 を知らないと分からんってのは平成生まれにはキツいのかも・・・(何の話だ

代替関数名 IsNoImeEdit とか。(センスねぇ...orz

Copy link
Contributor Author

@beru beru Mar 8, 2020

Choose a reason for hiding this comment

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

IsNotExpectedToUseIme はどうでしょうか?
もし私がRikenに勤めていたら STAPCellsDoExist にするかもですが…。

Copy link
Contributor

Choose a reason for hiding this comment

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

SMAPは大好きです。

これも結論出なさそうなので先延ばしで良いと思います。

IsNotExpectedToUseImeは長いっす。

代替命名案2 IsAsciiOnlyEdit
指定されたhwndCtlがASCIIのみ受け付けるエディットコントロールな場合に true を返す。

とかw

Copy link
Contributor Author

@beru beru Mar 8, 2020

Choose a reason for hiding this comment

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

IsNotExpectedToUseIme が長いなら IsImeUnfavourable とかでしょうか。
類義語的な近い意味の言葉は非常に多いので ImeIsNoNoNo とかでもいい気はします。

Copy link
Contributor

@berryzplus berryzplus Mar 8, 2020

Choose a reason for hiding this comment

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

hwndCtl is 何なのよ?が答えになると思います。

hwndCtl is IME undesirable だと文章(?)になっとらん気がするんです。
代替案で執拗に Edit 入れてるのはこれを見越した意図です。

どーせみんな英語なんて読めねーよ、という考え方にも一理あると思いますが、
「英語は読めません」という人が「C言語なら少し読めます」という理屈が、おいらには理解不能なので、ウザがられない程度にこだわりたいかなぁとも思っています。

hwndCtl is not expected to use ime なら意味は通る気がしています。
ただ、imeを使うのは hwndCtrl じゃなくて(ry

Copy link
Contributor Author

@beru beru Mar 8, 2020

Choose a reason for hiding this comment

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

それじゃあ is not suitable for IME とかかなぁ。。

hwndCtl が this なわけではなくてあくまでリソースIDの判定です。引数の型は HWND ではないです。関数名なので命名の自由度というかカオス度は高いですね。

Copy link
Contributor Author

@beru beru Mar 8, 2020

Choose a reason for hiding this comment

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

主語や目的語が関数名に含まれていないという話ならコードやコメント読めば文脈から内容を十分推定出来るかなと思います。

まぁくどいくらい長い名前を付けるのも別に良いかな派です。1行80文字制限が無ければですが…

Copy link
Contributor

@berryzplus berryzplus Mar 8, 2020

Choose a reason for hiding this comment

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

それじゃあ is not suitable for IME とかかなぁ。。
hwndCtl が this なわけではなくてあくまでリソースIDの判定です。引数の型は HWND ではないです。関数名なので命名の自由度というかカオス度は高いですね。

そうか、メンバ関数だから主語は this と考えるのが自然なのかもですね。
this のコンテキストで引数が主語になる判定関数だととらえて話してました。

うーむ。wID は IME に適さない・・・w

ちなみにこのやり取りは「改善の余地があれば」と思って続けているだけっす。
納得がいったら即マージで問題なさそうに思っております。

sakura_core/util/os.h Show resolved Hide resolved
Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

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

問題なさそうに思います。

追加や除外は随時行っていけばよいと思うので、対象コントロールの妥当性に関しては特に判断を行っていません。

@beru
Copy link
Contributor Author

beru commented Mar 8, 2020

レビューありがとうございます。Merge します。
もし問題が見つかったら別途PRを作成して修正します。

@beru beru merged commit 51343db into sakura-editor:master Mar 8, 2020
@m-tmatma m-tmatma added this to the v2.4.0 milestone Apr 19, 2020
@beru beru deleted the ImeSetOpen branch April 20, 2020 17:07
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 16, 2020
設定画面の数値入力用のエディット コントロールにフォーカス時にIMEを使わないように設定
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
specification change ■仕様変更
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants