vim-abolish icon indicating copy to clipboard operation
vim-abolish copied to clipboard

Extract code to autoload file similar to the recent commit in fugitive

Open kiryph opened this issue 7 years ago • 3 comments

The fugitive commit is https://github.com/tpope/vim-fugitive/commit/5d11ff75014818da25596a89cea71df680e31788

My current startup log lists

...
064.553  001.091  001.091: sourcing ~/.vim/pack/minpac/start/vim-abolish/plugin/abolish.vim
...
073.387  000.323  000.323: sourcing ~/.vim/pack/minpac/start/vim-fugitive/plugin/fugitive.vim
...

I use abolish less often than fugitive but abolish occupies unnecessarily more startup time.

Any chance you would do this extraction as well?

kiryph avatar Jul 11 '18 10:07 kiryph

I'm open to it but probably won't get around to it myself any time soon. One caveat is that dumb Abolish global variable is a public interface and would need to be preserved or replaced with something else.

tpope avatar Jul 11 '18 17:07 tpope

I have given it a naive shot on my machine. My attempt works for me. I have two questions:

  1. What is the function Abolished() for? I have moved it to autoload as well without adding the prefix abolish#. Does a user might want to call it? The docs do not mention it.

  2. I have no problems with the global dictionary Abolish. As long as I have not used a coercion mapping, Abolish is undefined. When I press e.g. cru to capitalize a word, the file will be autoloaded and the global variable is defined in the right moment. Does a user want to define it in his vimrc? The documentation does not suggest this.

The new startup time for abolish looks nicer:

078.862  000.407  000.407: sourcing ~/.vim/pack/minpac/start/vim-abolish/plugin/abolish.vim

However, I fear this is a little bit of bikeshedding. On the otherside nowadays the number of used plugins increases constantly. If every plugin follows this way, it has an effect. On its own it is not a noticable improvement.

plugin/abolish.vim looks now:

" abolish.vim - Language friendly searches, substitutions, and abbreviations
" Maintainer:   Tim Pope <http://tpo.pe/>
" Version:      1.1
" GetLatestVimScripts: 1545 1 :AutoInstall: abolish.vim

" Initialization {{{1

if exists("g:loaded_abolish") || &cp || v:version < 700
  finish
endif
let g:loaded_abolish = 1

if !exists("g:abolish_save_file")
  if isdirectory(expand("~/.vim"))
    let g:abolish_save_file = expand("~/.vim/after/plugin/abolish.vim")
  elseif isdirectory(expand("~/vimfiles")) || has("win32")
    let g:abolish_save_file = expand("~/vimfiles/after/plugin/abolish.vim")
  else
    let g:abolish_save_file = expand("~/.vim/after/plugin/abolish.vim")
  endif
endif

" }}}1

nnoremap <silent> <Plug>Coerce :<C-U>call abolish#coerce(nr2char(getchar()))<CR>

if !exists("g:abolish_no_mappings") || ! g:abolish_no_mappings
  nmap cr  <Plug>Coerce
endif

command! -nargs=+ -bang -bar -range=0 -complete=custom,abolish#Complete Abolish
      \ :exec abolish#dispatcher(<bang>0,<line1>,<line2>,<count>,[<f-args>])
command! -nargs=1 -bang -bar -range=0 -complete=custom,abolish#SubComplete Subvert
      \ :exec abolish#subvert_dispatcher(<bang>0,<line1>,<line2>,<count>,<q-args>)
if exists(':S') != 2
  command -nargs=1 -bang -bar -range=0 -complete=custom,abolish#SubComplete S
        \ :exec abolish#subvert_dispatcher(<bang>0,<line1>,<line2>,<count>,<q-args>)
endif

" vim:set ft=vim sw=2 sts=2:

Everything else is in autoload/abolish.vim with following functions made available with the prefix abolish#:

...
function! abolish#SubComplete(A,L,P)
...
function! abolish#Complete(A,L,P)
...
function! abolish#dispatcher(bang,line1,line2,count,args)
...
function! abolish#subvert_dispatcher(bang,line1,line2,count,args)
...
function! abolish#coerce(transformation)
...

Anyhow, if you would seriously consider merging it, I will create a PR. If you think, this would need a more advanced refactoring, I have to leave this to you.

kiryph avatar Jul 12 '18 14:07 kiryph

Abolished() is for use in :s/.../\=Abolished() in the generated substitute command. I feel weird having a global function in an autoload file so I guess rename it to abolish#Replacement().

I extend g:Abolish in rails.vim. Other plugins can do the same.

Let's use mixed case for all the global functions. And can you go ahead and add abort to them? (Don't worry about doing this for all functions, just the ones you're adding abolish# to.)

tpope avatar Aug 17 '19 13:08 tpope