patch 8.0.0736: OptionSet not triggered when entering diff mode

Problem:    The OptionSet autocommand event is not triggered when entering
            diff mode.
Solution:   use set_option_value() instead of setting the option directly.
            Change the tests from old to new style. (Christian Brabandt)

src/Makefile

@@ -2088,7 +2088,6 @@ run_message_test: $(MESSAGE_TEST_TARGET)
 
 # Run individual OLD style test, assuming that Vim was already compiled.
 test1 \
-	test_autocmd_option \
 	test_autoformat_join \
 	test_changelist \
 	test_close_count \

src/diff.c

@@ -1137,6 +1137,20 @@ ex_diffthis(exarg_T *eap UNUSED)
     diff_win_options(curwin, TRUE);
 }
 
+    static void
+set_diff_option(win_T *wp, int value)
+{
+    win_T *old_curwin = curwin;
+
+    curwin = wp;
+    curbuf = curwin->w_buffer;
+    ++curbuf_lock;
+    set_option_value((char_u *)"diff", (long)value, NULL, OPT_LOCAL);
+    --curbuf_lock;
+    curwin = old_curwin;
+    curbuf = curwin->w_buffer;
+}
+
 /*
  * Set options in window "wp" for diff mode.
  */
@@ -1198,10 +1212,10 @@ diff_win_options(
     if (vim_strchr(p_sbo, 'h') == NULL)
 	do_cmdline_cmd((char_u *)"set sbo+=hor");
 #endif
-    /* Saved the current values, to be restored in ex_diffoff(). */
+    /* Save the current values, to be restored in ex_diffoff(). */
     wp->w_p_diff_saved = TRUE;
 
-    wp->w_p_diff = TRUE;
+    set_diff_option(wp, TRUE);
 
     if (addbuf)
 	diff_buf_add(wp->w_buffer);
@@ -1227,7 +1241,7 @@ ex_diffoff(exarg_T *eap)
 	    /* Set 'diff' off. If option values were saved in
 	     * diff_win_options(), restore the ones whose settings seem to have
 	     * been left over from diff mode.  */
-	    wp->w_p_diff = FALSE;
+	    set_diff_option(wp, FALSE);
 
 	    if (wp->w_p_diff_saved)
 	    {

src/testdir/Make_all.mak

@@ -66,7 +66,6 @@ SCRIPTS_ALL = \
 	test104.out \
 	test107.out \
 	test108.out \
-	test_autocmd_option.out \
 	test_autoformat_join.out \
 	test_changelist.out \
 	test_close_count.out \

src/testdir/test_autocmd.vim

@@ -2,13 +2,13 @@
 
 set belloff=all
 
-function! s:cleanup_buffers() abort
+func! s:cleanup_buffers() abort
   for bnr in range(1, bufnr('$'))
     if bufloaded(bnr) && bufnr('%') != bnr
       execute 'bd! ' . bnr
     endif
   endfor
-endfunction
+endfunc
 
 func Test_vim_did_enter()
   call assert_false(v:vim_did_enter)
@@ -49,7 +49,7 @@ if has('timers')
   endfunc
 endif
 
-function Test_bufunload()
+func Test_bufunload()
   augroup test_bufunload_group
     autocmd!
     autocmd BufUnload * call add(s:li, "bufunload")
@@ -80,7 +80,7 @@ function Test_bufunload()
 endfunc
 
 " SEGV occurs in older versions.  (At least 7.4.2005 or older)
-function Test_autocmd_bufunload_with_tabnext()
+func Test_autocmd_bufunload_with_tabnext()
   tabedit
   tabfirst
 
@@ -98,7 +98,7 @@ function Test_autocmd_bufunload_with_tabnext()
   quit
 endfunc
 
-function Test_autocmd_bufwinleave_with_tabfirst()
+func Test_autocmd_bufwinleave_with_tabfirst()
   tabedit
   augroup sample
     autocmd!
@@ -110,7 +110,7 @@ function Test_autocmd_bufwinleave_with_tabfirst()
 endfunc
 
 " SEGV occurs in older versions.  (At least 7.4.2321 or older)
-function Test_autocmd_bufunload_avoiding_SEGV_01()
+func Test_autocmd_bufunload_avoiding_SEGV_01()
   split aa.txt
   let lastbuf = bufnr('$')
 
@@ -128,7 +128,7 @@ function Test_autocmd_bufunload_avoiding_SEGV_01()
 endfunc
 
 " SEGV occurs in older versions.  (At least 7.4.2321 or older)
-function Test_autocmd_bufunload_avoiding_SEGV_02()
+func Test_autocmd_bufunload_avoiding_SEGV_02()
   setlocal buftype=nowrite
   let lastbuf = bufnr('$')
 
@@ -351,7 +351,7 @@ endfunc
 
 " Closing a window might cause an endless loop
 " E814 for older Vims
-function Test_autocmd_bufwipe_in_SessLoadPost()
+func Test_autocmd_bufwipe_in_SessLoadPost()
   tabnew
   set noswapfile
   mksession!
@@ -380,7 +380,7 @@ function Test_autocmd_bufwipe_in_SessLoadPost()
 endfunc
 
 " SEGV occurs in older versions.
-function Test_autocmd_bufwipe_in_SessLoadPost2()
+func Test_autocmd_bufwipe_in_SessLoadPost2()
   tabnew
   set noswapfile
   mksession!
@@ -422,3 +422,194 @@ endfunc
 func Test_empty_doau()
   doau \|
 endfunc
+
+func s:AutoCommandOptionSet(match)
+  let item     = remove(g:options, 0)
+  let expected = printf("Option: <%s>, Oldval: <%s>, NewVal: <%s>, Scope: <%s>\n", item[0], item[1], item[2], item[3])
+  let actual   = printf("Option: <%s>, Oldval: <%s>, NewVal: <%s>, Scope: <%s>\n", a:match, v:option_old, v:option_new, v:option_type)
+  let g:opt    = [expected, actual]
+  "call assert_equal(expected, actual)
+endfunc
+
+func Test_OptionSet()
+  if !has("eval") || !has("autocmd") || !exists("+autochdir")
+    return
+  endif
+
+  call test_override('starting', 1)
+  set nocp
+  au OptionSet * :call s:AutoCommandOptionSet(expand("<amatch>"))
+
+  " 1: Setting number option"
+  let g:options=[['number', 0, 1, 'global']]
+  set nu
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 2: Setting local number option"
+  let g:options=[['number', 1, 0, 'local']]
+  setlocal nonu
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 3: Setting global number option"
+  let g:options=[['number', 1, 0, 'global']]
+  setglobal nonu
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 4: Setting local autoindent option"
+  let g:options=[['autoindent', 0, 1, 'local']]
+  setlocal ai
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 5: Setting global autoindent option"
+  let g:options=[['autoindent', 0, 1, 'global']]
+  setglobal ai
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 6: Setting global autoindent option"
+  let g:options=[['autoindent', 1, 0, 'global']]
+  set ai!
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " Should not print anything, use :noa
+  " 7: don't trigger OptionSet"
+  let g:options=[['invalid', 1, 1, 'invalid']]
+  noa set nonu
+  call assert_equal([['invalid', 1, 1, 'invalid']], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 8: Setting several global list and number option"
+  let g:options=[['list', 0, 1, 'global'], ['number', 0, 1, 'global']]
+  set list nu
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 9: don't trigger OptionSet"
+  let g:options=[['invalid', 1, 1, 'invalid'], ['invalid', 1, 1, 'invalid']]
+  noa set nolist nonu
+  call assert_equal([['invalid', 1, 1, 'invalid'], ['invalid', 1, 1, 'invalid']], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 10: Setting global acd"
+  let g:options=[['autochdir', 0, 1, 'local']]
+  setlocal acd
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 11: Setting global autoread (also sets local value)"
+  let g:options=[['autoread', 0, 1, 'global']]
+  set ar
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 12: Setting local autoread"
+  let g:options=[['autoread', 1, 1, 'local']]
+  setlocal ar
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 13: Setting global autoread"
+  let g:options=[['autoread', 1, 0, 'global']]
+  setglobal invar
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 14: Setting option backspace through :let"
+  let g:options=[['backspace', '', 'eol,indent,start', 'global']]
+  let &bs="eol,indent,start"
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 15: Setting option backspace through setbufvar()"
+  let g:options=[['backup', 0, 1, 'local']]
+  " try twice, first time, shouldn't trigger because option name is invalid,
+  " second time, it should trigger
+  call assert_fails("call setbufvar(1, '&l:bk', 1)", "E355")
+  " should trigger, use correct option name
+  call setbufvar(1, '&backup', 1)
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 16: Setting number option using setwinvar"
+  let g:options=[['number', 0, 1, 'local']]
+  call setwinvar(0, '&number', 1)
+  call assert_equal([], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " 17: Setting key option, shouldn't trigger"
+  let g:options=[['key', 'invalid', 'invalid1', 'invalid']]
+  setlocal key=blah
+  setlocal key=
+  call assert_equal([['key', 'invalid', 'invalid1', 'invalid']], g:options)
+  call assert_equal(g:opt[0], g:opt[1])
+
+  " Cleanup
+  au! OptionSet
+  for opt in ['nu', 'ai', 'acd', 'ar', 'bs', 'backup', 'cul', 'cp']
+    exe printf(":set %s&vi", opt)
+  endfor
+  call test_override('starting', 0)
+  delfunc! AutoCommandOptionSet
+endfunc
+
+func Test_OptionSet_diffmode()
+  call test_override('starting', 1)
+  " 18: Changing an option when enetering diff mode
+  new
+  au OptionSet diff :let &l:cul=v:option_new
+
+  call setline(1, ['buffer 1', 'line2', 'line3', 'line4'])
+  call assert_equal(0, &l:cul)
+  diffthis
+  call assert_equal(1, &l:cul)
+
+  vnew
+  call setline(1, ['buffer 2', 'line 2', 'line 3', 'line4'])
+  call assert_equal(0, &l:cul)
+  diffthis
+  call assert_equal(1, &l:cul)
+
+  diffoff
+  call assert_equal(0, &l:cul)
+  call assert_equal(1, getwinvar(2, '&l:cul'))
+  bw!
+
+  call assert_equal(1, &l:cul)
+  diffoff!
+  call assert_equal(0, &l:cul)
+  call assert_equal(0, getwinvar(1, '&l:cul'))
+  bw!
+
+  " Cleanup
+  au! OptionSet
+  call test_override('starting', 0)
+endfunc
+
+func Test_OptionSet_diffmode_close()
+  call test_override('starting', 1)
+  " 19: Try to close the current window when entering diff mode
+  " should not segfault
+  new
+  au OptionSet diff close
+
+  call setline(1, ['buffer 1', 'line2', 'line3', 'line4'])
+  call assert_fails(':diffthis', 'E788')
+  call assert_equal(1, &diff)
+  vnew
+  call setline(1, ['buffer 2', 'line 2', 'line 3', 'line4'])
+  call assert_fails(':diffthis', 'E788')
+  call assert_equal(1, &diff)
+  bw!
+  call assert_fails(':diffoff!', 'E788')
+  bw!
+
+  " Cleanup
+  au! OptionSet
+  call test_override('starting', 0)
+  "delfunc! AutoCommandOptionSet
+endfunc

src/testdir/test_autocmd_option.in

@@ -1,77 +0,0 @@
-Test for option autocommand
-
-STARTTEST
-:so small.vim
-:if !has("eval") || !has("autocmd") || !exists("+autochdir") | e! test.ok | w! test.out | qa! | endif
-:fu! AutoCommand(match)
-:	let c=g:testcase
-:       let item=remove(g:options, 0)
-:       let c.=printf("Expected: Name: <%s>, Oldval: <%s>, NewVal: <%s>, Scope: <%s>\n", item[0], item[1], item[2], item[3])
-:       let c.=printf("Autocmd Option: <%s>,", a:match)
-:       let c.=printf(" OldVal: <%s>,", v:option_old)
-:       let c.=printf(" NewVal: <%s>,", v:option_new)
-:       let c.=printf(" Scope: <%s>\n", v:option_type)
-:       call setreg('r', printf("%s\n%s", getreg('r'), c))
-:endfu
-:au OptionSet * :call AutoCommand(expand("<amatch>"))
-:let g:testcase="1: Setting number option\n"
-:let g:options=[['number', 0, 1, 'global']]
-:set nu
-:let g:testcase="2: Setting local number option\n"
-:let g:options=[['number', 1, 0, 'local']]
-:setlocal nonu
-:let g:testcase="3: Setting global number option\n"
-:let g:options=[['number', 1, 0, 'global']]
-:setglobal nonu
-:let g:testcase="4: Setting local autoindent option\n"
-:let g:options=[['autoindent', 0, 1, 'local']]
-:setlocal ai
-:let g:testcase="5: Setting global autoindent option\n"
-:let g:options=[['autoindent', 0, 1, 'global']]
-:setglobal ai
-:let g:testcase="6: Setting global autoindent option\n"
-:let g:options=[['autoindent', 1, 0, 'global']]
-:set ai!
-: Should not print anything, use :noa
-:noa :set nonu
-:let g:testcase="7: Setting several global list and number option\n"
-:let g:options=[['list', 0, 1, 'global'], ['number', 0, 1, 'global']]
-:set list nu
-:noa set nolist nonu
-:let g:testcase="8: Setting global acd\n"
-:let g:options=[['autochdir', 0, 1, 'global']]
-:setlocal acd
-:let g:testcase="9: Setting global autoread\n"
-:let g:options=[['autoread', 0, 1, 'global']]
-:set ar
-:let g:testcase="10: Setting local autoread\n"
-:let g:options=[['autoread', 0, 1, 'local']]
-:setlocal ar
-:let g:testcase="11: Setting global autoread\n"
-:let g:options=[['autoread', 1, 0, 'global']]
-:setglobal invar
-:let g:testcase="12: Setting option backspace through :let\n"
-:let g:options=[['backspace', '', 'eol,indent,start', 'global']]
-:let &bs="eol,indent,start"
-:let g:testcase="13: Setting option backspace through setbufvar()\n"
-:let g:options=[['backup', '', '1', 'local']]
-: "try twice, first time, shouldn't trigger because option name is invalid, second time, it should trigger
-:call setbufvar(1, '&l:bk', 1)
-: "should trigger, use correct option name
-:call setbufvar(1, '&backup', 1)
-:let g:testcase="14: Setting number option using setwinvar\n"
-:let g:options=[['number', 0, 1, 'local']]
-:call setwinvar(0, '&number', 1)
-:" Write register now, because next test shouldn't output anything.
-:$put r
-:let @r=''
-:let g:testcase="\n15: Setting key option, shouldn't trigger\n"
-:let g:options=[['key', 'invalid', 'invalid1', 'invalid']]
-:setlocal key=blah
-:setlocal key=
-:$put =g:testcase
-:$put r
-:/^dummy text/,$w! test.out
-:qa!
-ENDTEST
-dummy text

src/testdir/test_autocmd_option.ok

@@ -1,64 +0,0 @@
-dummy text
-
-1: Setting number option
-Expected: Name: <number>, Oldval: <0>, NewVal: <1>, Scope: <global>
-Autocmd Option: <number>, OldVal: <0>, NewVal: <1>, Scope: <global>
-
-2: Setting local number option
-Expected: Name: <number>, Oldval: <1>, NewVal: <0>, Scope: <local>
-Autocmd Option: <number>, OldVal: <1>, NewVal: <0>, Scope: <local>
-
-3: Setting global number option
-Expected: Name: <number>, Oldval: <1>, NewVal: <0>, Scope: <global>
-Autocmd Option: <number>, OldVal: <1>, NewVal: <0>, Scope: <global>
-
-4: Setting local autoindent option
-Expected: Name: <autoindent>, Oldval: <0>, NewVal: <1>, Scope: <local>
-Autocmd Option: <autoindent>, OldVal: <0>, NewVal: <1>, Scope: <local>
-
-5: Setting global autoindent option
-Expected: Name: <autoindent>, Oldval: <0>, NewVal: <1>, Scope: <global>
-Autocmd Option: <autoindent>, OldVal: <0>, NewVal: <1>, Scope: <global>
-
-6: Setting global autoindent option
-Expected: Name: <autoindent>, Oldval: <1>, NewVal: <0>, Scope: <global>
-Autocmd Option: <autoindent>, OldVal: <1>, NewVal: <0>, Scope: <global>
-
-7: Setting several global list and number option
-Expected: Name: <list>, Oldval: <0>, NewVal: <1>, Scope: <global>
-Autocmd Option: <list>, OldVal: <0>, NewVal: <1>, Scope: <global>
-
-7: Setting several global list and number option
-Expected: Name: <number>, Oldval: <0>, NewVal: <1>, Scope: <global>
-Autocmd Option: <number>, OldVal: <0>, NewVal: <1>, Scope: <global>
-
-8: Setting global acd
-Expected: Name: <autochdir>, Oldval: <0>, NewVal: <1>, Scope: <global>
-Autocmd Option: <autochdir>, OldVal: <0>, NewVal: <1>, Scope: <local>
-
-9: Setting global autoread
-Expected: Name: <autoread>, Oldval: <0>, NewVal: <1>, Scope: <global>
-Autocmd Option: <autoread>, OldVal: <0>, NewVal: <1>, Scope: <global>
-
-10: Setting local autoread
-Expected: Name: <autoread>, Oldval: <0>, NewVal: <1>, Scope: <local>
-Autocmd Option: <autoread>, OldVal: <1>, NewVal: <1>, Scope: <local>
-
-11: Setting global autoread
-Expected: Name: <autoread>, Oldval: <1>, NewVal: <0>, Scope: <global>
-Autocmd Option: <autoread>, OldVal: <1>, NewVal: <0>, Scope: <global>
-
-12: Setting option backspace through :let
-Expected: Name: <backspace>, Oldval: <>, NewVal: <eol,indent,start>, Scope: <global>
-Autocmd Option: <backspace>, OldVal: <>, NewVal: <eol,indent,start>, Scope: <global>
-
-13: Setting option backspace through setbufvar()
-Expected: Name: <backup>, Oldval: <>, NewVal: <1>, Scope: <local>
-Autocmd Option: <backup>, OldVal: <0>, NewVal: <1>, Scope: <local>
-
-14: Setting number option using setwinvar
-Expected: Name: <number>, Oldval: <0>, NewVal: <1>, Scope: <local>
-Autocmd Option: <number>, OldVal: <0>, NewVal: <1>, Scope: <local>
-
-15: Setting key option, shouldn't trigger
-

src/version.c

@@ -769,6 +769,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    736,
 /**/
     735,
 /**/