キーワードヘルプの説明文に"\n"を表示できるようにしたい。 #1399

berryzplus · 2020-09-12T09:00:47Z

PR の目的

キーワードヘルプの説明文に"\n"を表示できるようにします。

カテゴリ

仕様変更

PR の背景

#1359 キーワードヘルプの説明文に"\n"を表示できるようにしたい。 を参照。

PR のメリット

キーワードヘルプの説明文に"\n"を表示できるようになります。
#1399 (comment) で教えてもらった不具合が直ります。
（キーワードヘルプのコピーでキーワードヘルプの説明文以外の\nが改行されてしまう。）

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

キーワードヘルプの説明文の記述ルールが複雑になります。
説明文に \x5C を含んだ既存khpの表示がおかしくなります。(\x5Cでなく\と表示されてしまう。）

仕様・動作説明

キーワードヘルプの説明文の記述ルールを追加します。

--	ルール内容	備考
記述ルール①	`\n` と入れたら改行できる。	この仕様は変更なし。
記述ルール②	`\x5C` と入れたら `\` を表示できる。	このPRでこの仕様を追加します。大文字小文字を区別するので `\x5c` は `\x5c` のまま表示されます。

記述ルールの適用順序は①、②の順番になります。

記述ルールは、エスケープシーケンスではなく「単なる置換」です。
記述ルールとマッチしない \ はそのまま表示されます。

見え方のサンプル

bat.khpを改変して \\n を挿入し、表示確認したキャプチャを貼ります。
~~使用したkhpは bat.zip で取得できます。~~
~~使用したkhpは [https://github.com/sakura-editor/sakura/files/5213150/bat_v2.zip) で取得できます。~~
使用したkhpは [https://github.com/sakura-editor/sakura/files/5213999/bat_v3.zip) で取得できます。

--	表示例
変更前
変更後

テスト内容

仕様説明にある変更前後比較を行いました。

タイプ別設定を開きます。
キーワードヘルプタブを選択します。
キーワードヘルプを使える状態にします。

PR の影響範囲

キーワードヘルプ・辞書ツールの表示に影響します。
キーワードヘルプの右クリックコピーでコピーされる文字列に影響があります。

参考資料

https://osdn.net/projects/sakura-editor/forums/34071/42347/

AppVeyorBot · 2020-09-12T09:22:14Z

✅ Build sakura 1.0.3079 completed (commit 7949f055c8 by @berryzplus)

usagisita · 2020-09-12T11:15:10Z

なんだっけキーヘルプが出る状態で右クリックすると出る「キーワードの説明をクリップボードにコピー」というのがあるんだった気がします。
探しました。ここです。

sakura/sakura_core/cmd/CViewCommander_CustMenu.cpp

Lines 43 to 49 in 17bcab7

    
           // 見た目と同じように、\n を CR+LFへ変換する 
        
           for( i = 0; i < nLength ; ++i){ 
        
           	if( pszWork[i] == L'\\' && pszWork[i + 1] == L'n'){ 
        
           		pszWork[i] =     WCODE::CR; 
        
           		pszWork[i + 1] = WCODE::LF; 
        
           	} 
        
           }

置換そのものはパッチのコードで置換されますが、\\nなどの置換後の文字列がさらに置換されてしまいます。
あとパッチの新置換コードのみだとLFになってコピペ時にCRLFにはなりません。まあここはシーケンスの置換処理はLFにしておいて、コピー直前にファイルの改行コードに置換しなおしたほうが親切かもしれません。

ところでCプリプロセッサの複数行定義みたいに「行末に\マークを置きたい」場合は、今までは\\nだったのが\\\nになるんですよね。
コード見た感じコレには対応していないように思うんですけど、C言語互換というか、ちゃんと\の数、数えて行末に\が来ても改行できるようにしておいたほうが、いいんではないでしょうか。
あと「キーワードも表示する」がOFFのときにパッチのコードだと先頭にある\nが置換できなくなっています。
それから「キーワードも表示する」がONのとき、キーワードに\nが入っているとその部分も置換してしまいますが、たぶん意図するものではないはずです。
ついでに既存コードのCustMenu.cppのほうの置換処理にキーワード部分も置換してしまうバグが元からあるようです。どうせ直すならぜひに一緒にお願いします。
蛇足ですが「ヒットした次の辞書も検索」のとき、ファイル名も表示されますがパスは表示されないっぽいのでパスの\を追加で置換する必要はないみたいです。

たぶん置換処理を関数でくくって、各キーワードからm_cInfoに拾い上げる部分でそれぞれ置換しないとだめっぽいですね。
ついでに置換処理関数のテストを(ry 単体テスト君はこういう変換関数とか大好物ですから。

ちなみにa\nbみたいなキーワードはマウスオーバーだけでは表示されませんが、範囲選択すればキーワード対象になります。
この際、該当処理内の可能な範囲でバグっぽいのは追放しませう。

;テスト用khpファイル
CMDA /// CMDA\\\\nhogehoge (back-slash n)
CMDB /// CMDB\\\nhogehoge (back-slash LF)
CMDC /// CMDC\\hogehoge (back-slash)
CMDD /// CMDD\nhogehoge (LF)
CMDE /// CMDE\hogehoge (non-convert)
CMDF /// \nhogehoge (Top Line LF)
a\nb /// a￥￥nb
a\\nb /// a￥￥nb

berryzplus · 2020-09-12T13:35:21Z

なんだっけキーヘルプが出る状態で右クリックすると出る「キーワードの説明をクリップボードにコピー」というのがあるんだった気がします。
探しました。ここです。

sakura/sakura_core/cmd/CViewCommander_CustMenu.cpp

Lines 43 to 49 in 17bcab7

// 見た目と同じように、\n を CR+LFへ変換する

for( i = 0; i < nLength ; ++i){

if( pszWork[i] == L'\\' && pszWork[i + 1] == L'n'){

pszWork[i] = WCODE::CR;

pszWork[i + 1] = WCODE::LF;

}

}

置換そのものはパッチのコードで置換されますが、\\nなどの置換後の文字列がさらに置換されてしまいます。
あとパッチの新置換コードのみだとLFになってコピペ時にCRLFにはなりません。まあここはシーケンスの置換処理はLFにしておいて、コピー直前にファイルの改行コードに置換しなおしたほうが親切かもしれません。

そんな機能ありましたね・・・完全に「漏れ」なので考慮入れたいと思います。

berryzplus · 2020-09-12T13:47:11Z

ところでCプリプロセッサの複数行定義みたいに「行末に\マークを置きたい」場合は、今までは\\nだったのが\\\nになるんですよね。
コード見た感じコレには対応していないように思うんですけど、C言語互換というか、ちゃんと\の数、数えて行末に\が来ても改行できるようにしておいたほうが、いいんではないでしょうか。

提示した仕様では\nと表示するために\\nを使うので、行末の\を置いたコメントは実現できないです。

これはデグレですね・・・。

正規表現で人間が意図した行末を検知するのは不可能なので、普通に考えると実現不能、って結論になりそうです。

k-takata · 2020-09-12T15:03:46Z

\ によるエスケープの仕様をしっかり定義するのが重要だと思います。PRの「仕様・動作説明」の記載内容では \n 以外の \ をどう扱うのか定義されていません。

berryzplus · 2020-09-12T15:17:06Z

\ によるエスケープの仕様をしっかり定義するのが重要だと思います。PRの「仕様・動作説明」の記載内容では \n 以外の \ をどう扱うのか定義されていません。

もともと定義されていたエスケープは \n のみです。
今回 \n をそのままの文字列として表示させるために \\n という新たなエスケープを作りました。
　　↓
PRでは \\n に瓦当しない \\ も \ に置換してしまっています。
そこも1つ問題であるように思いました。（定義してない処理を行っているから。）

This reverts commit 803c8d9.

berryzplus · 2020-09-12T19:17:53Z

いかん、アニメ見てたら夜があけてしまった...orz

説明はあとで書き直します。

C言語的に言う "\\n" を "\n" に置換する機能のみ
　　↓
C言語的に言う "\\n" を "\n" に置換する機能
C言語的に言う "\\x53" を "\\" に置換する機能　←今回追加。

bat_v2.zip

AppVeyorBot · 2020-09-12T19:22:29Z

✅ Build sakura 1.0.3083 completed (commit 51ca690edb by @berryzplus)

sakura_core/cmd/CViewCommander_CustMenu.cpp

beru · 2020-09-13T06:32:53Z

\ によるエスケープの仕様をしっかり定義するのが重要だと思います。PRの「仕様・動作説明」の記載内容では \n 以外の \ をどう扱うのか定義されていません。

もともと定義されていたエスケープは \n のみです。
今回 \n をそのままの文字列として表示させるために \\n という新たなエスケープを作りました。
　　↓
PRでは \\n に瓦当しない \\ も \ に置換してしまっています。
そこも1つ問題であるように思いました。（定義してない処理を行っているから。）

エスケープシーケンスですが、下記の2つだけでは何が問題なんでしょうか？

khpファイルのエスケープシーケンス記述	置換後
`\n`	LF
`\\`	\

sakura_core/view/CEditView_Search.cpp

k-takata · 2020-09-13T06:36:04Z

C言語的に言う "\\x53" を "\\" に置換する機能　←今回追加。

置換されるのは \x53 だけですか？\x52などはどうなりますか？

beru

CTipWnd::Show メソッドの第3引数の const WCHAR* szText ですが削除してしまった方が良いと思います。その理由ですが

呼び出しで実際に文字列を設定している箇所が無いです。つまり利用されていないです。
引数を無くせば m_cInfo メンバーの差し替えもなくなるので、UnEscapeInfoText を呼び出す必要も無くなります。あとそうすれば UnEscapeInfoText 関数のスコープもファイル内に限定出来るのではないかと。。

berryzplus · 2020-09-13T10:46:16Z

#1399 (comment) の質問に回答します。

エスケープシーケンスですが、下記の2つだけでは何が問題なんでしょうか？

khpに \\n と書いたときの挙動が変わる気がしたので2つ目を \x53 としてました。

これ、エスケープシーケンスではなくて、特定パターンを順序通り置換する仕様なので \\ で問題ないですね。
ちょっと変更後仕様を修正しようと思います。

k-takata · 2020-09-13T10:55:18Z

\\ を表示するために \\ と書いてある khp の動作が変わってしまいますね。(bat_win2k.khp)

berryzplus · 2020-09-13T10:59:41Z

C言語的に言う "\\x53" を "\\" に置換する機能　←今回追加。

置換されるのは \x53 だけですか？\x52などはどうなりますか？

なんか間違っとりました。
やるなら\x53じゃなくて\x5Cをやるべきでした。
https://ja.wikipedia.org/wiki/JIS_X_0201

エスケープシーケンスではなくて「単なる置換」なので、
規定されないシーケンスを変更しない仕様のつもりです。

berryzplus · 2020-09-13T11:28:37Z

\\ を表示するために \\ と書いてある khp の動作が変わってしまいますね。(bat_win2k.khp)

普通にありますね。\\computer-name\とか。

置換仕様を変えておきます。

beru · 2020-09-13T11:33:21Z

\\ を表示するために \\ と書いてある khp の動作が変わってしまいますね。(bat_win2k.khp)

仕様変更ならそれも仕方がないんじゃないかと思いますよ。

berryzplus · 2020-09-13T11:40:51Z

\\ を表示するために \\ と書いてある khp の動作が変わってしまいますね。(bat_win2k.khp)

仕様変更ならそれも仕方がないんじゃないかと思いますよ。

厳密には仕様追加なので、既存khpの表示がおかしくなる仕様追加はマズいと思いました。

AppVeyorBot · 2020-09-13T11:43:36Z

✅ Build sakura 1.0.3090 completed (commit f4be851c20 by @berryzplus)

k-takata · 2020-09-13T11:58:14Z

HLP000105.html も更新すべきでしょう。

AppVeyorBot · 2020-09-13T12:04:44Z

✅ Build sakura 1.0.3091 completed (commit 662c760ce6 by @berryzplus)

beru · 2020-09-13T12:43:01Z

厳密には仕様追加なので、既存khpの表示がおかしくなる仕様追加はマズいと思いました。

確かにユーザーの負担がなるべく少なくなる仕様変更の方が望ましいですね。

AppVeyorBot · 2020-09-13T12:46:45Z

✅ Build sakura 1.0.3093 completed (commit c3be95843c by @berryzplus)

sakura_core/window/CTipWnd.cpp

beru · 2020-09-13T12:49:59Z

sakura_core/window/CTipWnd.cpp

+const WCHAR* UnEscapeInfoText( CNativeW& cInfo )
+{
+	cInfo.Replace( L"\\n", L"\n" );
+	cInfo.Replace( L"\\x5C", L"\\" );


\x5C だけでなく \x5c も \ に置換してあげた方が良かったりするでしょうか？

もともと \n の置換でも \N は考慮してないので、
大文字小文字の違いを考慮する必要はないと思います。（強引

アメリカ人の知能指数が将来的に低下してアルファベットの大文字だけしか使われなくなるかもしれないのでそれで良さそうですね。

C には \N なんて存在しませんし、\x5C も \x5c も使えますので強引すぎますね。
C 系のエスケープシーケンス表記に慣れた人には \x5c が使えないというのは不自然に感じるでしょうから、\x5C にしか対応しないのであれば、ヘルプにも注意書きとして明記しておいた方が親切ですね。

AppVeyorBot · 2020-09-13T14:20:03Z

✅ Build sakura 1.0.3094 completed (commit 7ae443760f by @berryzplus)

berryzplus · 2020-09-14T02:27:58Z

\\ を表示するために \\ と書いてある khp の動作が変わってしまいますね。(bat_win2k.khp)

仕様変更ならそれも仕方がないんじゃないかと思いますよ。

厳密には仕様追加なので、既存khpの表示がおかしくなる仕様追加はマズいと思いました。

よく考えたら\x5C を含んだkhpを独自に使っている人には結局影響ある変更ですね。

k-takata · 2020-09-14T04:06:49Z

新たなルールを追加するのは避けられないので、何らかの形で非互換性が出るのは仕方ないです。

\\ -> \: ルールは分かりやすいが、影響を受けるものが実在する。
\x5C -> \: ルールは複雑。それゆえ影響を受けるものは少なそう。（ゼロとは言えない）

結局、どの程度非互換性を許容するかです。

あるいは、互換性は壊したくないので見た目だけ \n が表示できればいいというのならば、\ + U+2060 + n と入力するのでも行けるかもしれませんが、コピペしたら U+2060 が入ってしまいますね。（U+2060 をどうやって入力するのかという問題もあります。）

berryzplus · 2020-09-14T06:16:18Z

あるいは、互換性は壊したくないので見た目だけ \n が表示できればいいというのならば、\ + U+2060 + n と入力するのでも行けるかもしれませんが、コピペしたら U+2060 が入ってしまいますね。（U+2060 をどうやって入力するのかという問題もあります。）

ぬぅ・・・。
http://www.fileformat.info/info/unicode/char/2060/index.htm

\x5C に円マークを当てるのは日本語版Windowsのローカル仕様なので、「見た目通り」ということならば \xA5 とか ¥ を使うほうが「確からしい」のかも知らんです。
http://www.fileformat.info/info/unicode/char/00a5/index.htm

k-takata · 2020-09-14T07:24:54Z

\x5C に円マークを当てるのは日本語版Windowsのローカル仕様なので、「見た目通り」ということならば \xA5 とか ¥ を使うほうが「確からしい」のかも知らんです。

それは全然別のレイヤーの話です。

berryzplus · 2020-09-14T09:53:25Z

一旦この仕様でレビューをお願いします。

キーワードヘルプの説明文の記述ルールを追加します。

--	ルール内容	備考
記述ルール①	`\n` と入れたら改行できる。	この仕様は変更なし。
記述ルール②	`\x5C` と入れたら `\` を表示できる。	このPRでこの仕様を追加します。大文字小文字を区別するので `\x5c` は `\x5c` のまま表示されます。

記述ルールの適用順序は①、②の順番になります。

記述ルールは、エスケープシーケンスではなく「単なる置換」です。
記述ルールとマッチしない \ はそのまま表示されます。

k-takata · 2020-09-14T10:33:09Z

仕様はそれでいいと思います。
（もう少しシンプルな仕様にできないだろうかという思いはありますが、互換性をあまり壊さないようにするにはこれ以上のものは思いつきませんでした。\x5c を受け付けないのは違和感がありますが、ヘルプに明記しておけばとりあえずは許容範囲でしょう。）

コードレビューは他の方にお任せします。

beru · 2020-09-14T17:33:47Z

動作確認しました。確認するべきパターンがこれだけかというとそうでは無いかもしれませんが考える事を止めます。

;テスト用khpファイル
CMDA /// CMDA\
CMDB /// CMDB\\
CMDC /// CMDC\nCMDC
CMDD /// CMDD\\nCMDD
CMDE /// CMDE\n\nCMDE
CMDF /// CMDF\x5CnCMDF
CMDG /// CMDG\nx5CCMDG

ツールチップ表示

キーワード	訳語	スクリーンショット
CMDA	`CMDA\`
CMDB	`CMDB\\`
CMDC	`CMDC\nCMDC`
CMDD	`CMDD\\nCMDD`
CMDE	`CMDE\n\nCMDE`
CMDF	`CMDF\x5CnCMDF`
CMDG	`CMDG\nx5CCMDG`

beru

問題無いと思います。

beru · 2020-09-14T18:12:07Z

このPRの変更とは無関係ですが、タイプ別設定のキーワードヘルプの画面で辞書ファイルをファイルダイアログで読む際に、Vista以降の新しいスタイルのファイルダイアログが使われる事に気付きました。

共通設定で有効化していなくても自動で使われるのはどうしてか調べてみたところ、下記のフラグが付加されていない場合にそうなるようです。

OFN_ENABLETEMPLATE      0x00000040
OFN_ENABLEHOOK          0x00000020

Ctrl + O のショートカットキー押しでファイルダイアログを開くAPI GetOpenFileName を呼び出す CDlgOpenFile_CommonFileDialog::_GetOpenFileNameRecover が実行されますが、そこで ofn->Flags の値を書き換えて確認しました。

標準を超えてカスタマイズすると将来になって後継への移行に支障が出るというのは何だか色々な事に当てはまりそうです。

berryzplus · 2020-09-15T13:33:48Z

レビューありがとうございます。マージしちゃいます。

berryzplus · 2020-09-15T13:42:57Z

このPRの変更とは無関係ですが、タイプ別設定のキーワードヘルプの画面で辞書ファイルをファイルダイアログで読む際に、Vista以降の新しいスタイルのファイルダイアログが使われる事に気付きました。

共通設定で有効化していなくても自動で使われるのはどうしてか調べてみたところ、下記のフラグが付加されていない場合にそうなるようです。

これは不具合として対策します？

個人的には、ここで表示される「ファイルを開く」ダイアログはサクラエディタ仕様でなくて構わないので「そういうもんです」で済ませてよい気がします。サクラエディタ仕様の「ファイルを開く」ダイアログにはコードページや改行コードを選択できる機能がありますが、ここの処理でそれらを指定しても反映してないと思うので標準で構わないはず。

beru · 2020-09-15T18:11:23Z

これは不具合として対策します？

いや、不具合ではないので対策する必要は無いと思います。

キーワードヘルプの説明文に"\n"を表示できるようにする

803c8d9

beru added the specification change ■仕様変更 label Sep 12, 2020

berryzplus marked this pull request as draft September 12, 2020 13:35

berryzplus added 2 commits September 13, 2020 03:44

Revert "キーワードヘルプの説明文に"\n"を表示できるようにする"

791bf8b

This reverts commit 803c8d9.

キーワードヘルプの説明文に"\n"を表示できるようにする

931b07e

berryzplus marked this pull request as ready for review September 13, 2020 02:35

beru reviewed Sep 13, 2020

View reviewed changes

sakura_core/cmd/CViewCommander_CustMenu.cpp Show resolved Hide resolved

beru reviewed Sep 13, 2020

View reviewed changes

sakura_core/view/CEditView_Search.cpp Outdated Show resolved Hide resolved

beru reviewed Sep 13, 2020

View reviewed changes

レビュー指摘によりコメント修正

9c95e44

berryzplus marked this pull request as draft September 13, 2020 10:46

置換仕様を修正

ca93514

berryzplus added 2 commits September 13, 2020 20:12

レビュー指摘により制御構造のリファクタリングを取消

b008715

レビュー指摘により CTipWnd::Show の第3引数を除去

7e3c597

置換仕様を変更

29ddc5c

berryzplus marked this pull request as ready for review September 13, 2020 11:38

レビュー指摘によりヘルプに説明を追加

eb0ef5b

beru reviewed Sep 13, 2020

View reviewed changes

sakura_core/window/CTipWnd.cpp Show resolved Hide resolved

beru reviewed Sep 13, 2020

View reviewed changes

レビュー指摘によりヘルプに注意喚起を追加

cc95705

beru approved these changes Sep 14, 2020

View reviewed changes

berryzplus merged commit 26e92e8 into sakura-editor:master Sep 15, 2020

berryzplus deleted the feature/enable_escape_for_tip branch September 15, 2020 13:44

キーワードヘルプの説明文に"\n"を表示できるようにしたい。 #1399

キーワードヘルプの説明文に"\n"を表示できるようにしたい。 #1399

Conversation

berryzplus commented Sep 12, 2020 • edited Loading

PR の目的

カテゴリ

PR の背景

PR のメリット

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

仕様・動作説明

見え方のサンプル

テスト内容

PR の影響範囲

関連 issue, PR

参考資料

AppVeyorBot commented Sep 12, 2020

usagisita commented Sep 12, 2020

berryzplus commented Sep 12, 2020

berryzplus commented Sep 12, 2020

k-takata commented Sep 12, 2020

berryzplus commented Sep 12, 2020

berryzplus commented Sep 12, 2020

AppVeyorBot commented Sep 12, 2020

beru commented Sep 13, 2020 • edited Loading

k-takata commented Sep 13, 2020

beru left a comment

Choose a reason for hiding this comment

berryzplus commented Sep 13, 2020

k-takata commented Sep 13, 2020

berryzplus commented Sep 13, 2020

berryzplus commented Sep 13, 2020

beru commented Sep 13, 2020

berryzplus commented Sep 13, 2020

AppVeyorBot commented Sep 13, 2020

k-takata commented Sep 13, 2020

AppVeyorBot commented Sep 13, 2020

beru commented Sep 13, 2020

AppVeyorBot commented Sep 13, 2020

beru Sep 13, 2020

Choose a reason for hiding this comment

berryzplus Sep 13, 2020

Choose a reason for hiding this comment

beru Sep 13, 2020

Choose a reason for hiding this comment

k-takata Sep 13, 2020

Choose a reason for hiding this comment

AppVeyorBot commented Sep 13, 2020

berryzplus commented Sep 14, 2020

k-takata commented Sep 14, 2020

berryzplus commented Sep 14, 2020

k-takata commented Sep 14, 2020

berryzplus commented Sep 14, 2020

k-takata commented Sep 14, 2020

beru commented Sep 14, 2020

beru left a comment

Choose a reason for hiding this comment

beru commented Sep 14, 2020

berryzplus commented Sep 15, 2020

berryzplus commented Sep 15, 2020

beru commented Sep 15, 2020

berryzplus commented Sep 12, 2020 •

edited

Loading

beru commented Sep 13, 2020 •

edited

Loading