メモ
すでに covimerage 側で修正されているかもしれませんが gina.vim に covimerage を導入した際に空っぽの関数があるとパースエラーが出ました。gina.vim では関数内部にコメントを一行書くことで対処しました。念のため

coverage ちゃんと取れてるっぽいです💪

提案
lint なので強制力は無いですが、出ないほうが望ましいと思います。
特に Result と合わせる必要もないので Err の代わりに v:exception を直接使えばいいかなという気がします

確認
map に lambda 式を使うと 1.5 倍ほど遅くなりますがよろしいでしょうか？

https://gist.github.com/lambdalisue/7cbaa0ece9496ec8052f5a86d33d8f17#file-test-vim

ただ最速の expr タイプはわかりにくくなるので all で想定する promises 数がそんなに多くないのであれば許容範囲だと思います。

知りませんでした… 文字列版も検討してみます．ここは all() の引数なので，そこまで要素数が多くなることを想定していませんが，それでも速いに越したことは無いですし．

確認
ここも同様に map(..., 'expr') タイプの方が 1.5 倍ほど早いですがよろしいでしょうか？

https://gist.github.com/lambdalisue/7cbaa0ece9496ec8052f5a86d33d8f17#file-test-vim

ベンチマークを測った上で検討してみます．正直，.then のオーバーヘッドのほうが大きくてほぼ影響ないんじゃないかという気もしています．

上記のベンチマークは 100000 回のループなので無視しても良いと思っています。
そのため「確認」としました

質問
辞書に対して直接関数定義を行わないで、一度スクリプト関数として定義してから function で funcref 化して辞書に代入しているのには何か理由があるのでしょうか？

ここは以前ツイッターで @haya14busa さんに指摘されたのですが，辞書関数だとエラーが出た時のコールバックの関数名が数字になってしまってデバッグが困難になるためです．

なるほど納得。でもデバッグの事情の為にこういうハックするのは少しモニョりますね。今回のケースだと非同期なのでデバッグの簡潔さを重要視した方が良いですが‥‥

コードが若干複雑になってしまうのは確かにあると思います．

実は vital core でもやってます

書くの面倒だという以外はいいことしかないと思ってる

提案
ECMA script では Promise.race()で Promise の状態取得が出来ますが Vim script は関数呼び出しのオーバーヘッドが大きいので上記の定数を s:_vital_created(module) を使ってモジュール定数として定義し s:PROMISE._state を公開アトリビュートとする（必要であれば lockvar も？）のはどうでしょうか？

" 使用感イメージ
let p1 = Promise.resolve()
let p2 = Promise.reject('foobar')
let p3 = Promise.new({ resolve -> execute('echo "Do nothing"') })

echo p1.state == Promise.FULFILLED
echo p2.state == Promise.REJECTED
echo p3.state == Promise.PENDING

すみません，#526 に1文だけ書いてこの PR には書いていなかったのですが，この PR は ES6 の Promise の API の実装にとどめる形にさせてください．理由は，

• bluebird のような強力な API を実装することも考えられるため，今入れたい API を言い始めるとキリが無さそう
• まずは最小の状態で master にマージして実戦投入して，real world で本当に必要な API だけを足したい

ためです．PR の本文にも追記しておきます💪

なのでここからは一応オフトピですが，せっかく提案いただいたので僕の考えを書いておきます．

一般的に，非同期処理で実行の状態を見て処理を分岐するのはバッドプラクティスだと思います．例えば JavaScript には VM 上で他の非同期に走っているコンテキストの状態を取る手段はありませんし，Go ではあえてゴルーチン（coroutine）の状態や ID，今自分がどのゴルーチン（コンテキスト）で実行されているかを取る API を提供していません．
また，飽くまで僕自身の経験としてですが，Promise の状態を取りたいと思ったことが無いというのもあります．

ただ，bluebird のように Promise の 3rd party 実装では Promise の状態を同期的に introspection できるものもあるので，real world で本当に必要なユースケースが発生した時に入れることを考えて議論するのが良いかなと思います．

ご説明ありがとうございます。納得しかない

提案
辞書関数ではなく wait_group インスタンスを第一引数に持つ通常関数にしませんか？以下理由

1. wait_group の定義位置と離れているので self が何を指しているのかパッと理解できない
2. Vim script だと . によるアトリビュート検索も結構遅かった気がするので partial で変数化して Notify(i, v) みたいに呼び出したほうが繰り返し時に早そう（パフォーマンス測定はしていません）

なるほど．特に 1. はその通りですし，ここで辞書関数にする必要は実際無さそうなので普通の関数にします．ありがとうございます．

p は Funcref になり得るので l:P がいいと思います

あれ p は s:PROMISE オブジェクトのコピー（インスタンス）な気がします。

@lambdalisue さんのおっしゃる通り，p は dict なので小文字で大丈夫です．

あ、確かに。了解です。

同名のグローバル関数が定義されていた場合にエラーにならないように l:Result がいいと思います

同名のグローバル関数が定義されていた場合にエラーにならないように l:Result がいいと思います

同名のグローバル関数があってもローカル変数が優先されてエラーにならないようにみえます．7.4 だとそうなるのでしょうか？

https://wandbox.org/permlink/CGv0HxAPsALNt0yk

7.4.729 でも試してみましたが結果は上のものと変わりませんでした

同名のグローバル関数が定義されていた場合にエラーにならないように l:Err がいいと思います

（コメント書いた diff が outdated で閉じちゃったのでこちらにも）

同名のグローバル関数があってもローカル変数が優先されてエラーにならないようにみえます．7.4.729 までは同じ挙動のようなのですが，もっと古いものだとそうなったりするのでしょうか？

https://wandbox.org/permlink/CGv0HxAPsALNt0yk

これだとアウトです（参照ではなく let での定義時に l: が付いてないとエラー）
https://wandbox.org/permlink/vubuQFFNBkoSp47g

v:exeption は関数ではないので関係ないのでは?　Result は関数になりうりそうなので l: つけておいてもよさそう。

あと関連して Err が funcref にならないなら大文字にする必要はないとおもいます. 逆に funcref なら大文字かつ l: はつけるべき。

@tyru なるほど…ありがとうございます．そういうことでしたか．

@haya14busa 確かに例外として投げられるのは文字列なので Err のほうは err で大丈夫そうですね．誤解生みそうなので修正します

同名のグローバル関数が定義されていた場合にエラーにならないように l:Res がいいと思います

execute('echo e') か execute('echo ' . string(e)) ?

execute('echo v') か execute('echo ' . string(v)) ?

execute('echo v') か execute('echo ' . string(v)) ?

execute('echo reason') か execute('echo ' . string(reason)) ?

. が足らないです。
execute('echom "Failed to clone: " . ' . string(err), '')

これでもいいかも？
execute('echom "Failed to clone:" ' . string(err), '')

自分も詳しく job の仕様を把握してませんが、試した限りだと exit_cb が close_cb よりも後に実行されるとは限らないようです。
https://gist.github.com/tyru/0ffee5ba2b8cab7390f2652c9a97bc24
上記の結果 (messages) だと exit_cb のが先に実行されてしまっているため、 buf が空になってしまっています。
close_cb のみ指定して s:read_to_buf() に resolve してもらうとかですかね？ (それでいいという確証はないです…)

私の環境だとコールバックの発火順は exit_cb のほうが後なので，どうやらプラットフォーム依存ですね…ちなみに close_cb だけだと exit status が分からないのでコマンドが失敗したかどうか分からないです．

ちゃんとやるならどちらの順序で発火しても動くようにすべきですが，それをやると example としてイマイチになってしまう気がします（本質的でないコードが増えすぎる）．なにか良い修正案とかないでしょうか… @lambdalisue @thinca @mattn

プロセスの終了とチャネルのクローズはそもそも別の物なので例として不適かと思います。
プロセスが終了した後に全ての出力が欲しいのであれば、オプションに 'drop': 'never' を渡してチャネルから ch_read で持ってくるのが良いかと思います

あと、プロセスの終了はどうでもよくて出力が全部欲しい場合は Vim のヘルプにある通り close_cb でやるべきですかね。
http://vim-jp.org/vimdoc-ja/channel.html#read-in-close-cb

個人的には実用面で言えば「プロセスの出力結果がほしい」が要求であって「プロセスが終了してから出力結果がほしい」ではないと思います。なので例としては、こちらのほうが適切かと思います。

すでに喋るべき内容は語られていた。

@lambdalisue @tyru

お手数ですが，これで動いているか確認いただけませんでしょうか？
自分の環境では元のコードも動いてしまっていたので，これで問題が解決したのかどうかが確認できず…

let s:Promise = vital#vital#import('Async.Promise')

function! s:read(chan, part) abort
  let out = ''
  while ch_status(a:chan, {'part' : a:part}) ==# 'buffered'
    let out .= ch_read(a:chan, {'part' : a:part})
  endwhile
  return out
endfunction

function! s:sh(...) abort
  let cmd = join(a:000, ' ')
  let buf = {}
  return s:Promise.new({resolve, reject -> job_start(cmd, {
        \   'drop' : 'never',
        \   'close_cb' : {ch -> 'do nothing' },
        \   'exit_cb' : {ch, code ->
        \     code ?
        \       reject(s:read(ch, 'err')) :
        \       resolve(s:read(ch, 'out'))
        \   },
        \ })})
endfunction

call s:sh('ls', '-l')
      \.then({out -> execute('echom ' . string('Output: ' . out), '')})
      \.catch({err -> execute('echom ' . string('Error: ' . err), '')})

ちなみに，close_cb は指定しないと job の開始に失敗してしまいました

@rhysd 動きました。ただいくつか修正した方が良さそうな点がありました。

1. open の場合も考慮する
   • ch_status(a:chan, {'part' : a:part}) =~# 'open\|buffered'
2. 末尾に改行入れる
   • let out .= ch_read(a:chan, {'part' : a:part}) . "\n"
3. :echom だと改行を含む文字列の場合つらい感じになるので :echo のがいいかもです (最後の .then() と .catch() の中)

ちなみに，close_cb は指定しないと job の開始に失敗してしまいました

これ自分の環境 (WSL) でも再現しました。詳しく追ってないですが Error: みたいに err が空文字で渡ってきちゃいますね。

Twitter でも言いましたが、このコードだとエラーにならなかったです。
もしかして job 以外の所でコケて .catch() に来てるかも？
https://gist.github.com/tyru/4707726873f3e4955f331109f9dc0734

私の手元で確認した限りでは channel status が fail になっていたので job の開始に失敗しているんじゃないかと思います．他の箇所で例外が起きているなら err が空になるのは（Promise の実装がバグってない限り）おかしいですね

動きました。修正点としては @tyru さんと同意見で、以下がいいかなと思います。

let s:Promise = vital#vital#import('Async.Promise')

function! s:read(ch, part) abort
  let out = []
  while ch_status(a:ch, {'part' : a:part}) =~# '^\%(open\|buffered\)$'
    call add(out, ch_read(a:ch, {'part': a:part}))
  endwhile
  return join(out, "\n")
endfunction

function! s:sh(...) abort
  let cmd = join(a:000, ' ')
  return s:Promise.new({resolve, reject -> job_start(cmd, {
        \   'drop' : 'never',
        \   'close_cb' : {ch -> 'do nothing' },
        \   'exit_cb' : {ch, code ->
        \     code ?
        \       reject(s:read(ch, 'err')) :
        \       resolve(s:read(ch, 'out'))
        \   },
        \ })})
endfunction

call s:sh('ls', '-l')
      \.then({out -> execute('echo ' . string('Output: ' . out), '')})
      \.catch({err -> execute('echo ' . string('Error: ' . err), '')})

あと s:sh('cat', '/foo/bar') でちゃんと Error: cat: /foo/bar: No such file or directory が出ました。

環境: macOS High Sierra

[nit] total じゃなくて remaining にしてデクリメントしていって0ならresolveってすると一つ変数を減らせる説 (total, resolved -> remaining)

(https://github.com/stefanpenner/es6-promise/blob/97478eb6fcd628e0d679438c88bd64b5079a9122/lib/es6-promise/enumerator.js#L95 らへんとかをみた)

あー，確かに．キーの数は減らしたいのでそうしてみます．

[nit] results とかのほうがわかりやすい気がしますがどうでしょう? done って done フラグ感がある(Go脳かもしれない) これもes6-promise では results だった

[nit] get(a:000, 0, v:null) とか他のところで使ってるし書き方統一してもよさそう。

(個人的には get(a:, 1, v:null) が好みだけど統一されてたらなんでもok)

個人的な好みにより a:0 になりました

[感想] 自分がいちばん好みじゃないの来た...! (勿論直してほしいとかではなくて単なる感想で、言い分としてはa:0は考えることが増えるのと(a:0が数でぇ...+1を見に行く...)、ほかの get or default イディオムと合わせたいので自分は get(a:, n, default) を使ってる (g: とかと一緒のイディオムにする)

[optional] call timer_start(0, ...) っていうイディオム関数化しておいてもよさそう? asap とか next_tick とか?
読者が暗黙に timer_start(0,...) がどういう意味かわかってるという仮定をおいてるけど関数化されててコメントもあったりすると可読性よさそう

とても気持ちはわかるのですが

1. ユーザー定義関数のオーバーヘッドが大きい
2. 無駄にコールスタックを積んでしまう

の二点から今のままが良いと思います。

#567 (comment)

と同じで Vim script の事情から本来不要なハックや最適化が必要になるの少しモニョりますが、仕方ない……

その最適化が本当に必要な箇所かどうかが結構あやしいとは思いますが(2はなんかchainしすぎるとダメとか見た気がするのでわからんでもない), まぁoptionalですね。

はい。ここがユーザーが触る場所ならば僕も関数でラップする事には同意なのですが、ライブラリ内部のコードで（個人の主観としては）ほぼイディオム的な使い方なのでラップする必要が薄いと思います。

提案ありがとうございます．個人的な感覚では timer_start(0, ...) のままで良いかなと思います．0秒のタイマーでも意味は通じると思いますし，ラップして関数1つ増やす保守コストに見合うほどではないなと思いました．

JavaScript では確かに asap とか next_tick みたいにラップすることが多いのですが，それは環境によって下が setImmediate() だったり setTimeout() だったりするからなのですよね（使えるなら setImmediate のほうが良いため）

このチェックはfor loop の外で settled 変数に入れたあと or 前くらいがよさそう

publish の呼ばれ方的にこのパスが呼ばれることがない（防衛的に入れてた）ので気づきませんでしたが，確かにそうですね．ループ前にチェックするようにします．

[nit][optional] 実装のほうにも似たコメント書きましたが done より result 感がある?

確かに．フラグをセットするんじゃなくて値をセットしてるとこは result にしといたほうが値の意図が明確で良さそうです

あってるので別によさそうだけど, call s:wait_has_key(l, 'done') してあげないとフェアじゃないかも?
...と思ったけど無駄にunit test が長くなるのもアレだしムズカシイ

(見直すとそもそも wait_has_key は done すでに定義してるので使えなかった.... ので sleep 1ms とかするとフェア感?(本当か?)

ここは待たなくても timer_start でキューイングされた処理は後のどこかで無害な感じで実行されて終わるだけなので，待たなくて良いかなと

あーすみません，このコメントの意図を読み違えてました．.then がタイマーを挟むようになった今，確かにある程度待たないと .then が発火していないことを確認するのにはこれでは不十分でした．ちなみに .then は永遠に発火しないので，done が定義済みかによらず wait_has_key は使えないです．

wait_has_key の前でまだ Done には値入ってないことを確認すると "asynchronously" なことがわかりやすくてよさそう

ここも wait_has_key のまえにチェック入れたほうが asynchronouslyのテストであることがわかりやすくてよさそう?

このテスト catch 使ってないし Describe .then のとこにあったほうがよさそう

これは .then のとこに置くべきなのを間違えてますね…ありがとうございます！