Merge branch 'feature/vim-plug-review' into develop

* feature/vim-plug-review:
  Use strlen() instead of len()
  Escape option value assign correctly
  Extend toggle_option_flag.vim for string flags
  Rename l:op to l:operation for clarity
  Separate command building from execution in toggle
  Use idiomatic VimL regex for param validation
  Restore some judicious \m\C hedging in Vim
  Remove redundant has('eval') VimL test
  Caution about :execute not eval() in VimL comments
  Move pipes from end to start of continued lines
  Remove \m\C prefix from inapplicable VimL regexes
  Don't use VimL ==# for number comparisons
  Don't overwrite plugin-specified user commands
  Make background detection return not set value
  Complete ToggleOptionFlag commands with opt names

10 files changed, 69 insertions, 50 deletions

diff --git a/vim/autoload/detect_background.vim b/vim/autoload/detect_background.vim
index e4fee199..c010dc53 100644
--- a/vim/autoload/detect_background.vim
+++ b/vim/autoload/detect_background.vim
@@ -3,6 +3,9 @@
 " light backgrounds; we'll default to choosing a dark background unless we
 " find some reason *not* to.
 "
+" Return the string to which we think the option should be set, to allow the
+" caller to use it as they see fit.
+"
 " Author: Tom Ryder <tom@sanctum.geek.nz>
 " License: Same as Vim itself
 "
@@ -25,11 +28,11 @@ function! detect_background#DetectBackground() abort
 
   " Choose the background setting based on this value
   if l:bg ==# 'default'
-        \ || l:bg ==# '7'
-        \ || l:bg ==# '15'
-      set background=light
+        \ || l:bg == 7
+        \ || l:bg == 15
+      return 'light'
   else
-      set background=dark
+      return 'dark'
   endif
 
 endfunction
diff --git a/vim/config/format.vim b/vim/config/format.vim
index 54d46dc2..1f24ee56 100644
--- a/vim/config/format.vim
+++ b/vim/config/format.vim
@@ -5,7 +5,7 @@ if has('eval')
   " Figure out if we have the 'j' flag for 'formatoptions', to automatically
   " delete comment leaders when joining lines; keep it in a script variable
   let s:formatoptions_has_j = v:version > 703
-        \ || v:version ==# 703 && has('patch541')
+        \ || v:version == 703 && has('patch541')
 
   " If we do have 'j', default to setting it
   if s:formatoptions_has_j
@@ -38,7 +38,7 @@ if has('eval')
   " 'textwidth' wrapping to the current paragraph on every insertion or
   " deletion; keep in a script variable
   let s:formatoptions_has_a = v:version > 610
-        \ || v:version ==# 610 && has('patch142')
+        \ || v:version == 610 && has('patch142')
 
   " 'a' needs testing
   if s:formatoptions_has_a
diff --git a/vim/config/substitution.vim b/vim/config/substitution.vim
index 415665ef..982d25f3 100644
--- a/vim/config/substitution.vim
+++ b/vim/config/substitution.vim
@@ -5,7 +5,7 @@ nnoremap <silent>
       \ :<C-U>&&<CR>
 
 " Same again for visual mode; we use vnoremap rather than xnoremap to stay
-" compatible with old Vims without doing eval() dances
+" compatible with old Vims without doing :execute dances
 vnoremap <silent>
       \ &
       \ :<C-U>&&<CR>
diff --git a/vim/config/syntax.vim b/vim/config/syntax.vim
index 8cb1228b..b493c0a9 100644
--- a/vim/config/syntax.vim
+++ b/vim/config/syntax.vim
@@ -5,9 +5,11 @@ if has('syntax')
   silent! syntax enable
   silent! syntax sync minlines=100
 
-  " If we can, detect a light background, but default to a dark one
-  if has('eval') && v:version >= 701
-    silent! call detect_background#DetectBackground()
+  " If we can, detect a light background, but default to a dark one. This is
+  " only because it's more likely the author of this configuration will be
+  " using one.
+  if v:version >= 701
+    silent! let &background = detect_background#DetectBackground()
   else
     set background=dark
   endif
diff --git a/vim/doc/toggle_option_flag.txt b/vim/doc/toggle_option_flag.txt
index 16557d5c..9415ae6b 100644
--- a/vim/doc/toggle_option_flag.txt
+++ b/vim/doc/toggle_option_flag.txt
@@ -1,14 +1,15 @@
-*toggle_option_flag.txt*  Commands to toggle single-character option flags
+*toggle_option_flag.txt*  Commands to toggle option flags
 
 Author:  Tom Ryder <tom@sanctum.geek.nz>
 License: Same terms as Vim itself (see |license|)
 
 This plugin provides commands :ToggleOptionFlag and :ToggleOptionFlagLocal to
 toggle the values of options like |'formatoptions'| or |'complete'| that have
-values comprised of single-character flags. The author originally designed it
-for toggling flags in |'formatoptions'| quickly.
+values comprised of single-character or comma-separated flags. The author
+originally designed it for toggling flags in |'formatoptions'| quickly.
 
     :ToggleOptionFlag formatoptions a
+    :ToggleOptionFlag switchbuf useopen
     :ToggleOptionFlagLocal shortmess I
 
 This plugin lives in Tom Ryder's "dotfiles" suite, and will eventually be spun
diff --git a/vim/ftdetect/sh.vim b/vim/ftdetect/sh.vim
index 3df9c3ce..f00a5659 100644
--- a/vim/ftdetect/sh.vim
+++ b/vim/ftdetect/sh.vim
@@ -5,19 +5,19 @@ augroup dotfiles_ftdetect_sh
   " Names/paths of things that are Bash shell script
   autocmd BufNewFile,BufRead
         \ **/.dotfiles/bash/**,bash-fc-*
-        \ let b:is_bash = 1 |
-        \ setfiletype sh
+        \ let b:is_bash = 1
+        \ | setfiletype sh
 
   " Names/paths of things that are Korn shell script
   autocmd BufNewFile,BufRead
         \ **/.dotfiles/ksh/**,.kshrc,*.ksh
-        \ let b:is_kornshell = 1 |
-        \ setfiletype sh
+        \ let b:is_kornshell = 1
+        \ | setfiletype sh
 
   " Names/paths of things that are POSIX shell script
   autocmd BufNewFile,BufRead
         \ **/.dotfiles/sh/**,.shinit,.shrc,.xinitrc,/etc/default/*
-        \ let b:is_posix = 1 |
-        \ setfiletype sh
+        \ let b:is_posix = 1
+        \ | setfiletype sh
 
 augroup END
diff --git a/vim/plugin/command_typos.vim b/vim/plugin/command_typos.vim
index 16ba654d..adf2d0eb 100644
--- a/vim/plugin/command_typos.vim
+++ b/vim/plugin/command_typos.vim
@@ -14,30 +14,30 @@ endif
 let g:loaded_command_typos = 1
 
 " Define commands
-command! -bang -complete=file -nargs=?
+command -bang -complete=file -nargs=?
       \ E
       \ edit<bang> <args>
-command! -bang -complete=file -nargs=?
+command -bang -complete=file -nargs=?
       \ W
       \ write<bang> <args>
-command! -bang -complete=file -nargs=?
+command -bang -complete=file -nargs=?
       \ WQ
       \ wq<bang> <args>
-command! -bang -complete=file -nargs=?
+command -bang -complete=file -nargs=?
       \ Wq
       \ wq<bang> <args>
-command! -bang
+command -bang
       \ Q
       \ quit<bang>
-command! -bang
+command -bang
       \ Qa
       \ qall<bang>
-command! -bang
+command -bang
       \ QA
       \ qall<bang>
-command! -bang
+command -bang
       \ Wa
       \ wall<bang>
-command! -bang
+command -bang
       \ WA
       \ wa<bang>
diff --git a/vim/plugin/strip_trailing_whitespace.vim b/vim/plugin/strip_trailing_whitespace.vim
index ceeaec7c..d5f5624a 100644
--- a/vim/plugin/strip_trailing_whitespace.vim
+++ b/vim/plugin/strip_trailing_whitespace.vim
@@ -34,7 +34,7 @@ function! s:StripTrailingWhitespace()
 
     " If this line has any non-whitespace characters on it, update l:lw with
     " its index
-    if l:line =~# '\m\C\S'
+    if l:line =~# '\m\S'
       let l:lw = l:li
     endif
 
diff --git a/vim/plugin/toggle_option_flag.vim b/vim/plugin/toggle_option_flag.vim
index ad89d080..da9a6110 100644
--- a/vim/plugin/toggle_option_flag.vim
+++ b/vim/plugin/toggle_option_flag.vim
@@ -1,8 +1,6 @@
 "
-" toggle_option_flag.vim: Provide commands to toggle flags in single-char
-" grouped options like 'formatoptions', 'shortmess', 'complete' etc.
-"
-" This will fail hilariously if you try to set e.g. 'switchbuf' with it!
+" toggle_option_flag.vim: Provide commands to toggle flags in grouped options
+" like 'formatoptions', 'shortmess', 'complete', 'switchbuf', etc.
 "
 " Author: Tom Ryder <tom@sanctum.geek.nz>
 " License: Same as Vim itself
@@ -17,38 +15,53 @@ let g:loaded_toggle_option_flag = 1
 " Internal function to do the toggling
 function! s:Toggle(option, flag, local)
 
-  " Check for weird options, we don't want to eval() anything funny
-  if a:option =~# '[^a-z]'
+  " Check for weird options, we don't want to :execute anything funny
+  if a:option =~# '\m\L'
     echoerr 'Illegal option name'
     return
   endif
 
-  " Weird flags, too; should be a single inoffensive char
-  if a:flag !~# '^[a-z0-9.]$'
-    echoerr 'Illegal flag'
-    return
-  endif
-
   " Choose which set command to use
   let l:set = a:local
         \ ? 'setlocal'
         \ : 'set'
 
-  " eval() to assign -= or += to l:op for the option toggle
+  " Make a flag pattern to allow us to search for the literal string with no
+  " regular expression devilry at all
+  let l:flag_pattern = escape(a:flag, '\')
+
+  " Horrible :execute to get the option's current current into a variable
   " (I couldn't get {curly braces} indirection to work)
-  let l:op = ''
-  execute 'let l:op = &' . a:option . ' =~# a:flag ? "-=" : "+="'
+  let l:current = ''
+  execute 'let l:current = &' . a:option
+
+  " If the flag we're toggling is longer than one character, this must by
+  " necessity be a delimited option. I think all of those in VimL are
+  " comma-separated. Extend the pattern and current setting so that they'll
+  " still match at the start and end.
+  if strlen(a:flag) > 1
+    let l:flag_pattern = ',' . l:flag_pattern . ','
+    let l:current = ',' . l:current . ','
+  endif
+
+  " Assign -= or += as the operation to run based on whether the flag already
+  " appears in the option value or not
+  let l:operation = l:current =~# '\V\C' . l:flag_pattern ? '-=' : '+='
+
+  " Build the command strings to set and then show the value
+  let l:cmd_set = l:set . ' ' . a:option . l:operation . escape(a:flag, '\ ')
+  let l:cmd_show = l:set . ' ' . a:option . '?'
 
-  " eval() to perform the option toggle and then print the value
-  execute l:set . ' ' . a:option . l:op . a:flag
-  execute l:set . ' ' . a:option . '?'
+  " Run the set and show command strings
+  execute l:cmd_set
+  execute l:cmd_show
 
 endfunction
 
 " User commands wrapping around calls to the above function
-command! -nargs=+
+command -nargs=+ -complete=option
       \ ToggleOptionFlag
       \ call <SID>Toggle(<f-args>, 0)
-command! -nargs=+
+command -nargs=+ -complete=option
       \ ToggleOptionFlagLocal
       \ call <SID>Toggle(<f-args>, 1)
diff --git a/vim/vimrc b/vim/vimrc
index 1730edd2..eab24946 100644
--- a/vim/vimrc
+++ b/vim/vimrc
@@ -16,7 +16,7 @@ if v:version >= 701
   " The 'sahara' colorscheme only works for dark backgrounds with 256 colors
   if has('syntax')
         \ && &background ==# 'dark'
-        \ && (has('gui_running') || &t_Co ==# 256)
+        \ && (has('gui_running') || &t_Co == 256)
     silent! colorscheme sahara
   endif
 endif