sakura
sakura copied to clipboard
sakura-editor
マクロ関数の選択開始桁と選択終了桁で取得できる数値が桁数でない
バグ？の報告です。 OSDNフォーラムとどちらに報告しようかと迷いましたが、
GitHub Issues … 要望・バグ報告等
とあったので、こちらに報告いたします。
2.3.0.0～2.3.2.0で、以下の二つのマクロ関数を使ったところ、 桁数ではない値が返ってきました。 ・function GetSelectColmFrom() :Integer; ・function GetSelectColmTo() :Integer;
使用環境は以下の通りです。 ・エディション Windows 10 Home 64bit ・バージョン 1803
確認に使ったマクロを以下に抜粋します。 ご確認をよろしくおねがいします。
----- マクロのサンプル -----
// 選択範囲を取得
var lineFrom = Editor.GetSelectLineFrom();
var colmFrom = Editor.GetSelectColmFrom();
var lineTo = Editor.GetSelectLineTo();
var colmTo = Editor.GetSelectColmTo();
// メッセージを用意
var msg = "From [" + lineFrom + ", " + colmFrom + "]\n";
msg += "To [" + lineTo + ", " + colmTo + "]";
// メッセージボックス表示
MessageBox(msg, 0);
----- マクロの結果 2.2.0.1 ----- From [2, 2] To [2, 3]
----- マクロの結果 2.3.0.0 ----- From [2, 8] To [2, 15]
ご報告ありがとうございます。
OSDNフォーラムのほうは「GitHub怖いと感じている人向けの避難所」みたいなものですので、本当は直接Issue投げてもらえるのが一番助かります。
内容は後ほど確認しますね。 ご協力ありがとうございます。
情報あざっす。
ぼくの認識では、桁位置報告の誤りは既知の問題の認識でした。 サクラエディタの内部では桁をあらわす型がたくさんあるので、 取り違えが起こっているのではと思っています。
- 位置 行データの先頭から何文字目にあたるかを示す
- レイアウト位置 レイアウトの先頭から何文字目にあたるかを示す
- 桁位置 行データの先頭から何桁目にあたるかを示す
- レイアウト桁位置 レイアウトの先頭から何桁目にあたるかを示す
- 座標位置 ウインドウのクライアント左端から何px目にあたるかを示す
「桁」という概念は、サクラエディタ特有の考え方だと思っています。
----- マクロの結果 2.2.0.1 ----- From [2, 2] To [2, 3]
----- マクロの結果 2.3.0.0 ----- From [2, 8] To [2, 15]
報告された値からすると、to{行,桁}の桁に入っている値がおかしいですね。
15 = 3 × 5 で、期待値は 3 です。じゃあ 5 はなんだ？と考えると・・・ こないだどこかで書きましたが、標準的な半角フォントの文字幅は 6px くらいです。 桁位置ではなく座標を返してるんではないか、と推定するに足る根拠だと思います。
いろいろと考え方はあると思いますが、 コードの特定箇所を示して「ここおかしくね？」とやるには GitHub Issues が便利だと思います。
今回はここで良かったんじゃないかな、というのがぼくの所感です。
* 「```Editor.GetSelectLineTo()```が返すべき値は桁位置である」が仕様なのであれば、git(svn)の履歴からいつ壊したかを特定できると思っています。
* 「```Editor.GetSelectLineTo()```が返すべき値は画面座標である」が仕様なのであれば、仕様変更の展開漏れだと思うので、ヘルプに記載するなり対応が必要だと思っています。
いや、原則Issue推奨にしませんか。個人的にはそのほうがありがたいです。この話(issue or osdn)の続きは management-forum で話しましょうか
こないだどこかで書きましたが、標準的な半角フォントの文字幅は 6px くらいです。 桁位置ではなく座標を返してるんではないか、と推定するに足る根拠だと思います。
なるほど、そうゆうことだったのですね。 マクロのサンプルを以下のように修正して確認してみました。
----- マクロのサンプル -----
// 選択範囲を取得
var lineFrom = Editor.GetSelectLineFrom();
var colmFrom = Editor.GetSelectColmFrom();
var lineTo = Editor.GetSelectLineTo();
var colmTo = Editor.GetSelectColmTo();
// ピクセル数を桁数に変換
colmFrom = Math.floor(colmFrom / 7) + 1;
colmTo = Math.floor(colmTo / 7) + 1;
// メッセージを用意
var msg = "From [" + lineFrom + ", " + colmFrom + "]\n";
msg += "To [" + lineTo + ", " + colmTo + "]";
// メッセージボックス表示
MessageBox(msg, 0);
折り返さない設定で一行に500文字ほど入力して、 500桁目（とその付近）を選択した後、マクロを実行して確認したところ、 2.2.0.1で取得できた値と同じ値を算出することが出来ました！
6pxだと少しずれてしまったので、7pxで計算しました。 （使用しているフォントにもよるのかもしれません）
一先ず、2.3.0.0～2.3.2.0のバージョンでは、上記のサンプルのように使用していこうと思います。 ご回答ありがとうございます。
「Editor.GetSelectLineTo()が返すべき値は桁位置である」が仕様なのであれば、git(svn)の履歴からいつ壊したかを特定できると思っています。
http://svn.code.sf.net/p/sakura-editor/code/sakura/trunk2/ に対して svn bisect? (二分探索)で特定しました。
以下のファイルにあるテキストファイルと、報告いただいたマクロファイルを元に 確認しました。
手順
- 添付の中の 無題1.txt を開く
- ツール → キーマクロの読み込みを選ぶ
- 添付ファイル中 の test.js を選択する
- 以下のスクリーンショットのように選択する
- ツール → キーマクロの実行を選ぶ
結果まとめ
| revision | From | To | メモ |
|---|---|---|---|
| 4013 | 3, 2 | 4, 3 | |
| 4026 | 3, 2 | 4, 3 | |
| 4033 | 3, 2 | 4, 3 | |
| 4034 | 3, 2 | 4, 3 | |
| 4035 | 3, 8 | 4, 15 | ここで変わった |
| 4036 | 3, 8 | 4, 15 | |
| 4039 | 3, 8 | 4, 15 | |
| 4064 | 3, 8 | 4, 15 | |
| 4115 | 3, 8 | 4, 15 | |
| 4216 | 3, 8 | 4, 15 |
対応する git の commit
66f5d68d3f6a4bbf65b6f02fea35e560298e9a07 です。
commit 66f5d68d3f6a4bbf65b6f02fea35e560298e9a07
Author: syat <[email protected]>
Date: Sun Sep 20 08:51:56 2015 +0000
New: プロポーショナルフォント
[patchunicode:#713] wiki:Request/500
フォント設定、印刷ページ設定でプロポーショナルフォントが使用できるようになります。
git-svn-id: https://svn.code.sf.net/p/sakura-editor/code/sakura/trunk2@4035 f7ce1907-e4c7-47ca-9f76-12c87ed2c91c
66f5d68d3f6a4bbf65b6f02fea35e560298e9a07 の一つ前の 23f826e3f8651c9ed063eb620e934ae20025054b では従来の動きでした。
commit 23f826e3f8651c9ed063eb620e934ae20025054b
Author: ryoji <[email protected]>
Date: Sat Sep 19 00:23:24 2015 +0000
Fix: Undo/Redo時にキャレットが期待する位置に移動しない
[patchunicode:#999]
Undo/Redo時にキャレットが期待する位置に移動しないケースがあるのを修正します。
rev.3871 「Fix: キャレットの更新でGetDrawSwitchフラグを見るように」以後で発生します
全置換を実行して５カ所以上が一度に置換された場合にUndo/Redoすると再現します。
git-svn-id: https://svn.code.sf.net/p/sakura-editor/code/sakura/trunk2@4034 f7ce1907-e4c7-47ca-9f76-12c87ed2c91c
プロポーショナルフォント対応は v2.2.0 の目玉の一つと認識しています。 あまりに画期的過ぎる機能がたくさん入った結果、リリース後にゴタゴタしたっていうですね・・・ｗ こちらでも少し見て見ます。
66f5d68d3f6a4bbf65b6f02fea35e560298e9a07 ではいっぱい変更点がある。
死ぬほど変更されていて速攻で萎えたんですがｗ
TortoiseGit のログに表示されていた 66f5d68d3f6a4bbf65b6f02fea35e560298e9a07 の変更内容
行数: 1239増 784減、ファイル: 変更 = 77 追加 = 0 削除 = 0 置換 = 0
もちろん、GitHubの情報とも一致しています。
Showing 77 changed files with 1,239 additions and 784 deletions.
GetSelectLineTo の呼出階層から洗った方が速そうです。
GetSelectLineTo の呼出階層から洗った方が速そうです。
単に書き間違いかもしれませんが、問題は行ではないです。列です。 GetSelectColmFrom と GetSelectColmTo です。
単に書き間違いかもしれませんが、問題は行ではないです。列です。
たんに書き間違いです。 調べ始めてすぐ気付きました「ま、いっか」的な感じになってました。
\bF_GETSEL(LINE|COLUMN)(FROM|TO)\b で検索すれば、
マクロからの呼出箇所は1箇所だけであることがわかります。
https://github.com/sakura-editor/sakura/blob/e904d435359cb7db59e0a5346178b33714c597c6/sakura_core/macro/CMacro.cpp#L1629-L1634
呼出行は問題コミットでは触られていません。
View->GetSelectionInfo().m_sSelectが返すモノの中身が変わってるということです。
絵に描いたようなスパゲティコードなので原因追及は手間取りそうです。
う～む、原因っぽいものを見付けました。 似たような変更がたくさんあります。 情報共有のために右矢印キー（→）のハンドラメソッドを例に考え方を解説します。
ここから解説
diff --git a/sakura_core/cmd/CViewCommander_Cursor.cpp b/sakura_core/cmd/CViewCommander_Cursor.cpp
index 8b614a80..711833e4 100644
--- a/sakura_core/cmd/CViewCommander_Cursor.cpp
+++ b/sakura_core/cmd/CViewCommander_Cursor.cpp
@@ -307,7 +307,7 @@ void CViewCommander::Command_RIGHT( bool bSelect, bool bIgnoreCurrentSelection,
const bool nextline_exists = pcLayout->GetNextLayout() || pcLayout->GetLayoutEol() != EOL_NONE; // EOFのみの行も含め、キャレットが移動可能な次行が存在するか。
// 現在のキャレットの右の位置( to_x )を求める。
- CMemoryIterator it( pcLayout, GetDocument()->m_cLayoutMgr.GetTabSpace(), GetDocument()->m_cLayoutMgr.m_tsvInfo );
+ CMemoryIterator it = GetDocument()->m_cLayoutMgr.CreateCMemoryIterator(pcLayout);
for( ; ! it.end(); it.scanNext(), it.addDelta() ) {
if( ptCaret.x < it.getColumn() ) {
break;
CMemoryIterator it の取得方法が変更されてます。 よくわかりませんが、コンストラクタによる生成をファクトリメソッドに切り替えたようです。 独自クラス CMemoryIterator は、よくわからないクラスですが、使われ方からして reverse_iterator っぽいものと考えられます。とりあえずここは、取得方法が変更されてるねってとこだけ押さえておいてください。
CMemoryIterator itの取得結果は、移動先の桁位置に影響します。
https://github.com/sakura-editor/sakura/blob/e904d435359cb7db59e0a5346178b33714c597c6/sakura_core/cmd/CViewCommander_Cursor.cpp#L316
取得した it.GetColumn() と「キャレットの現在の桁位置+1」を比較して大きい方を取ってます。
通常ルートの移動先位置の決定はこうなってます。
https://github.com/sakura-editor/sakura/blob/e904d435359cb7db59e0a5346178b33714c597c6/sakura_core/cmd/CViewCommander_Cursor.cpp#L370-L371
x_maxには最大桁数っぽいものが入っています。 まぁ、見たまんまですね。 ここで std::min することによって、移動先がはみ出すことを防止しています。 ※t_minはstd::minのシノニムみたいなもんです。
問題になってる GetSelectColmFrom と GetSelectColmTo の取得結果に影響を与えるのはカーソル移動の「前」にある以下の呼出です。
https://github.com/sakura-editor/sakura/blob/e904d435359cb7db59e0a5346178b33714c597c6/sakura_core/cmd/CViewCommander_Cursor.cpp#L382
メソッド名に関してのツッコミは一旦置いておくとして、 ここまでが右方向矢印キー押下時の処理になります。 キャレット移動の処理では起点となるのが CMemoryIterator で、 そこに「何かの変更」が入ったという雰囲気はつかめたと思います。
原因箇所の特定、なんですが一旦コメントを分けることにします。 ざっくりとした原因はつかめているのですが、 うまく説明できそうにないのでもうちょっと考えたい感じです。
なんだこれ？な追加関数を発見しました。
https://github.com/sakura-editor/sakura/blob/e904d435359cb7db59e0a5346178b33714c597c6/sakura_core/view/CTextMetrics.h#L59-L66
関数名は描画単位（ピクセル幅）を返す関数のように見えますが、 実装は渡した数値をそのまま返すようになっています。
引数なし版で固定値1を返していることから、 ピクセル幅ではなく桁数を返す関数であるように見えます。
次のアクションをどうしたらいいか分からなくなってきました。 バグとして対応するには影響範囲が大きすぎるので。
某匿名掲示板で「昔cyclemanにまったく同じ質問が投げられてるじゃん」という書込みがありました。
http://sakura-editor.sourceforge.net/cgi-bin/cyclamen/cyclamen.cgi?log=unicode&v=2364
掲示板ログでは「これはプロポーショナルフォント対応に伴う仕様変更」と説明されています。 v2.3.0の新機能、とも書いてありますね・・・。 ぼくが見始めたの導入後ですわ・・・。
選択桁位置を取得する関数をどう仕様変更したら 選択桁位置の描画座標を取得する関数になるのか。 理解がおっつかないのはぼくだけなんでしょうか？
登場人物（？）の整理
| 用語 | 説明 |
|---|---|
| 文字位置 | 前行の改行（またはファイル先頭）から何個目にあたるか(TCHAR単位) |
| 桁位置 | 行頭からN文字目が何桁目にあたるか(桁単位、半角が1桁、全角は2桁) |
| 描画位置 | ウインドウ左端から何px目にあたるか(デバイス単位) |
用語と内部構造をマッピングするとこんな感じ。
| 用語 | サクラエディタ用語 | 定義型 |
|---|---|---|
| 文字位置 | ロジック位置 | CLogicInt |
| 桁位置 | レイアウト位置 | CLayoutInt(CLayoutXInt、CKetaXIntという定義もある) |
| 描画位置 | レイアウト位置 | CLayoutInt |
描画位置の概念は、問題のコミットではいったものです。 新要素「描画位置」を入れるにあたって既存「桁位置」の置き換えが十分にできてない感じです。
次アクションは、「仕様、結局どうする？」の議論なのかなと思っています。
次アクションは、「仕様、結局どうする？」の議論なのかなと思っています。
今更ですが、この場合元の関数の意味を考えると、元の振る舞いに戻すのが正しい気がします。
選択桁位置の描画座標を取得する関数
これを別名で（今の仕様）で作るのが今回についてはいいような。
既存の関数の振る舞いを変えた方が既存の利用しているマクロに影響が少ない場合も無きにしも非ずですが今回は意味が変わっちゃってますし。
今更ですが、この場合元の関数の意味を考えると、元の振る舞いに戻すのが正しい気がします。
同意します。 ただ、v2.3.0がリリースされてから3年の間に、この仕様に依存したマクロもそれなりに作られていることでしょうから、移行しやすいように別名で現行仕様の関数を用意するのは必要だと思います。 あとは、プロポーショナルフォントを使ったときに返ってくる値の意味を明確に定義することも必要でしょう。
(ところで、マクロでサクラエディタのバージョンを判定して、バージョンに応じた動作をさせることはできるんでしたっけ。)
この Issue が何を問題としているのかが自分には不明瞭です。
- 桁を知ろうとしたら桁ではなかった、というだけでは何が問題だったのかがわかりません。
- プロポーショナルフォントを使用しているときに「桁」が何を意味するのかを定義できなければ「桁を知りたい」という目的が意味をなしません。
これらを明確にしたうえで、取得した値が「桁数」であったにしろ「X 座標」であったにしろマクロでそれらの値を取得・設定する限りにおいて違いを意識せずに済むように、マクロ関数同士で対応が取れていれば問題にはならないはずです。
関連するかも知れない余談。Ruby ではあるバージョンから ?X で得られるものが文字定数から長さ１の文字列へと変更されましたが、?X の利用シーンを調べて ?X の受け入れ側で(長さ１の)文字列を受け入れるような変更が同時に為されたために、利用者は ?X の型の変更を意識する必要がありませんでした。
桁 というのは行の先頭から何文字目なのかを表す1から始まる数値という定義で良いと思います。等幅フォントだろうがプロポーショナルフォントだろうが。
Alt + 左右キーによる範囲選択が不必要にピクセル単位で動いてしまうケースがあってｷﾞｷﾞｷﾞｷﾞ
桁を知ろうとしたら桁ではなかった、というだけでは何が問題だったのかがわかりません。 プロポーショナルフォントを使用しているときに「桁」が何を意味するのかを定義できなければ「桁を知りたい」という目的が意味をなしません。
プロポーショナルフォントが入ってきたときに、その関数の期待値がどうあるべきかをあまり議論しないで、もしくは議論する必要性もないまま、実装的にやりやすいほうに倒れているのが現状なのかなと推測しますが、プロポーショナルフォントがないころを考えると、 @beru さん言われるように、 「先頭から何文字目」がその関数の振る舞いとしては正しいように思います、「桁」の定義が「何文字目」とイコールというわけではありませんが。 VBAの文字列関数でいうところの「MID$」と「MIDB$」みたいな別なX座標関数が、新しい概念が入ってきたときに加わってもよかったのかもと。
これらを明確にしたうえで、取得した値が「桁数」であったにしろ「X 座標」であったにしろマクロでそれらの値を取得・設定する限りにおいて違いを意識せずに済むように、マクロ関数同士で対応が取れていれば問題にはならないはずです。
この論点については同意します。
「先頭から何文字目」
「何文字目」というのは「半角文字何文字分」だったのではないかなと思います。そういう要求に対しては旧掲示板でもかさんが対応策が用意されていることを指摘し、問題報告者がそれに応じています。
Alt + 左右キーによる範囲選択が不必要にピクセル単位で動いてしまうケースがあってｷﾞｷﾞｷﾞｷﾞ
こういうパッチがありますよ>「SAKURA Editor / PatchUnicode / #1047 プロポーショナル版で変更された単語単位移動を戻す」
「先頭から何文字目」
「何文字目」というのは「半角文字何文字分」だったのではないかなと思います。そういう要求に対しては旧掲示板でもかさんが対応策が用意されていることを指摘し、問題報告者がそれに応じています。
お、確かに。等幅に限定しても半角と全角文字があるので、ちゃんと半角って単位に明言しないと駄目ですね。
「何文字目」というのは「半角文字何文字分」だったのではないかなと思います。そういう要求に対しては旧掲示板でもかさんが対応策が用意されていることを指摘し、問題報告者がそれに応じています。
お、なるほどですね。 この手の話って今だと、１バイト２バイト文字と同義でなくなってるので、結局スコープ次第なのかなと。
こういうパッチがありますよ>「SAKURA Editor / PatchUnicode / #1047 プロポーショナル版で変更された単語単位移動を戻す」
紹介ありがとうございます。
https://sourceforge.net/p/sakura-editor/patchunicode/1006/
こちらをまず適用しないといけないみたいですね。
両方適用したところ期待する動作になりました。 処理内容は全く理解していませんが ~~とりあえずPR出そうと思います~~ PR #738 を作りました。
この手の話って今だと、１バイト２バイト文字と同義でなくなってるので、結局スコープ次第なのかなと。
サクラエディタには「ルーラー」(非表示にできない)が付いてるので、 「ルーラーの1目盛り＝1桁」と考えればいいと思います。
プロポーショナルフォント対応としては、ルーラー出さないモードも考えたほうが良かったのかな？
@beru さん
両方適用したところ期待する動作になりました。 処理内容は全く理解していませんが ~~とりあえずPR出そうと思います~~ PR #738 を作りました。
ではいったんそれをマージする方向でいきますか（ #738 へ続く・・・
サクラエディタには「ルーラー」(非表示にできない)が付いてるので、 「ルーラーの1目盛り＝1桁」と考えればいいと思います。
プロポーショナルフォント対応としては、ルーラー出さないモードも考えたほうが良かったのかな？
プロポーショナルフォント対応のニーズってどこにあるのか自分は理解していません…。対応するにしても等幅フォント使用時の操作性を損ねてしまうと困るなぁと思います。逆もまた然りなんでしょうけど。
@beru さん
プロポーショナルフォント対応のニーズってどこにあるのか自分は理解していません…。対応するにしても等幅フォント使用時の操作性を損ねてしまうと困るなぁと思います。逆もまた然りなんでしょうけど。
経緯はよく分っていません。
文字を綺麗に表示したい要求から来たものと思っていますが、もしかしたら「使えないフォントがある」への対策なのかも知れません。単に「使えないフォント」への対策だとすれば、variadicに描画する必要はないので、「文字を綺麗に表示したかった」の方がよりしっくり来る予想です。
では、文字を綺麗に見せるためには可変ピッチにしないとダメなのか？ということですが・・・。 「日本語」を主眼においた場合「そうでもないんじゃね？」と思っています。 「英語」を主眼においた場合、可変ピッチで単語間を離したほうが読みやすいと思います。 「コード」を主眼においた場合、可変ピッチで描画されたらちゃぶ台返ししたくなります。
さて、ここからどうするか・・・。
「ルーラー」(非表示にできない)
嘘ですよ。そんな使い道のないものに自分がスペースを与えることはありません。
プロポーショナルフォント対応のニーズ
選べるフォントが増えることだと理解しています。矩形選択などを活用したい場合には等幅フォントを選べば済むことです(たぶん)。
「ルーラー」(非表示にできない)
嘘ですよ。そんな使い道のないものに自分がスペースを与えることはありません。
え、どうやって？
共通設定⇒ウインドウ⇒ルーラーの高さを0にすると、表示されなくなる・・・ と見せかけてルーラーの高さが2pxに制限されるようになるっぽいです。
利用バージョンはこれ。
サクラエディタ Ver. 2.3.2.0 (GitHash 3cea1c325490604087a6bf08bd55fa622535e35a)
なんか壊してるかしら・・・