patch 8.1.2035: recognizing octal numbers is confusing

Problem:    Recognizing octal numbers is confusing.
Solution:   Introduce scriptversion 4: do not use octal and allow for single
            quote inside numbers.

Author: brammool

runtime/doc/eval.txt

@@ -1,4 +1,4 @@
-*eval.txt*	For Vim version 8.1.  Last change: 2019 Sep 10
+*eval.txt*	For Vim version 8.1.  Last change: 2019 Sep 15
 
 
 		  VIM REFERENCE MANUAL	  by Bram Moolenaar
@@ -92,7 +92,8 @@ the Number.  Examples:
 							*octal*
 Conversion from a String to a Number is done by converting the first digits to
 a number.  Hexadecimal "0xf9", Octal "017", and Binary "0b10" numbers are
-recognized.  If the String doesn't start with digits, the result is zero.
+recognized (NOTE: when using |scriptversion-4| octal is not recognized).  If
+the String doesn't start with digits, the result is zero.
 Examples:
 	String "456"	-->	Number 456 ~
 	String "6bar"	-->	Number 6 ~
@@ -2757,7 +2758,8 @@ sqrt({expr})			Float	square root of {expr}
 str2float({expr})		Float	convert String to Float
 str2list({expr} [, {utf8}])	List	convert each character of {expr} to
 					ASCII/UTF8 value
-str2nr({expr} [, {base}])	Number	convert String to Number
+str2nr({expr} [, {base} [, {quoted}]])
+				Number	convert String to Number
 strchars({expr} [, {skipcc}])	Number	character length of the String {expr}
 strcharpart({str}, {start} [, {len}])
 				String	{len} characters of {str} at {start}
@@ -9075,9 +9077,11 @@ str2list({expr} [, {utf8}])				*str2list()*
 			GetString()->str2list()
 
 
-str2nr({expr} [, {base}])				*str2nr()*
+str2nr({expr} [, {base} [, {quoted}]])				*str2nr()*
 		Convert string {expr} to a number.
 		{base} is the conversion base, it can be 2, 8, 10 or 16.
+		When {quoted} is present and non-zero then embedded single
+		quotes are ignored, thus "1'000'000" is a million.
 
 		When {base} is omitted base 10 is used.  This also means that
 		a leading zero doesn't cause octal conversion to be used, as
@@ -12937,6 +12941,23 @@ instead of failing in mysterious ways.
 
 	Test for support with: >
 		has('vimscript-3')
+<
+							*scriptversion-4*  >
+ :scriptversion 4
+<	Numbers with a leading zero are not recognized as octal.  With the
+	previous version you get: >
+		echo 017   " displays 15
+		echo 018   " displays 18
+<	with script version 4: >
+		echo 017   " displays 17
+		echo 018   " displays 18
+<	Also, it is possible to use single quotes inside numbers to make them
+	easier to read: >
+		echo 1'000'000
+<	The quotes must be surrounded by digits.
+
+	Test for support with: >
+		has('vimscript-4')
 
 ==============================================================================
 11. No +eval feature				*no-eval-feature*

src/eval.c

@@ -2617,7 +2617,9 @@ eval7(
 		else
 		{
 		    // decimal, hex or octal number
-		    vim_str2nr(*arg, NULL, &len, STR2NR_ALL, &n, NULL, 0, TRUE);
+		    vim_str2nr(*arg, NULL, &len, current_sctx.sc_version >= 4
+				  ? STR2NR_NO_OCT + STR2NR_QUOTE
+				  : STR2NR_ALL, &n, NULL, 0, TRUE);
 		    if (len == 0)
 		    {
 			semsg(_(e_invexpr2), *arg);

src/evalfunc.c

@@ -728,7 +728,7 @@ static funcentry_T global_functions[] =
     {"str2float",	1, 1, FEARG_1,	  f_str2float},
 #endif
     {"str2list",	1, 2, FEARG_1,	  f_str2list},
-    {"str2nr",		1, 2, FEARG_1,	  f_str2nr},
+    {"str2nr",		1, 3, FEARG_1,	  f_str2nr},
     {"strcharpart",	2, 3, FEARG_1,	  f_strcharpart},
     {"strchars",	1, 2, FEARG_1,	  f_strchars},
     {"strdisplaywidth",	1, 2, FEARG_1,	  f_strdisplaywidth},
@@ -7323,7 +7323,7 @@ f_str2nr(typval_T *argvars, typval_T *rettv)
     int		base = 10;
     char_u	*p;
     varnumber_T	n;
-    int		what;
+    int		what = 0;
     int		isneg;
 
     if (argvars[1].v_type != VAR_UNKNOWN)
@@ -7334,6 +7334,8 @@ f_str2nr(typval_T *argvars, typval_T *rettv)
 	    emsg(_(e_invarg));
 	    return;
 	}
+	if (argvars[2].v_type != VAR_UNKNOWN && tv_get_number(&argvars[2]))
+	    what |= STR2NR_QUOTE;
     }
 
     p = skipwhite(tv_get_string(&argvars[0]));
@@ -7342,10 +7344,9 @@ f_str2nr(typval_T *argvars, typval_T *rettv)
 	p = skipwhite(p + 1);
     switch (base)
     {
-	case 2: what = STR2NR_BIN + STR2NR_FORCE; break;
-	case 8: what = STR2NR_OCT + STR2NR_FORCE; break;
-	case 16: what = STR2NR_HEX + STR2NR_FORCE; break;
-	default: what = 0;
+	case 2: what |= STR2NR_BIN + STR2NR_FORCE; break;
+	case 8: what |= STR2NR_OCT + STR2NR_FORCE; break;
+	case 16: what |= STR2NR_HEX + STR2NR_FORCE; break;
     }
     vim_str2nr(p, NULL, NULL, what, &n, NULL, 0, FALSE);
     // Text after the number is silently ignored.

src/scriptfile.c

@@ -1659,7 +1659,7 @@ ex_scriptversion(exarg_T *eap UNUSED)
     nr = getdigits(&eap->arg);
     if (nr == 0 || *eap->arg != NUL)
 	emsg(_(e_invarg));
-    else if (nr > 3)
+    else if (nr > 4)
 	semsg(_("E999: scriptversion not supported: %d"), nr);
     else
 	current_sctx.sc_version = nr;

src/testdir/test_eval_stuff.vim

@@ -74,7 +74,7 @@ func Test_readfile_binary()
   new
   call setline(1, ['one', 'two', 'three'])
   setlocal ff=dos
-  write XReadfile
+  silent write XReadfile
   let lines = 'XReadfile'->readfile()
   call assert_equal(['one', 'two', 'three'], lines)
   let lines = readfile('XReadfile', '', 2)
@@ -124,6 +124,15 @@ func Test_string_concatenation()
   call assert_equal('ab', a)
 endfunc
 
+" Test fix for issue #4507
+func Test_skip_after_throw()
+  try
+    throw 'something'
+    let x = wincol() || &ts
+  catch /something/
+  endtry
+endfunc
+
 scriptversion 2
 func Test_string_concat_scriptversion2()
   call assert_true(has('vimscript-2'))
@@ -183,17 +192,23 @@ func Test_dict_access_scriptversion2()
   call assert_true(1 && l:x.foo)
 endfunc
 
-func Test_scriptversion()
+scriptversion 4
+func Test_vvar_scriptversion4()
+  call assert_equal(17, 017)
+  call assert_equal(18, 018)
+  call assert_equal(64, 0b1'00'00'00)
+  call assert_equal(1048576, 0x10'00'00)
+  call assert_equal(1000000, 1'000'000)
+endfunc
+
+scriptversion 1
+func Test_vvar_scriptversion1()
+  call assert_equal(15, 017)
+  call assert_equal(18, 018)
+endfunc
+
+func Test_scriptversion_fail()
   call writefile(['scriptversion 9'], 'Xversionscript')
   call assert_fails('source Xversionscript', 'E999:')
   call delete('Xversionscript')
 endfunc
-
-" Test fix for issue #4507
-func Test_skip_after_throw()
-  try
-    throw 'something'
-    let x = wincol() || &ts
-  catch /something/
-  endtry
-endfunc

src/testdir/test_functions.vim

@@ -157,6 +157,12 @@ func Test_str2nr()
   call assert_equal(11259375, str2nr('0XABCDEF', 16))
   call assert_equal(-11259375, str2nr('-0xABCDEF', 16))
 
+  call assert_equal(1, str2nr("1'000'000", 10, 0))
+  call assert_equal(256, str2nr("1'0000'0000", 2, 1))
+  call assert_equal(262144, str2nr("1'000'000", 8, 1))
+  call assert_equal(1000000, str2nr("1'000'000", 10, 1))
+  call assert_equal(65536, str2nr("1'00'00", 16, 1))
+
   call assert_equal(0, str2nr('0x10'))
   call assert_equal(0, str2nr('0b10'))
   call assert_equal(1, str2nr('12', 2))

src/version.c

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

src/vim.h

@@ -307,11 +307,15 @@
 #define NUMBUFLEN 65
 
 // flags for vim_str2nr()
-#define STR2NR_BIN 1
-#define STR2NR_OCT 2
-#define STR2NR_HEX 4
+#define STR2NR_BIN 0x01
+#define STR2NR_OCT 0x02
+#define STR2NR_HEX 0x04
 #define STR2NR_ALL (STR2NR_BIN + STR2NR_OCT + STR2NR_HEX)
-#define STR2NR_FORCE 8 // only when ONE of the above is used
+#define STR2NR_NO_OCT (STR2NR_BIN + STR2NR_HEX)
+
+#define STR2NR_FORCE 0x80   // only when ONE of the above is used
+
+#define STR2NR_QUOTE 0x10   // ignore embedded single quotes
 
 /*
  * Shorthand for unsigned variables. Many systems, but not all, have u_char