Review: git branch in statusline

Question

Note: this is an educational exercise. For those wanting a ready-made solution, use vim-gitbranch.

I am learning vimscript, and the code below is part of my .vimrc used to display the current git branch in the statusline. I am aware that there are ready-made options out there, but this is more of an educational exercise. The purpose of this question is to solicit advice on improving the code.

The only arbitrary requirement of this exercise is that the git branch has to be obtained using a system call (instead of directly through the .git directory).

function! StatuslineGitBranch()
    if exists("g:git_branch")
        return g:git_branch
    else
        return ''
    endif
endfunction

function! GetGitBranch()
    let l:is_git_dir = system('echo -n $(git rev-parse --is-inside-work-tree)')
    let g:git_branch = l:is_git_dir == 'true' ?
        \ system('bash -c "echo -n $(git rev-parse --abbrev-ref HEAD)"') : ''
endfunction

autocmd BufEnter * call GetGitBranch()

set laststatus=2
set statusline=
set statusline+=%{StatuslineGitBranch()}

Reason for the existence of g:git_branch, and the slightly convoluted execution: so that statusline does not involve a system call (reference).

How can the above be improved? I'm new to this, so comments will be very helpful.

Answer 1

The most general improvements I can give are to avoid the long ?: and to make use of the get function on dictionaries.

For example, I would write

return get(g:, 'git_branch', '')

For the if, use a plain if

let l:is_git_dir = trim(system('git rev-parse --in-inside-work-tree'))
if l:is_git_dir is# 'true'
  let g:git_branch = trim(system('git rev-parse --abbrev-ref HEAD'))
else
  let g:git_branch = ''
endif

(Note that I've upgraded your test to use ==# for casing and the system call to be more portable: system() already uses 'shell', so no need to wrap that. Invoke git more or less directly. Also, echo -n is unreliable, and there is never a reason to echo $(command) when you could just command.)

Or, encode your data as data:

let l:is_git_dir = trim(system('git rev-parse --in-inside-work-tree'))
let g:git_branch = get(
        \ { 'true': trim(system('git rev-parse --abbrev-ref HEAD')) },
        \ l:is_git_dir,
        \ '')

Another note is that you're performing two system calls on every BufEnter; this may or may not be acceptable in terms of performance (measure!!!).

Finally, I'll add that your best bet is to dig into the ever-popular fugitive plugin and try to understand how its statusline function is constructed.

Answer 2

0- +1 to D. Ben Knoble suggestions.

---

Regarding pure vim-scripting

1- Never define autocommands globally. Prefer to define them in their own group, and when filling the group, start by clearing it. This way, the related snippet of code can be re-executed, which you'll will want when you'll be working on your script

aug MyGitBranch
  au!
  au BufEnter * call s:GetGitBranch()
aug END

2- Avoid globally defined functions. Global functions may be defined/changed/removed by any other piece of code. Either define them as script local functions (with the s: prefix as I've done above), or as autoloaded functions if you need to export them (i.e. if you're defining a library plugin), or if they can be lazily loaded.

Regarding the specific case of statusline, I don't remember if we can use script-local functions.

3- Also, it's better to annotate functions with the abort keyword. In case an error is triggered (even if this is impossible on the current code, problems may be introduced on different flavours of Vim (the suggested trim() is quite recent for instance), or in later versions of your plugin.

abort ensure the function doesn't continue (this should be the default, but this is not how Vim used to work, and backward compatibility has been chosen here), and it'll also help decoding possible error messages.

4- I'd use finddir() to recursively find the upstream .git/ directory instead of using system(). On user saturated machines, it can make the difference -- I've experienced such shared environments, and I've come to the conclusion it's always better to reduce the dependence of the system. Also, on Windows machines, calls to system() are really annoying and inefficient.

---

Otherwise, I not sure we can really avoid checking the current branch every time we switch the current buffer. If a git checkout is operated, we'll want to know the branch has changed. I don't know if there is a smart way to do it.