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

statuslineの%{}などでevalした際に,try-catch でVimのエラーをキャッチしていても statusline が disable されてしまう #893

Closed
haya14busa opened this Issue Mar 29, 2016 · 18 comments

Comments

Projects
None yet
5 participants
@haya14busa
Member

haya14busa commented Mar 29, 2016

質問・報告の内容

statuslineの%{}などでevalした際に,try-catch でエラーをキャッチしていても statusline が disable されてしまう

(キャッチされないエラーが発生したらstatuslineがdisableされるのは正しい)

再現方法

statusline_test.vimrc

set nocompatible

function! StatuslineKiller() abort
  try
    " :throw ではなく Vim のエラー(任意)を発生させる
    call eval('')
  catch
    return 'OK: error catched!'
  endtry
  return 'OK'
endfunction

set laststatus=2
set statusline=%{StatuslineKiller()}
  1. vim -u statusline_test.vimrc -i NONE -N
  • 'OK: error catched!' がstatuslineに表示される (actual & expected)
  1. :redrawstatus
    • " Press ENTER or type command to continue が表示 (actual & expected)
    • Enter を押す
    • statusline が disable される (&statusline == '')

Expected な挙動

  1. vim -u statusline_test.vimrc -i NONE -N
  • 'OK: error catched!' がstatuslineに表示される
  1. :redrawstatus
    • 'OK: error catched!' がstatuslineに表示される

関連問題

  • actual: statuslineでVimのエラーが発生し,キャッチもされないと, silentlyにstatuslineがdisableされる
  • 個人的な? expected: statuslineでVimのエラーが発生すると,エラーメッセージを表示した上でstatuslineがdisableされる

補足: statuslineがdisableされるのはstatuslineのredrawが起こるごとにエラーが出るとうっとおしいからdisableしているようです.

修正パッチ案

haya14busa/vim@0cd2c2f

ちょっと詳しくなくて付け焼き刃の調査をしながらの修正です.
なのでこれでいいのかよくわかりませんが,僕が意図した挙動になりました.

Vimのバージョン

7.4.1386

OSの種類

  • ArchLinux

どのOSでも再現するはず

その他

両者ともログ長いし読む必要はないですが,上記issueで問題が発覚し,silentlyにstatuslineがdisableされるせいで異常にデバッグが難しかったです.

問題や該当コードを見つけたのは @lambdalisue さんや watiko さんです.

cc @lambdalisue

@haya14busa haya14busa changed the title from statuslineの%{}などでevalした際に,try-catch でエラーをキャッチしていても statusline が disable されてしまう to statuslineの%{}などでevalした際に,try-catch でVimのエラーをキャッチしていても statusline が disable されてしまう Mar 29, 2016

@h-east

This comment has been minimized.

Show comment
Hide comment
@h-east

h-east Mar 30, 2016

Member

補足: statuslineがdisableされるのはstatuslineのredrawが起こるごとにエラーが出るとうっとおしいからdisableしているようです.

一応、ドキュメントに書いてあります。
:h 'statusline'の20行下くらい。

    オプションを評価している間にエラーが発生すると、それ以降のエラーを避け
    るためにオプションに空が設定される。そうしないと画面更新がループに陥っ
    てしまう。
Member

h-east commented Mar 30, 2016

補足: statuslineがdisableされるのはstatuslineのredrawが起こるごとにエラーが出るとうっとおしいからdisableしているようです.

一応、ドキュメントに書いてあります。
:h 'statusline'の20行下くらい。

    オプションを評価している間にエラーが発生すると、それ以降のエラーを避け
    るためにオプションに空が設定される。そうしないと画面更新がループに陥っ
    てしまう。
@h-east

This comment has been minimized.

Show comment
Hide comment
@h-east

h-east Mar 30, 2016

Member

本題とあんまり関係ないけど、LANGで挙動が違いますね。

$ LANG=ja_JP.UTF-8 vim -Nu statusline_test.vimrc

起動直後に既に'statusline'が空。だからステータスラインに[無名]と表示される。
Vim内部のmulti-byte時の処理の中でステータスライン描画処理が呼ばれていると思われる。

$ LANG=C vim -Nu statusline_test.vimrc

@haya14busa さん報告の動作。

Member

h-east commented Mar 30, 2016

本題とあんまり関係ないけど、LANGで挙動が違いますね。

$ LANG=ja_JP.UTF-8 vim -Nu statusline_test.vimrc

起動直後に既に'statusline'が空。だからステータスラインに[無名]と表示される。
Vim内部のmulti-byte時の処理の中でステータスライン描画処理が呼ばれていると思われる。

$ LANG=C vim -Nu statusline_test.vimrc

@haya14busa さん報告の動作。

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Mar 30, 2016

Member

did_emsg も前のパッチと同様に saved_did_emsg で退避すべきです。

Member

mattn commented Mar 30, 2016

did_emsg も前のパッチと同様に saved_did_emsg で退避すべきです。

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Mar 30, 2016

Member

一応、ドキュメントに書いてあります。

確かに書いてありますね.エラーを出すかどうかについては言及してないけど黙って空に設定するところまで意図していた場合はこの修正ではだめそうですね.どうなんだろうか...

退避なるほど.修正しました.

diff --git a/src/screen.c b/src/screen.c
index 9d854a2..edd1dea 100644
--- a/src/screen.c
+++ b/src/screen.c
@@ -6779,7 +6779,7 @@ win_redr_status(win_T *wp)
 redraw_custom_statusline(win_T *wp)
 {
     static int     entered = FALSE;
-    int            save_called_emsg = called_emsg;
+    int            saved_did_emsg = did_emsg;

     /* When called recursively return.  This can happen when the statusline
      * contains an expression that triggers a redraw. */
@@ -6787,9 +6787,9 @@ redraw_custom_statusline(win_T *wp)
    return;
     entered = TRUE;

-    called_emsg = FALSE;
+    did_emsg = FALSE;
     win_redr_custom(wp, FALSE);
-    if (called_emsg)
+    if (did_emsg)
     {
    /* When there is an error disable the statusline, otherwise the
     * display is messed up with errors and a redraw triggers the problem
@@ -6798,7 +6798,7 @@ redraw_custom_statusline(win_T *wp)
        (char_u *)"", OPT_FREE | (*wp->w_p_stl != NUL
                    ? OPT_LOCAL : OPT_GLOBAL), SID_ERROR);
     }
-    called_emsg |= save_called_emsg;
+    did_emsg |= saved_did_emsg;
     entered = FALSE;
 }
 #endif
Member

haya14busa commented Mar 30, 2016

一応、ドキュメントに書いてあります。

確かに書いてありますね.エラーを出すかどうかについては言及してないけど黙って空に設定するところまで意図していた場合はこの修正ではだめそうですね.どうなんだろうか...

退避なるほど.修正しました.

diff --git a/src/screen.c b/src/screen.c
index 9d854a2..edd1dea 100644
--- a/src/screen.c
+++ b/src/screen.c
@@ -6779,7 +6779,7 @@ win_redr_status(win_T *wp)
 redraw_custom_statusline(win_T *wp)
 {
     static int     entered = FALSE;
-    int            save_called_emsg = called_emsg;
+    int            saved_did_emsg = did_emsg;

     /* When called recursively return.  This can happen when the statusline
      * contains an expression that triggers a redraw. */
@@ -6787,9 +6787,9 @@ redraw_custom_statusline(win_T *wp)
    return;
     entered = TRUE;

-    called_emsg = FALSE;
+    did_emsg = FALSE;
     win_redr_custom(wp, FALSE);
-    if (called_emsg)
+    if (did_emsg)
     {
    /* When there is an error disable the statusline, otherwise the
     * display is messed up with errors and a redraw triggers the problem
@@ -6798,7 +6798,7 @@ redraw_custom_statusline(win_T *wp)
        (char_u *)"", OPT_FREE | (*wp->w_p_stl != NUL
                    ? OPT_LOCAL : OPT_GLOBAL), SID_ERROR);
     }
-    called_emsg |= save_called_emsg;
+    did_emsg |= saved_did_emsg;
     entered = FALSE;
 }
 #endif
@lambdalisue

This comment has been minimized.

Show comment
Hide comment
@lambdalisue

lambdalisue Mar 30, 2016

Member

try/catch が入ったのって何時なんだろう?もしも statusline が空っぽになるコードが追加されただいぶ後とかだったら「前はそういうの必要だったかもしれないけど今は try/catch あんだからそれを使ってない statusline のコンポーネント(ユーザーが書いてるものおよびプラグインの双方)が悪いだろJK」って言ってみるとか?

Member

lambdalisue commented Mar 30, 2016

try/catch が入ったのって何時なんだろう?もしも statusline が空っぽになるコードが追加されただいぶ後とかだったら「前はそういうの必要だったかもしれないけど今は try/catch あんだからそれを使ってない statusline のコンポーネント(ユーザーが書いてるものおよびプラグインの双方)が悪いだろJK」って言ってみるとか?

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Mar 30, 2016

Member

えと、try/catch してても駄目なんですよ。
本来「エラーがtry/catch無しの状態で投げられた」と判断すべきところが「エラーが発生したかどうか(try/catch有無関係なしに)」で判断しちゃってるというバグです。

Member

mattn commented Mar 30, 2016

えと、try/catch してても駄目なんですよ。
本来「エラーがtry/catch無しの状態で投げられた」と判断すべきところが「エラーが発生したかどうか(try/catch有無関係なしに)」で判断しちゃってるというバグです。

@lambdalisue

This comment has been minimized.

Show comment
Hide comment
@lambdalisue

lambdalisue Mar 30, 2016

Member

お、それがされてないのは無理だからだと勝手に思ってました。
内部のことは知らないのですが try/catch されたと判定できるのであればそちらの方が良いですね!

Member

lambdalisue commented Mar 30, 2016

お、それがされてないのは無理だからだと勝手に思ってました。
内部のことは知らないのですが try/catch されたと判定できるのであればそちらの方が良いですね!

haya14busa added a commit to haya14busa/vital.vim that referenced this issue Mar 30, 2016

vital: do not throw builtin error even if in the try-catch
Use :runtime and exists() instead of trying calling autoload function to
see the autoload function exists or not.

Cannot test &statusline in themis.vim, so Skip the test

ref: vim-jp/issues#893
@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Mar 30, 2016

Member

それを直してるつもりのパッチですね〜.(did_emsg で本当にいいか厳密に調べきれてませんが)

Member

haya14busa commented Mar 30, 2016

それを直してるつもりのパッチですね〜.(did_emsg で本当にいいか厳密に調べきれてませんが)

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Mar 30, 2016

Member

did_emsg で本当にいいか厳密に調べきれてませんが

did_emsg でいいですよ。

Member

mattn commented Mar 30, 2016

did_emsg で本当にいいか厳密に調べきれてませんが

did_emsg でいいですよ。

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Mar 31, 2016

Member

おお,よかったです:)

Member

haya14busa commented Mar 31, 2016

おお,よかったです:)

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Mar 31, 2016

Member

@haya14busa さん、テスト書けます?

Member

mattn commented Mar 31, 2016

@haya14busa さん、テスト書けます?

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Mar 31, 2016

Member

これでよさそうならテスト書いてみます.(よく考えたらコードが正しいかどうかに関係なくテストかけましたね)

なんかわからなかったら聞くかも知れないですが,その時はよろしくお願いします 🙇

Member

haya14busa commented Mar 31, 2016

これでよさそうならテスト書いてみます.(よく考えたらコードが正しいかどうかに関係なくテストかけましたね)

なんかわからなかったら聞くかも知れないですが,その時はよろしくお願いします 🙇

@haya14busa

This comment has been minimized.

Show comment
Hide comment
Member

haya14busa commented Mar 31, 2016

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Mar 31, 2016

Member

キャッチされないエラーもsilentlyにdisableされるとか冒頭コメントに書いていましたが完全に僕の勘違いだったのでコメントを修正しました.問題は try-catchで囲まれたVimのエラーによってstatuslineがdisableされるというところだけです

Member

haya14busa commented Mar 31, 2016

キャッチされないエラーもsilentlyにdisableされるとか冒頭コメントに書いていましたが完全に僕の勘違いだったのでコメントを修正しました.問題は try-catchで囲まれたVimのエラーによってstatuslineがdisableされるというところだけです

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Mar 31, 2016

Member

eval('') はコメントではっきりエラーを起こすと書くか、eval('unknown expression') みたいにした方がいいかもです。

Member

mattn commented Mar 31, 2016

eval('') はコメントではっきりエラーを起こすと書くか、eval('unknown expression') みたいにした方がいいかもです。

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Mar 31, 2016

Member

確かにそうですね.unknown expression にしておきました.vim/vim@7564ec7

Member

haya14busa commented Mar 31, 2016

確かにそうですね.unknown expression にしておきました.vim/vim@7564ec7

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Apr 4, 2016

Member

投げました vim/vim#729

Member

haya14busa commented Apr 4, 2016

投げました vim/vim#729

@haya14busa

This comment has been minimized.

Show comment
Hide comment
@haya14busa

haya14busa Apr 5, 2016

Member

patch 7.4.1711 vim/vim@a742e08

取り込まれました.ありがとうございましたっ 🐦

Member

haya14busa commented Apr 5, 2016

patch 7.4.1711 vim/vim@a742e08

取り込まれました.ありがとうございましたっ 🐦

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment