Code-review: persistent tag-stack display

Question

Update: I've created a plugin.

I wrote a bit of code for a friend who's dealing with large C code-bases and using cscope/ctags to navigate around. He was hoping to get a persistent display of the tagstack; here's what I've whipped up that seems mostly working. I would love a review of the code (especially any weird cases I'm missing).

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

function DisplayTagStack(display_id, tagstack_id, timer)
  const [display_tabnr, display_winnr] = win_id2tabwin(a:display_id)
  " don't update when not in the right tab
  if tabpagenr() isnot# display_tabnr
    return
  endif

  " get the tagstack
  const tagsitems = win_execute(a:tagstack_id, 'tags')->split('\n')

  " " if you prefer to have more control over the format, use the below code
  " const tagstack_winnr = win_id2win(a:tagstack_id)
  " const tagstack = gettagstack(tagstack)
  " const tagsindex = tagstack.curidx
  " const tagsitems = tagstack.items
  " " format it…

  " get the buffer to update
  const display_bufnr = winbufnr(display_winnr)

  " delete old content, like %delete
  silent call deletebufline(display_bufnr, 1, "$")
  " add the new content, like put =tagsitems | 1delete
  call appendbufline(display_bufnr, 0, tagsitems)
endfunction

function StartTagStack()
  if exists('w:stack_info')
    return
  endif
  " window id whose tagstack we want to display
  const tagstack_id = win_getid()
  new
  setlocal buftype=nofile
  setlocal bufhidden=hide
  setlocal noswapfile
  setlocal nobuflisted
  " window id where display will live
  const display_id = win_getid()

  let w:stack_info = #{
        \ timer: timer_start(500, function('DisplayTagStack', [display_id, tagstack_id]), #{repeat: -1}),
        \ display_id: display_id
        \ }
  wincmd p
endfunction

function StopTagStack()
  if exists('w:stack_info')
    call timer_stop(w:stack_info.timer)
    execute win_id2win(w:stack_info.display_id) 'close'
    unlet w:stack_info
  endif
endfunction

(No, the function names and the top-level "plugin name" g:loaded_tagstack aren't great, I know.)

Answer 1

Really, your solution seems fine so these are nit picks. However it does use a few techniques that can sometimes cause editor performance issues and "jank" (weird glitches) so I'll mention alternatives here. vim developers have mostly worked out major issues with these over time, but all else equal, it's better to avoid them. These include:

• constantly running timer in the background
• win_execute on the not active window
• using appendbufline on the not active buffer

A few suggestions

• Use autocmds to react to changes in the tagstack instead of the timer. CursorMoved and BufWinEnter might cover it, as most tag operations change the buffer or cursor. You can also intercept tagfunc() but that is not a general solution.
• Don't use win_execute(:tags). This involves switching windows (silently) and a lot of string parsing. gettagstack() would give a lot more control and as :tags isn't the most user-friendly display, a custom rendering might also be nicer.
• Consider using the location list, which can be used for general window-specific lists. appendbufline is still better for maximum control, though.

Answer 2

1. I don't think using timer is a crime per se. But in this case you should check for :h state() and :h b:changedtick to reduce display update.

2. gettagstack() is definitely more preferable than win_execute(...)

3. I guess wincmd p is supposed to go before let w:stack_info = ...

4. appendbufline() leaves extra empty line in buffer; setbufline(buf, 1, [...]) allows to get rid of it.

5. You don't seem to re-use buffer. Why bufhidden=hide and close then? If my brain can debug online, you'll end up having a ton of hidden unused buffers in :ls!