Cycle through a list of letters in multiple groups - Code review

Question

This is a follow up question to Cycle through a list of letters in multiple groups where I asked how to cycle a group of letters. I got a bunch of great tips and this is my follow up question.

This is simply asking for a code review of my attempt at solving this problem

---

Explanation

My code simply allows the user to hit some key combination (temporarily set to <localleader>ln and <localleader>lp) to replace the letter under the cursor with the next letter in the cycle.

---

Here are some of my thoughts on the current state of my code.

My script flashes the contents of the tr command upon execution to the statusline. I tried to avoid this by using ! (silent) without much success.

I tried to make sure I do not mess with the user's registers, however, this solution seems unnecessarily verbose. Any suggestions for improvements here are more than welcome.

s\<c-r>=tr(@\",'" . a:letters . "','" . a:replacements . "')\<cr>\<esc>" executes a command in normal mode, then immediately go to insert mode using s. Then it switches to invoking an (Ex?) command using <C-r>=. Is this really the best way to replace the letter under my cursor by invoking tr over it? It feels as my code jumps between two many modes.

Initially I had some issues with my code, I tried tr(@\"," . a:letters . "," . a:replacements . ") without much success. I had to add ' around the variables to make it work, why? I thought they already were stored as strings?

Can this code be written cleaner/better? This is the first time I've ever written a function in vimscript. I do not know about any style guides, nor how to lint my code. I choose to do this with vimscript, and I'm not really sure why I did that over lua. Alternative approaches are welcome.

Code

function! TryReplaceLetters(letters, replacements)
    let save_cb = &cb
    let regText = getreg('"', 1, 1)
    try
        execute "normal! s\<c-r>=tr(@\",'" . a:letters . "','" . a:replacements . "')\<cr>\<esc>"
    finally
        let &cb = save_cb
        call setreg('"', regText)
    endtry
endfunction 

let s:lookalikes = "1¹2²3³4⁴5⁵6⁶7⁶8⁸9⁹AẠȦÄÆBḄḂƁßCÇĊƇ©DÐḌḊƊEẸĖ£ÆŒFḞƑGĠƓHḤḢIỊİJȷKḲLḶĿMṂṀⱮNṆṄÑƝOȮØŒPṖƤQỌRṚṘ®SṢṠTṬṪƬUỤƲVṾWẈẆXẊYỴẎ¥ZẒŻaạȧæbḅḃþcċç¢dḍḋɗeẹæœfḟƒgḥḣiị¡jkƙlḷŀmṃṁnñoọȯøœpṗÞrṛṙɼsṣṡ$tṭṫƭvʋwẇxẋyżz"
let s:lookcycles = "¹1²2³3⁴4⁵5⁶6⁶7⁸8⁹9ÆAẠȦÄßBḄḂƁ©CÇĊƇƊDÐḌḊŒEẸĖ£ÆƑFḞƓGĠḢHḤİIỊȷJḲKĿLḶⱮMṂṀƝNṆṄÑŒOȮØƤPṖỌQ®RṚṘṠSṢƬTṬṪƲUỤṾVẆWẈẊX¥YỴẎŻZẒæaạȧþbḅḃ¢cċçɗdḍḋœeẹæƒfḟgḣḥ¡iịjƙkŀlḷṁmṃñnœoọȯøÞpṗɼrṛṙ$sṣṡƭtṭṫʋvẇwẋxżyz"

function! JellyLookaLikes()
    " Cycles through similar looking symbols, clockwise
    return TryReplaceLetters(s:lookalikes, s:lookcycles)
endfunction 

function! JellyLookaLikesPrev()
    " Cycles through similar looking symbols, counter-clockwise
    return TryReplaceLetters(s:lookcycles, s:lookalikes)
endfunction 

nnoremap <silent> <localleader>ln :call JellyLookaLikes()<cr><esc>
nnoremap <silent> <localleader>lp :call JellyLookaLikesPrev()<cr><esc>

Answer 1

Style

• You spell out functions/commands/&c., which I think is wonderful. But you didn't spell out &cb. Prefer &clipboard.
• It appears you mixed spaces and tabs. Prefer one or the other (I use 2 spaces for vimscript).
• You're missing abort on your functions.
• function! is not strictly necessary according to :help E127, but this changed—I believe its prevalence is mostly a historical artifact at this point.

Otherwise, stylistically this looks pretty solid. The only other thing I can think of is to use const if you have a new enough vim, and I think execute printf("…", …) looks nicer than string concatenation.

Code

• The duplication in s:lookalikes and s:lookcycles is somewhat troublesome. I would prefer to find a way to write the pairs down once and then programmatically derive the required parts.

• You save and restore the clipboard option, but don't change it. Perhaps you meant to include set clipboard&?

• The clipboard option is only available "in GUI versions or when the |+xterm_clipboard| is included"—you should test for those if you use it.

• For registers, you should use getreginfo()—it saves more information correctly, and the result can be given directly to setreg().

• As written, the quoting nastiness is necessary: tr needs string arguments. If you wrote, for example,

  let source = 'bcd'
  let target = 'abc'
  let a = tr('b', source, target)
  

  then everything is fine. But you're inside an execute concatenating strings. Try

  :echo "tr(@\",'" . source . "','" . target . "')"
  

  to see the command you're building—notice the strings have been concatenated, so the fact that they were string variables isn't seen in the final command. I get

  tr(@",'bcd','abc')
  

  Without the quotes, the command would look for variables bcd and abc.

  But we can avoid jumping through all these hoops. Let's use a simple command to grab the character under the cursor. :normal! yl will do, if you like the register stuff. :help strpart() suggests

  strpart(getline('.'), col('.') - 1, 1, v:true)
  

  to get the character under the cursor (but this might fail for multi-byte characters, such as emoji).

  Either way, we get the character in a variable we can manipulate normally, and then finally do an :execute ':normal! r'.replacement.

Putting it altogether, I would write something like below. I've left the letters alone because, to be honest, it was very difficult to read.

function TryReplaceLetters(letters, replacements) abort
  if has('gui') || has('xterm_clipboard')
    let save_cb = &clipboard
    set clipboard&
  endif
  let save_reg = getreginfo('"')
  try
    normal! yl
    let char_to_change = @"
  finally
    if has('gui') || has('xterm_clipboard')
      let &clipboard = save_cb
    endif
    call setreg('"', save_reg)
  endtry
  let replacement = tr(char_to_change, a:letters, a:replacements)
  " avoid changes if same character
  if replacement isnot# char_to_change
    execute ':normal! r'.replacement
  endif
endfunction

let s:lookalikes = "1¹2²3³4⁴5⁵6⁶7⁶8⁸9⁹AẠȦÄÆBḄḂƁßCÇĊƇ©DÐḌḊƊEẸĖ£ÆŒFḞƑGĠƓHḤḢIỊİJȷKḲLḶĿMṂṀⱮNṆṄÑƝOȮØŒPṖƤQỌRṚṘ®SṢṠTṬṪƬUỤƲVṾWẈẆXẊYỴẎ¥ZẒŻaạȧæbḅḃþcċç¢dḍḋɗeẹæœfḟƒgḥḣiị¡jkƙlḷŀmṃṁnñoọȯøœpṗÞrṛṙɼsṣṡ$tṭṫƭvʋwẇxẋyżz"
let s:lookcycles = "¹1²2³3⁴4⁵5⁶6⁶7⁸8⁹9ÆAẠȦÄßBḄḂƁ©CÇĊƇƊDÐḌḊŒEẸĖ£ÆƑFḞƓGĠḢHḤİIỊȷJḲKĿLḶⱮMṂṀƝNṆṄÑŒOȮØƤPṖỌQ®RṚṘṠSṢƬTṬṪƲUỤṾVẆWẈẊX¥YỴẎŻZẒæaạȧþbḅḃ¢cċçɗdḍḋœeẹæƒfḟgḣḥ¡iịjƙkŀlḷṁmṃñnœoọȯøÞpṗɼrṛṙ$sṣṡƭtṭṫʋvẇwẋxżyz"

function JellyLookaLikes() abort
  " Cycles through similar looking symbols, clockwise
  return TryReplaceLetters(s:lookalikes, s:lookcycles)
endfunction

function JellyLookaLikesPrev() abort
  " Cycles through similar looking symbols, counter-clockwise
  return TryReplaceLetters(s:lookcycles, s:lookalikes)
endfunction

nnoremap <silent> <localleader>ln :call JellyLookaLikes()<cr><esc>
nnoremap <silent> <localleader>lp :call JellyLookaLikesPrev()<cr><esc>

Here's an example of turning a list of cycles into the two arguments necessary: assume each cycle is a list of characters, and cycles is a list of cycles.

function ToTrArgs(cycles) abort
  let first_argument = deepcopy(a:cycles)->flatten()->join('')
  let second_argument = mapnew(a:cycles, {_, v -> [v[-1]]+v[:-2]})->flatten()->join('')
  return [first_argument, second_argument]
endfunction

let [s:forwards, s:backwards] = ToTrArgs([['a','á','à'], ['b', 'ḃ']])

function JellyLookaLikes()
  " Cycles through similar looking symbols, clockwise
  return TryReplaceLetters(s:forwards, s:backwards)
endfunction

function JellyLookaLikesPrev()
  " Cycles through similar looking symbols, counter-clockwise
  return TryReplaceLetters(s:backwards, s:forwards)
endfunction

nnoremap <silent> <localleader>ln :call JellyLookaLikes()<CR>
nnoremap <silent> <localleader>lp :call JellyLookaLikesPrev()<CR>
" alternatively, if you don't like the whole "s:var not valid in mapping" thing,
" use call() with a list argument
nnoremap <silent> <localleader>ln :call call('TryReplaceLetters', ToTrArgs([['a','á','à'], ['b', 'ḃ']]))<CR>
nnoremap <silent> <localleader>lp :call call('TryReplaceLetters', ToTrArgs([['a','á','à'], ['b', 'ḃ']])->reverse())<CR>