Hi Bram and List,
2015-12-17(Thu) 23:04:20 UTC+9 Bram Moolenaar:
> Hirohito Higashi wrote:
>
> > 2015-12-17(Thu) 21:30:43 UTC+9 Bram Moolenaar:
> > > Hirohito Higashi wrote:
> > >
> > > > I was refactoring the 'backspace' option of implementation.
> > > >
> > > > The following patch was incomplete.
> > > > (It's my first created and contributed patch! Four years ago...)
> > > >
> > > > Patch 7.3.354
> > > > Problem: ":set backspace+=eol" doesn't work when 'backspace' has a
> > > > backwards compatible value of 2.
> > > > Solution: Convert the number to a string. (Hirohito Higashi)
> > > > Files: src/option.c
> > > > https://groups.google.com/d/topic/vim_dev/F3uFnrdYO3g/discussion
> > > >
> > > >
> > > > A new patch has unified 'p_bs' variable handling to a string. So,
> > > > remove the numerical reference process.
> > >
> > > I don't think this works, using:
> > > :set bs=2
> > > Still results in the "2" value.
> >
> > Ah, I knew it. Probably I think that it be good when we unify the Vim
> > internal management of the string, I was sent this patch.
> > The reason for using the word refactoring you might've confused you.
> >
> > Before this patch:
> > :set bs=2
> > :set bs?
> > backspace=2
> >
> > After this patch:
> > :set bs=2
> > :set bs?
> > backspace=indent,eol,start
> >
> > I think this is no problem :-)
>
> Hmm, I'm not sure if that is better. It's confusing if the user is used
> to the old behavior. In cases like this I rather leave it alone.
I think, user would not confused because it is documented.
:h 'backspace'
snip...
doc> For backwards compatibility with version 5.4 and earlier:
doc> value effect
doc> 0 same as ":set backspace=" (Vi compatible)
doc> 1 same as ":set backspace=indent,eol"
doc> 2 same as ":set backspace=indent,eol,start"
snip...
>
> > > When refactoring it's always good to first have a test, so we can check
> > > if the behavior before and after is the same. There is some testing of
> > > 'bs' in test29, but not much.
> >
> > Also test38.
> > Do I need send a modified document and add a test?
>
> More tests is always better. Please do use the new style of tests, and
> add it to testdir/test_alot.vim
Yes sir!
I add a new style test.
Please check attached patch.
--
Best regards,
Hirohito Higashi (a.k.a h_east)
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
diff --git a/src/option.c b/src/option.c
index 0c38de6..71f9610 100644
--- a/src/option.c
+++ b/src/option.c
@@ -4706,25 +4706,28 @@ do_set(arg, opt_flags)
* adding, prepending and removing string.
*/
else if (varp == (char_u *)&p_bs
- && VIM_ISDIGIT(**(char_u **)varp))
+ && VIM_ISDIGIT(*arg))
{
- i = getdigits((char_u **)varp);
- switch (i)
+ if (*arg > '2' || arg[1] != NUL)
{
- case 0:
- *(char_u **)varp = empty_option;
- break;
- case 1:
- *(char_u **)varp = vim_strsave(
- (char_u *)"indent,eol");
- break;
- case 2:
- *(char_u **)varp = vim_strsave(
- (char_u *)"indent,eol,start");
- break;
+ errmsg = e_invarg;
+ goto skip;
+ }
+ *errbuf = NUL;
+ i = getdigits(&arg);
+ if (i >= 1)
+ {
+ STRCAT(errbuf, p_bs_values[0]);
+ STRCAT(errbuf, ",");
+ STRCAT(errbuf, p_bs_values[1]);
+ }
+ if (i == 2)
+ {
+ STRCAT(errbuf, ",");
+ STRCAT(errbuf, p_bs_values[2]);
}
- vim_free(oldval);
- oldval = *(char_u **)varp;
+ save_arg = arg;
+ arg = errbuf;
}
/*
* Convert 'whichwrap' number to string, for
@@ -7041,12 +7044,7 @@ did_set_string_option(opt_idx, varp, new_value_alloced, oldval, errbuf,
/* 'backspace' */
else if (varp == &p_bs)
{
- if (VIM_ISDIGIT(*p_bs))
- {
- if (*p_bs >'2' || p_bs[1] != NUL)
- errmsg = e_invarg;
- }
- else if (check_opt_strings(p_bs, p_bs_values, TRUE) != OK)
+ if (check_opt_strings(p_bs, p_bs_values, TRUE) != OK)
errmsg = e_invarg;
}
else if (varp == &p_bo)
@@ -12096,12 +12094,6 @@ check_opt_wim()
can_bs(what)
int what; /* BS_INDENT, BS_EOL or BS_START */
{
- switch (*p_bs)
- {
- case '2': return TRUE;
- case '1': return (what != BS_START);
- case '0': return FALSE;
- }
return vim_strchr(p_bs, what) != NULL;
}
diff --git a/src/testdir/test_alot.vim b/src/testdir/test_alot.vim
index 21a5241..437aec1 100644
--- a/src/testdir/test_alot.vim
+++ b/src/testdir/test_alot.vim
@@ -4,3 +4,4 @@
source test_lispwords.vim
source test_sort.vim
source test_undolevels.vim
+source test_backsoace_opt.vim
diff --git a/src/testdir/test_backsoace_opt.vim b/src/testdir/test_backsoace_opt.vim
new file mode 100644
index 0000000..53d5565
--- /dev/null
+++ b/src/testdir/test_backsoace_opt.vim
@@ -0,0 +1,58 @@
+" Tests for 'backspace' settings
+
+:func Exec(expr)
+ let str=''
+ try
+ exec a:expr
+ catch /.*/
+ let str=v:exception
+ endtry
+ return str
+:endfunc
+
+func Test_backspace_option()
+ set backspace=
+ call assert_equal('', &backspace)
+ set backspace=indent
+ call assert_equal('indent', &backspace)
+ set backspace=eol
+ call assert_equal('eol', &backspace)
+ set backspace=start
+ call assert_equal('start', &backspace)
+ " Add the value
+ set backspace=
+ set backspace=indent
+ call assert_equal('indent', &backspace)
+ set backspace+=eol
+ call assert_equal('indent,eol', &backspace)
+ set backspace+=start
+ call assert_equal('indent,eol,start', &backspace)
+ " Delete the value
+ set backspace-=indent
+ call assert_equal('eol,start', &backspace)
+ set backspace-=start
+ call assert_equal('eol', &backspace)
+ set backspace-=eol
+ call assert_equal('', &backspace)
+ " Check the error
+ call assert_false(match(Exec('set backspace=ABC'), '.*E474'))
+ call assert_false(match(Exec('set backspace+=def'), '.*E474'))
+ " NOTE: This is not an error. Vim does not check.
+ call assert_true(match(Exec('set backspace-=ghi'), '.*E474'))
+
+ " Check backwards compatibility with version 5.4 and earlier
+ set backspace=0
+ call assert_equal('', &backspace)
+ set backspace=1
+ call assert_equal('indent,eol', &backspace)
+ set backspace=2
+ call assert_equal('indent,eol,start', &backspace)
+ call assert_false(match(Exec('set backspace=3'), '.*E474'))
+ call assert_false(match(Exec('set backspace=10'), '.*E474'))
+
+ " Cleared when 'compatible' is set
+ set compatible
+ call assert_equal('', &backspace)
+endfunc
+
+" vim: tabstop=2 shiftwidth=0 expandtab
