vimrc 共有サービス vimrcbox のプラグインを添削した

id:kabiy が vimrcbox なるものを作ったらしい。vimrc を共有するサービス。

vimrcbox をリリースしました。 - cod.note

http://soralabo.net/s/vrcb/

で、vimrcをアップロードするのに vimrcbox.vim というプラグインを使うんだけど、初めての Vim プラグインで添削募集中とのことだったので僭越ながら添削してみる。

とりあえず元ソース

多分今後更新されるので、とりあえず現時点での vimrcbox.vim

" vimrcbox.vim
" Author: Sora harakami <sora134@gmail.com>
" Require: curl
" Licence: MIT Licence

if !exists('g:vimrcbox_user')
    let g:vimrcbox_user = ''
endif

if !exists('g:vimrcbox_pass')
    let g:vimrcbox_pass = ''
endif

if !exists('g:vimrcbox_vimrc')
    let g:vimrcbox_vimrc = ''
endif

if !exists('g:vimrcbox_gvimrc')
    let g:vimrcbox_gvimrc = ''
endif

"change baseurl if debug
"let g:vimrcbox_baseurl = 'http://127.0.0.1/vimrcb/'
let g:vimrcbox_baseurl = 'http://soralabo.net/s/vrcb/'
function! s:VrbUpdateVimrc(...)
    let user = g:vimrcbox_user
    let pass = g:vimrcbox_pass
    if exists('a:1')
        let postfile = a:1
    elseif len(g:vimrcbox_vimrc) > 0
        let postfile = g:vimrcbox_vimrc
    else
        let postfile = $MYVIMRC
    endif
    if user == ''
        let user = input("Username: ")
        if !len(user)
            echo "Cancel"
            return []
        endif
    endif
    if pass == ''
        let pass = inputsecret("Password: ")
        if !len(pass)
            echo "Cancel"
            return []
        endif
    endif
    "post
    let result = system("curl -s -F vimrc=@".postfile." -F user=".user." -F pass=".pass." -F gvim=0 ".g:vimrcbox_baseurl."api/register")
    if result =~ '1'
        echo "Update success"
    else
        echo "Update failure" 
    end
endfunction

function! s:VrbUpdateGvimrc(...)
    let user = g:vimrcbox_user
    let pass = g:vimrcbox_pass
    if exists('a:1')
        let postfile = a:1
    elseif len(g:vimrcbox_gvimrc) > 0
        let postfile = g:vimrcbox_gvimrc
    else
        let postfile = $MYGVIMRC
    endif
    if user == ''
        let user = input("Username: ")
        if !len(user)
            echo "Cancel"
            return []
        endif
    endif
    if pass == ''
        let pass = inputsecret("Password: ")
        if !len(pass)
            echo "Cancel"
            return []
        endif
    endif
    "post
    let result = system("curl -s -F vimrc=@".postfile." -F user=".user." -F pass=".pass." -F gvim=1 ".g:vimrcbox_baseurl."api/register")
    if result =~ '1'
        echo "Update success"
    else
        echo "Update failure" 
    end
endfunction

command! -nargs=? -complete=file RcbVimrc :call s:VrbUpdateVimrc(<f-args>)
command! -nargs=? -complete=file RcbGVimrc :call s:VrbUpdateGvimrc(<f-args>)

添削

多重読み込み防止

基本中の基本。Vim プラグインの先頭には以下のようなものを書いておく。

if exists('g:loaded_vimrcbox')
    finish
endif
let g:loaded_vimrcbox = 1

let がファイルの最後にあったり値としてバージョン番号入れたりすることもあるけどそこは好みで。変数名は g:loaded_{plugin-name} とするのが慣習。
これは単なる多重読み込み防止のほか、 'runtimepath' に同じプラグイン複数あった場合に最初に見つかった物のみ有効にしたり、ユーザが vimrc で事前に変数を定義することで任意のプラグインを無効にしたりするのに使うのでかなり重要。

'cpoption' を初期化

無用なトラブルを避けるために 'cpoption' を初期値にしておくといい。完璧な処置じゃないけどないよりマシなので。

let s:save_cpo = &cpo
set cpo&vim

" ... 中略

let &cpo = s:save_cpo
unlet s:save_cpo
" EOF

上記の多重読み込み防止と合わせてほぼ定型なのでテンプレート化しておくといい。

L24: g:vimrcbox_baseurl

これも他の変数と同様に設定されていない場合のみ初期化の方が良い。そうすればデバッグ用の値は自分の vimrc に書いておけばよいし。

if !exists('g:vimrcbox_baseurl')
    let g:vimrcbox_baseurl = 'http://soralabo.net/s/vrcb/'
endif
L28: exists('a:1')

動作として間違いではないけど、ここは a:0 を使うべきところ。a:0 は可変長引数部分に渡された引数の数が入ってる。

if 0 < a:0
    let postfile = a:1
elseif len(g:vimrcbox_vimrc) > 0
    let postfile = g:vimrcbox_vimrc
else
    let postfile = $MYVIMRC
endif
L37: !len(user)

用途を考えると empty() の方がよい。ただし、とある調査によると空文字列の判定で一番速いのは user == '' だったらしいので速度を気にするならこっちを使う。
…この程度の長さのスクリプトで速さ気にしてもしょうがないけどね。

L39: return []

空配列を返す意味がよくわからない。普通に return でいいかと。

L50: system() で POST

system() に渡す引数は、引数ごとに shellescape() を通す。実際 Windows なんかじゃ $MYVIMRC は C:\Documents and Settings\〜 とかで空白文字を含むので通さないと引数が分割されて POST できない。

ちょっとトリッキーだけど私なら以下のようにするかな。

let result = system('curl -s ' . join(values(map({
    \ 'vimrc': '@' . postfile,
    \ 'user': user,
    \ 'pass': pass,
    \ 'gvim': 0,
    \ }, '"-F " . shellescape(v:key . "=" . v:val)')), ' ')
    \ . ' ' . shellescape(g:vimrcbox_baseurl . 'api/register'))

解説面倒なので頑張って解読してください。質問は受け付けます。

postfile

これは expand() を通しておくと便利。% や ~/ を展開してくれる。ついでにその後 fnamemodify() でフルパスにしておくと確実。
この辺りは好みもあるので絶対ではないけど。

let postfile = fnamemodify(expand(postfile), ':p')
s:VrbUpdateVimrc() と s:VrbUpdateGvimrc()

中身ほぼ同じだから統合した方がいいんじゃないかな。見たところ postfile の初期値と gvim= の値くらいしか違いがない。
と言ってもこれは Vim に限らずプログラミング全般のお話なので敢えてここで解説する必要はないと思われる。

L91: command! RcbVimrc

を使ってるけど、関数の中身見た限りだと受け付ける引数の数は 0 か 1 なので の方がいいと思う。
そうすればそもそも関数の引数を可変長にする必要がなくなる。 なら引数がない時でも空文字列になってくれるので。

以上を踏まえて書き直した

" vimrcbox.vim
" Author: Sora harakami <sora134@gmail.com>
" Modified by: thinca <thinca@gmail.com>
" Require: curl
" Licence: MIT Licence

if exists('g:loaded_vimrcbox')
    finish
endif
let g:loaded_vimrcbox = 1

let s:save_cpo = &cpo
set cpo&vim

if !exists('g:vimrcbox_user')
    let g:vimrcbox_user = ''
endif

if !exists('g:vimrcbox_pass')
    let g:vimrcbox_pass = ''
endif

if !exists('g:vimrcbox_vimrc')
    let g:vimrcbox_vimrc = ''
endif

if !exists('g:vimrcbox_gvimrc')
    let g:vimrcbox_gvimrc = ''
endif

if !exists('g:vimrcbox_baseurl')
    let g:vimrcbox_baseurl = 'http://soralabo.net/s/vrcb/'
endif



function! s:VrbUpdate(postfile, gvim)
    let user = g:vimrcbox_user
    let pass = g:vimrcbox_pass

    let postfile = a:postfile != ''
    \ ? a:postfile
    \ : a:gvim
    \   ? (g:vimrcbox_gvimrc != '' ? g:vimrcbox_gvimrc : $MYGVIMRC)
    \   : (g:vimrcbox_vimrc  != '' ? g:vimrcbox_vimrc  : $MYVIMRC)

    if user == ''
        let user = input("Username: ")
        if empty(user)
            echo "Cancel"
            return
        endif
    endif
    if pass == ''
        let pass = inputsecret("Password: ")
        if empty(pass)
            echo "Cancel"
            return
        endif
    endif
    "post
    let result = system('curl -s ' . join(values(map({
        \ 'vimrc': '@' . fnamemodify(expand(postfile), ':p'),
        \ 'user': user,
        \ 'pass': pass,
        \ 'gvim': a:gvim,
        \ }, '"-F " . shellescape(v:key . "=" . v:val)')), ' ')
        \ . ' ' . shellescape(g:vimrcbox_baseurl . 'api/register'))
    if result =~ '1'
        echo "Update success"
    else
        echo "Update failure"
    end
endfunction

command! -nargs=? -complete=file RcbVimrc  :call s:VrbUpdate(<q-args>, 0)
command! -nargs=? -complete=file RcbGVimrc :call s:VrbUpdate(<q-args>, 1)

let &cpo = s:save_cpo
unlet s:save_cpo

大体こんなもんかなー。

vimrcboxは現状だと単にファイルのホスティングの域を出ていないので今後vimrcに特化した機能が実装されるのを期待。