tani / vim-jetpack

The lightning-fast plugin manager, alternative to vim-plug
https://gist.asciidoctor.org/?github-tani/vim-jetpack/main//README.adoc&source-highlighter=highlightjs
MIT License
317 stars 31 forks source link

Refactor #88

Closed uga-rosa closed 1 year ago

uga-rosa commented 1 year ago

準備できてます

tani commented 1 year ago

お返事が遅くなりすみません。https://github.com/tani/vim-jetpack/pull/88/commits/95b7c9473147ea2ea70556d16329393dc46d28eb はとてもクールな変更で、取り込むことに賛成します。

一方で https://github.com/tani/vim-jetpack/pull/88/commits/9e1dc11a38597a7445f343cb0c28f4dca2d0226d を取り込むことには、あまり納得できていません。 たしかに require でコケるのは問題ですが, これは testだけで発生する現象ではなく、初回インストール時にも 同様の問題が発生するはずです。そのため引数変数名が is_test になっているのは違和感があります。

また、場当たり的にケース回避のために引数を増やすのも、将来を考えると負債になると思われるため、避けたいです。 たとえば、大域変数 g:jetpack_skip_config とか、如何でしょうか?

uga-rosa commented 1 year ago

packerではconfigの中身でpcallするくらいしか対策なくて、ここをユーザーに触らせるつもりはありませんでした。 ただ確かに引数でやるのもよろしくなさそうなので大域変数使う方針にしましょうか。

tani commented 1 year ago

ここをユーザーに触らせるつもりはありませんでした。

事情は理解します。ありがとうございます。 では、Undocumented な形でお願いします。 ただ、僕自身は物忘れが多いので、コメントとして変数の意図を書いておいてくださると嬉しいです。

uga-rosa commented 1 year ago

これでどうでしょう

tani commented 1 year ago

Perfect! ありがとうございます。:beers: