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

wildmode=longest 時にマルチバイト文字がバイト単位で補完される #791

Closed
thinca opened this issue Nov 19, 2015 · 24 comments

Comments

@thinca
Copy link
Member

thinca commented Nov 19, 2015

再現スクリプト:

" sample.vim
scriptencoding utf-8

function! s:complete(lead, line, pos) abort
  return ['', '']
endfunction

command -nargs=1 -complete=customlist,s:complete Test :

set wildmode=longest
$ vim -u NONE -i NONE -N -S sample.vim

:Test <Tab> と入力。

結果:

:Test <e3><81>

list:longest の場合も同様。

UTF-8 で、「あ」(0xE3 0x81 0x82) と「い」(0xE3 0x81 0x84) の共通部分が補完されてしまっている。
マルチバイト文字を認識して文字単位で補完してほしい。

@h-east h-east self-assigned this Nov 19, 2015
@h-east
Copy link
Member

h-east commented Nov 19, 2015

patched.
ついでに内側のforの開始値が0なのは無駄なので1に修正。

@thinca test書きませんか?

diff -r 873eae260c97 src/ex_getln.c
--- a/src/ex_getln.c    Tue Nov 10 21:30:05 2015 +0100
+++ b/src/ex_getln.c    Thu Nov 19 18:03:55 2015 +0900
@@ -3691,9 +3691,43 @@
     /* Find longest common part */
     if (mode == WILD_LONGEST && xp->xp_numfiles > 0)
     {
+#ifdef FEAT_MBYTE
+   if (has_mbyte)
+   {
+       int mb_len = 0;
+
+       for (len = 0; xp->xp_files[0][len]; len+=mb_len)
+       {
+       mb_len = (*mb_ptr2len)(&xp->xp_files[0][len]);
+       for (i = 1; i < xp->xp_numfiles; ++i)
+       {
+           if (mb_len == 1 && p_fic
+               && (xp->xp_context == EXPAND_DIRECTORIES
+               || xp->xp_context == EXPAND_FILES
+               || xp->xp_context == EXPAND_SHELLCMD
+               || xp->xp_context == EXPAND_BUFFERS))
+           {
+           if (TOLOWER_LOC(xp->xp_files[i][len]) !=
+                       TOLOWER_LOC(xp->xp_files[0][len]))
+               break;
+           }
+           else if (vim_memcmp(&xp->xp_files[i][len],
+               &xp->xp_files[0][len], mb_len))
+           break;
+       }
+       if (i < xp->xp_numfiles)
+       {
+           if (!(options & WILD_NO_BEEP))
+           vim_beep(BO_WILD);
+           break;
+       }
+       }
+   }
+   else
+#endif
    for (len = 0; xp->xp_files[0][len]; ++len)
    {
-       for (i = 0; i < xp->xp_numfiles; ++i)
+       for (i = 1; i < xp->xp_numfiles; ++i)
        {
        if (p_fic && (xp->xp_context == EXPAND_DIRECTORIES
            || xp->xp_context == EXPAND_FILES

@thinca
Copy link
Member Author

thinca commented Nov 19, 2015

はやい! 👍

test書きませんか?

ちょっと考えてみます!(着手は早くても来週以降になりそうです)

@mattn
Copy link
Member

mattn commented Nov 19, 2015

@h-east コード見ずに言ってしまって申し訳ないですが、パッチを見た感じ not FEAT_MBYTE 部分とコード共通部分が多いんじゃないかと思ってます。オフセット値のみを FEAT_MBYTE で変えるってのだと難しいでしょうか?

@h-east
Copy link
Member

h-east commented Nov 19, 2015

@thinca 土曜日の恰好のライブコーディングネタだと思うんだけどなぁ:smile:

@mattn 3箇所違うのでこのままでいこうかと思います。

@thinca
Copy link
Member Author

thinca commented Nov 19, 2015

もうネタ決まってるんで…。

@h-east
Copy link
Member

h-east commented Nov 19, 2015

test書いてvim_devに投げた。
https://groups.google.com/d/msg/vim_dev/THHZ20KDguQ/GW4pQjXjBgAJ

@mattn
Copy link
Member

mattn commented Nov 19, 2015

👍

@crazymaster
Copy link
Member

@h-east
Copy link
Member

h-east commented Nov 20, 2015

Thanks!

Appveyorでコケている件、patch投げました。
https://groups.google.com/d/msg/vim_dev/IeXBr0P7k7M/BJvkRrgBBwAJ

mattn>
Bram氏からまっつんさんと同じこと言われてBram氏が共通化した修正をしてくれました。。

@mattn
Copy link
Member

mattn commented Nov 20, 2015

でも good try! 👍

@h-east
Copy link
Member

h-east commented Nov 20, 2015

test_utf8 がAppveyorでfaildになっている件、PRした奴でもダメだったorz
これ普通のWindowsビルド環境(MSVC, MinGW)でもFAILDになるのでしょうか?

@thinca
Copy link
Member Author

thinca commented Nov 21, 2015

まずはパッチ+テスト作成&投稿ありがとうございます!(返事遅れてすいません)
7.4.932 でテストのパッチが取り込まれたっぽいですが、おっしゃられてた通り同じようにテスト落ちてますね…。気まずい…。
余裕あったら後でちょっと見てみます(と言ってもWindowsでのVimのビルド環境がそもそも整ってないので、あまり期待しない方向で…)。

@h-east
Copy link
Member

h-east commented Nov 21, 2015

MinGW + MSYS でAppVeyorの結果と同じ動作になることを確認しました。
ワークアラウンドで一旦変数に入れて先頭が : だったらそれ以降を $put= するように変えてみたけどさらに訳の分からん動作になったw 根本原因を突き止めないとだめですね。

@h-east
Copy link
Member

h-east commented Nov 21, 2015

うーん、これはどう考えてもWindowsでfeedkeys()がバグっている気がする。

@k-takata
Copy link
Member

Windowsのfeedkeys()は以前にもtestで変な動きをしていたことがあったような…。

@h-east
Copy link
Member

h-east commented Nov 21, 2015

@thinca Linux上で構わないので、feedkeys()を使わないtest方法を考えてもらえないですか?

@k-takata なんかそんなことがあった記憶がちょっと蘇って来ました。

@thinca
Copy link
Member Author

thinca commented Nov 21, 2015

全く試さずに勘で言ってしまうんですが、 :Test1 コマンドの実装を :$put =<q-args> にして、feedkeys(":Test1 \<C-L>\<CR>", 't') とかしたらどうなりますかね? (feedkeys() がバグってるならそっち直した方が良さそうではありますが…)

@thinca
Copy link
Member Author

thinca commented Nov 21, 2015

テストは同じ感じで落ちてるので関係ないとは思いますが、longest 関係で修正パッチ入りました。
https://groups.google.com/d/topic/vim_dev/rvHTwwHtUF4/discussion

Patch 7.4.933 (after 7.4.926)
Problem:    Crash when using longest completion match.
Solution:   Fix array index. 

@h-east
Copy link
Member

h-east commented Nov 21, 2015

うーん、これはどう考えてもWindowsでfeedkeys()がバグっている気がする。

を伝えたのと、test_utf8 のrevertを提案してみました。
https://groups.google.com/d/msg/vim_dev/rvHTwwHtUF4/-Ey-bP2ABwAJ

@thinca
Copy link
Member Author

thinca commented Nov 21, 2015

👍

@k-takata
Copy link
Member

Windowsのfeedkeys()は以前にもtestで変な動きをしていたことがあったような…。

見つかりました。 #641, #429

@h-east
Copy link
Member

h-east commented Nov 21, 2015

@k-takata ありがとうございます。そこを参考にSuper Ad Hocな変更(*1)を入れたらAppVeyor OKになりました。(LinuxでもOK)
https://github.com/h-east/vim/blob/master/src/testdir/test_utf8.in

*1

  • feedkeys()の次行を空行(:だけの行)にする。
  • 一旦変数に代入して先頭が:ならそれを抜いて出力。

でもこれ本家に投げれないよなぁ。。

@h-east
Copy link
Member

h-east commented Nov 22, 2015

ダメ元でPR出したった
vim/vim#493

@ynkdir 昔のコメントを引用させて頂きました。ありがとうございます。

@h-east
Copy link
Member

h-east commented Nov 22, 2015

いろいろあったんですが、最終的に以下のpatchでAPPVeyor PASSしました。
Patch 7.4.935
https://groups.google.com/forum/#!topic/vim_dev/_SkdY76yLPw

結論:
Vimのtest script中にfeedkeys()を使う場合は必ずmodeに iを指定しましょう。(今回の場合は it)
なぜなら、test scriptはmappingから呼び出されているため。

皆さんお疲れ様でした。

@h-east h-east closed this as completed Nov 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants