Ways to improve a function

Question

Perhaps this is more of a 'code-review' question, but I think this is perhaps the best (or only) place to post this question. I've been working over the past few days to understand how vim functions work, etc, and thank you to all of you that have been taken the time on those.

I often will have to convert a list of ids or a key-value pair into a python/json/whatever value so that I can then use it in another script. It's something that I probably do ~5 times or more a day. Here is what I have so far which will take a one-column list of items and convert it into a tuple, or a two-column list of items and convert it into a dict:

function IsNumber(str)
    return match(a:str, '^\d\+$') != -1
endfunction

function FormatInput()

    try
        let example_line = getreg('*', 1, 1)[1]
    catch
        " Note this function is not shown here, but it basically just
        " pipes the output to a file via `redir`
        call LogOutput('Not enough lines -- exiting', "WARNING", {'line': expand('<sflnum>'), 'func': expand('<sfile>')[9:]})
        return
    endtry
    call LogOutput('Example line: ' . example_line, "INFO", {'line': expand('<sflnum>'), 'func': expand('<sfile>')[9:]})

    let terms = split(example_line, '\t')
    let num_terms = len(terms)
    call LogOutput("Num terms parsed: " . num_terms, "INFO", {'line': expand('<sflnum>'), 'func': expand('<sfile>')[9:]})
    if num_terms <= 2

        normal! "*p
        '[,']sort

        " If just one term, convert it to a tuple
        if num_terms == 1

            " Whether we parse the item as a number (no quotes) or as a string
            normal! '[V']J
            if IsNumber(example_line)
                s/ /,/g
                execute "normal! i(\<c-o>$)\<esc>V\"*y"
                call LogOutput("Parsing as num", "INFO", {'line': expand('<sflnum>'), 'func': expand('<sfile>')[9:]})
            else
                s/ /","/g
                execute "normal! i(\"\<c-o>$\")\<esc>V\"*y"
                call LogOutput("Parsing as string", "INFO", {'line': expand('<sflnum>'), 'func': expand('<sfile>')[9:]})
            endif

        " if two terms, parse it into a dict
        " check if number for each term to see whether we need to quote it
        else
            call LogOutput("Parsing as dict", "INFO", {'line': expand('<sflnum>'), 'func': expand('<sfile>')[9:]})
            let [q1, q2, q3, q4] = ['', '', '', '']
            if !IsNumber(terms[0]) | let [q1, q2] = ['"', '"'] | endif
            if !IsNumber(terms[1]) | let [q3, q4] = ['"', '"'] | endif
            let regex = "'[,']" . printf('s/\v([^\t]+)\t(.+)/%s\1%s:%s\2%s,/', q1, q2, q3, q4)
            execute(regex)
            '[,'] join
            execute "normal! i{\<c-o>$}\<esc>V\"*y"
        endif

    else
        call LogOutput("Only one or two terms currently supported.", "INFO", {'line': expand('<sflnum>'), 'func': expand('<sfile>')[9:]})
        return
    endif
endfunc 
nnoremap <silent> # :silent call FormatInput()<CR>

The cool thing is it works! Any feedback on the above or ways that I could improve it would be great -- thank you.

Answer 1

Classical issues, addressed in another Q/A

I could copy paste a few feedbacks I gave in another Q/A: https://vi.stackexchange.com/a/11329/626

• use guards (this used to be used to permit to inhibit a plugin from being loaded, this is less critical with package/plugin managers nowadays)
• don't forget the licence
• avoid global functions -- here everything but the mapping could go in another file: an autoload plugin file
• declare the functions with abort

And I'll add

bang the function declarations

This permits to reload a plugin that is being maintained without having and close and open vim after every change we want to test

See the plugin creation tutorial I've referenced yesterday.

EDIT: My mistake. It's no longer required with recent versions of Vim.

Inputs

I would have like to see transformation examples (before and after)

Also, instead of working on a register, you could work on a range. This is how other mappings and commands works usually.

Simplifications

Instead of match(), I prefer to stay simple: text =~ regex. Now, I've never compared the performances of both.

buffer vs string transformations

Over the time, I've developed a habit of avoiding to directly apply transformations on the buffer if I can. I prefer to work in memory.

My first intuition would be to store the object in the buffer as a string or a dictionary then apply transformations. On this topic, we already have json_encode() and json_decode(). But I'm not sure whether they could be used without an example.