Mercurial > vim
changeset 15322:c6c306c5027f v8.1.0669
patch 8.1.0669: the ex_sign() function is too long
commit https://github.com/vim/vim/commit/a355652ea5b0c1633e8126ad9af2d970e05f4e1a
Author: Bram Moolenaar <Bram@vim.org>
Date: Mon Dec 31 22:02:29 2018 +0100
patch 8.1.0669: the ex_sign() function is too long
Problem: The ex_sign() function is too long.
Solution: Refactor the function. Add a bit more testing. (Yegappan
Lakshmanan, closes #3745)
| author | Bram Moolenaar <Bram@vim.org> |
|---|---|
| date | Mon, 31 Dec 2018 22:15:06 +0100 |
| parents | baa9f456f02a |
| children | 5e421ec0b6ae |
| files | src/ex_cmds.c src/testdir/test_signs.vim src/version.c |
| diffstat | 3 files changed, 397 insertions(+), 284 deletions(-) [+] |
line wrap: on
line diff
--- a/src/ex_cmds.c +++ b/src/ex_cmds.c @@ -7897,6 +7897,9 @@ sign_place( int sign_unplace(int sign_id, char_u *sign_group, buf_T *buf, linenr_T atlnum) { + if (buf->b_signlist == NULL) // No signs in the buffer + return OK; + if (sign_id == 0) { // Delete all the signs in the specified buffer @@ -7932,6 +7935,349 @@ sign_unplace_at_cursor(char_u *groupname } /* + * sign define command + * ":sign define {name} ..." + */ + static void +sign_define_cmd(char_u *sign_name, char_u *cmdline) +{ + char_u *arg; + char_u *p = cmdline; + char_u *icon = NULL; + char_u *text = NULL; + char_u *linehl = NULL; + char_u *texthl = NULL; + int failed = FALSE; + + // set values for a defined sign. + for (;;) + { + arg = skipwhite(p); + if (*arg == NUL) + break; + p = skiptowhite_esc(arg); + if (STRNCMP(arg, "icon=", 5) == 0) + { + arg += 5; + icon = vim_strnsave(arg, (int)(p - arg)); + } + else if (STRNCMP(arg, "text=", 5) == 0) + { + arg += 5; + text = vim_strnsave(arg, (int)(p - arg)); + } + else if (STRNCMP(arg, "linehl=", 7) == 0) + { + arg += 7; + linehl = vim_strnsave(arg, (int)(p - arg)); + } + else if (STRNCMP(arg, "texthl=", 7) == 0) + { + arg += 7; + texthl = vim_strnsave(arg, (int)(p - arg)); + } + else + { + EMSG2(_(e_invarg2), arg); + failed = TRUE; + break; + } + } + + if (!failed) + sign_define_by_name(sign_name, icon, linehl, text, texthl); + + vim_free(icon); + vim_free(text); + vim_free(linehl); + vim_free(texthl); +} + +/* + * :sign place command + */ + static void +sign_place_cmd( + buf_T *buf, + linenr_T lnum, + char_u *sign_name, + int id, + char_u *group, + int prio) +{ + if (id <= 0) + { + // List signs placed in a file/buffer + // :sign place file={fname} + // :sign place group={group} file={fname} + // :sign place group=* file={fname} + // :sign place buffer={nr} + // :sign place group={group} buffer={nr} + // :sign place group=* buffer={nr} + // :sign place + // :sign place group={group} + // :sign place group=* + if (lnum >= 0 || sign_name != NULL || + (group != NULL && *group == '\0')) + EMSG(_(e_invarg)); + else + sign_list_placed(buf, group); + } + else + { + // Place a new sign + if (sign_name == NULL || buf == NULL || + (group != NULL && *group == '\0')) + { + EMSG(_(e_invarg)); + return; + } + + sign_place(&id, group, sign_name, buf, lnum, prio); + } +} + +/* + * :sign unplace command + */ + static void +sign_unplace_cmd( + buf_T *buf, + linenr_T lnum, + char_u *sign_name, + int id, + char_u *group) +{ + if (lnum >= 0 || sign_name != NULL || (group != NULL && *group == '\0')) + { + EMSG(_(e_invarg)); + return; + } + + if (id == -2) + { + if (buf != NULL) + // :sign unplace * file={fname} + // :sign unplace * group={group} file={fname} + // :sign unplace * group=* file={fname} + // :sign unplace * buffer={nr} + // :sign unplace * group={group} buffer={nr} + // :sign unplace * group=* buffer={nr} + sign_unplace(0, group, buf, 0); + else + // :sign unplace * + // :sign unplace * group={group} + // :sign unplace * group=* + FOR_ALL_BUFFERS(buf) + if (buf->b_signlist != NULL) + buf_delete_signs(buf, group); + } + else + { + if (buf != NULL) + // :sign unplace {id} file={fname} + // :sign unplace {id} group={group} file={fname} + // :sign unplace {id} group=* file={fname} + // :sign unplace {id} buffer={nr} + // :sign unplace {id} group={group} buffer={nr} + // :sign unplace {id} group=* buffer={nr} + sign_unplace(id, group, buf, 0); + else + { + if (id == -1) + { + // :sign unplace group={group} + // :sign unplace group=* + sign_unplace_at_cursor(group); + } + else + { + // :sign unplace {id} + // :sign unplace {id} group={group} + // :sign unplace {id} group=* + FOR_ALL_BUFFERS(buf) + sign_unplace(id, group, buf, 0); + } + } + } +} + +/* + * Jump to a placed sign + * :sign jump {id} file={fname} + * :sign jump {id} buffer={nr} + * :sign jump {id} group={group} file={fname} + * :sign jump {id} group={group} buffer={nr} + */ + static void +sign_jump_cmd( + buf_T *buf, + linenr_T lnum, + char_u *sign_name, + int id, + char_u *group) +{ + if (buf == NULL && sign_name == NULL && group == NULL && id == -1) + { + EMSG(_(e_argreq)); + return; + } + + if (buf == NULL || (group != NULL && *group == '\0') || + lnum >= 0 || sign_name != NULL) + { + // File or buffer is not specified or an empty group is used + // or a line number or a sign name is specified. + EMSG(_(e_invarg)); + return; + } + + if ((lnum = buf_findsign(buf, id, group)) <= 0) + { + EMSGN(_("E157: Invalid sign ID: %ld"), id); + return; + } + + // goto a sign ... + if (buf_jump_open_win(buf) != NULL) + { // ... in a current window + curwin->w_cursor.lnum = lnum; + check_cursor_lnum(); + beginline(BL_WHITE); + } + else + { // ... not currently in a window + char_u *cmd; + + if (buf->b_fname == NULL) + { + EMSG(_("E934: Cannot jump to a buffer that does not have a name")); + return; + } + cmd = alloc((unsigned)STRLEN(buf->b_fname) + 25); + if (cmd == NULL) + return; + sprintf((char *)cmd, "e +%ld %s", (long)lnum, buf->b_fname); + do_cmdline_cmd(cmd); + vim_free(cmd); + } +# ifdef FEAT_FOLDING + foldOpenCursor(); +# endif +} + +/* + * Parse the command line arguments for the ":sign place", ":sign unplace" and + * ":sign jump" commands. + * The supported arguments are: line={lnum} name={name} group={group} + * priority={prio} and file={fname} or buffer={nr}. + */ + static int +parse_sign_cmd_args( + int cmd, + char_u *arg, + char_u **sign_name, + int *signid, + char_u **group, + int *prio, + buf_T **buf, + linenr_T *lnum) +{ + char_u *arg1; + char_u *name; + char_u *filename = NULL; + + // first arg could be placed sign id + arg1 = arg; + if (VIM_ISDIGIT(*arg)) + { + *signid = getdigits(&arg); + if (!VIM_ISWHITE(*arg) && *arg != NUL) + { + *signid = -1; + arg = arg1; + } + else + arg = skipwhite(arg); + } + + while (*arg != NUL) + { + if (STRNCMP(arg, "line=", 5) == 0) + { + arg += 5; + *lnum = atoi((char *)arg); + arg = skiptowhite(arg); + } + else if (STRNCMP(arg, "*", 1) == 0 && cmd == SIGNCMD_UNPLACE) + { + if (*signid != -1) + { + EMSG(_(e_invarg)); + return FAIL; + } + *signid = -2; + arg = skiptowhite(arg + 1); + } + else if (STRNCMP(arg, "name=", 5) == 0) + { + arg += 5; + name = arg; + arg = skiptowhite(arg); + if (*arg != NUL) + *arg++ = NUL; + while (name[0] == '0' && name[1] != NUL) + ++name; + *sign_name = name; + } + else if (STRNCMP(arg, "group=", 6) == 0) + { + arg += 6; + *group = arg; + arg = skiptowhite(arg); + if (*arg != NUL) + *arg++ = NUL; + } + else if (STRNCMP(arg, "priority=", 9) == 0) + { + arg += 9; + *prio = atoi((char *)arg); + arg = skiptowhite(arg); + } + else if (STRNCMP(arg, "file=", 5) == 0) + { + arg += 5; + filename = arg; + *buf = buflist_findname_exp(arg); + break; + } + else if (STRNCMP(arg, "buffer=", 7) == 0) + { + arg += 7; + filename = arg; + *buf = buflist_findnr((int)getdigits(&arg)); + if (*skipwhite(arg) != NUL) + EMSG(_(e_trailing)); + break; + } + else + { + EMSG(_(e_invarg)); + return FAIL; + } + arg = skipwhite(arg); + } + + if (filename != NULL && *buf == NULL) + { + EMSG2(_("E158: Invalid buffer name: %s"), filename); + return FAIL; + } + + return OK; +} + +/* * ":sign" command */ void @@ -7943,7 +8289,7 @@ ex_sign(exarg_T *eap) sign_T *sp; buf_T *buf = NULL; - /* Parse the subcommand. */ + // Parse the subcommand. p = skiptowhite(arg); idx = sign_cmd_idx(arg, p); if (idx == SIGNCMD_LAST) @@ -7955,12 +8301,10 @@ ex_sign(exarg_T *eap) if (idx <= SIGNCMD_LIST) { - /* - * Define, undefine or list signs. - */ + // Define, undefine or list signs. if (idx == SIGNCMD_LIST && *arg == NUL) { - /* ":sign list": list all defined signs */ + // ":sign list": list all defined signs for (sp = first_sign; sp != NULL && !got_int; sp = sp->sn_next) sign_list_defined(sp); } @@ -7969,13 +8313,9 @@ ex_sign(exarg_T *eap) else { char_u *name; - char_u *icon = NULL; - char_u *text = NULL; - char_u *linehl = NULL; - char_u *texthl = NULL; - - /* Isolate the sign name. If it's a number skip leading zeroes, - * so that "099" and "99" are the same sign. But keep "0". */ + + // Isolate the sign name. If it's a number skip leading zeroes, + // so that "099" and "99" are the same sign. But keep "0". p = skiptowhite(arg); if (*p != NUL) *p++ = NUL; @@ -7984,59 +8324,12 @@ ex_sign(exarg_T *eap) name = vim_strsave(arg); if (idx == SIGNCMD_DEFINE) - { - int failed = FALSE; - - /* ":sign define {name} ...": define a sign */ - - /* set values for a defined sign. */ - for (;;) - { - arg = skipwhite(p); - if (*arg == NUL) - break; - p = skiptowhite_esc(arg); - if (STRNCMP(arg, "icon=", 5) == 0) - { - arg += 5; - icon = vim_strnsave(arg, (int)(p - arg)); - } - else if (STRNCMP(arg, "text=", 5) == 0) - { - arg += 5; - text = vim_strnsave(arg, (int)(p - arg)); - } - else if (STRNCMP(arg, "linehl=", 7) == 0) - { - arg += 7; - linehl = vim_strnsave(arg, (int)(p - arg)); - } - else if (STRNCMP(arg, "texthl=", 7) == 0) - { - arg += 7; - texthl = vim_strnsave(arg, (int)(p - arg)); - } - else - { - EMSG2(_(e_invarg2), arg); - failed = TRUE; - break; - } - } - - if (!failed) - sign_define_by_name(name, icon, linehl, text, texthl); - - vim_free(icon); - vim_free(text); - vim_free(linehl); - vim_free(texthl); - } + sign_define_cmd(name, p); else if (idx == SIGNCMD_LIST) - /* ":sign list {name}" */ + // ":sign list {name}" sign_list_by_name(name); else - /* ":sign undefine {name}" */ + // ":sign undefine {name}" sign_undefine_by_name(name); vim_free(name); @@ -8050,230 +8343,18 @@ ex_sign(exarg_T *eap) char_u *sign_name = NULL; char_u *group = NULL; int prio = SIGN_DEF_PRIO; - char_u *arg1; - int bufarg = FALSE; - - if (*arg == NUL) - { - if (idx == SIGNCMD_PLACE) - { - /* ":sign place": list placed signs in all buffers */ - sign_list_placed(NULL, NULL); - } - else if (idx == SIGNCMD_UNPLACE) - /* ":sign unplace": remove placed sign at cursor */ - sign_unplace_at_cursor(NULL); - else - EMSG(_(e_argreq)); - return; - } - - if (idx == SIGNCMD_UNPLACE && arg[0] == '*' && arg[1] == NUL) - { - /* ":sign unplace *": remove all placed signs */ - buf_delete_all_signs(NULL); + + // Parse command line arguments + if (parse_sign_cmd_args(idx, arg, &sign_name, &id, &group, &prio, + &buf, &lnum) == FAIL) return; - } - - /* first arg could be placed sign id */ - arg1 = arg; - if (VIM_ISDIGIT(*arg)) - { - id = getdigits(&arg); - if (!VIM_ISWHITE(*arg) && *arg != NUL) - { - id = -1; - arg = arg1; - } - else - { - arg = skipwhite(arg); - if (idx == SIGNCMD_UNPLACE && *arg == NUL) - { - /* ":sign unplace {id}": remove placed sign by number */ - FOR_ALL_BUFFERS(buf) - sign_unplace(id, NULL, buf, 0); - return; - } - } - } - - /* - * Check for line={lnum} name={name} group={group} priority={prio} - * and file={fname} or buffer={nr}. Leave "arg" pointing to {fname}. - */ - while (*arg != NUL) - { - if (STRNCMP(arg, "line=", 5) == 0) - { - arg += 5; - lnum = atoi((char *)arg); - arg = skiptowhite(arg); - } - else if (STRNCMP(arg, "*", 1) == 0 && idx == SIGNCMD_UNPLACE) - { - if (id != -1) - { - EMSG(_(e_invarg)); - return; - } - id = -2; - arg = skiptowhite(arg + 1); - } - else if (STRNCMP(arg, "name=", 5) == 0) - { - arg += 5; - sign_name = arg; - arg = skiptowhite(arg); - if (*arg != NUL) - *arg++ = NUL; - while (sign_name[0] == '0' && sign_name[1] != NUL) - ++sign_name; - } - else if (STRNCMP(arg, "group=", 6) == 0) - { - arg += 6; - group = arg; - arg = skiptowhite(arg); - if (*arg != NUL) - *arg++ = NUL; - } - else if (STRNCMP(arg, "priority=", 9) == 0) - { - arg += 9; - prio = atoi((char *)arg); - arg = skiptowhite(arg); - } - else if (STRNCMP(arg, "file=", 5) == 0) - { - arg += 5; - buf = buflist_findname_exp(arg); - bufarg = TRUE; - break; - } - else if (STRNCMP(arg, "buffer=", 7) == 0) - { - arg += 7; - buf = buflist_findnr((int)getdigits(&arg)); - if (*skipwhite(arg) != NUL) - EMSG(_(e_trailing)); - bufarg = TRUE; - break; - } - else - { - EMSG(_(e_invarg)); - return; - } - arg = skipwhite(arg); - } - - if ((!bufarg && group == NULL) || (group != NULL && *group == '\0')) - { - // File or buffer is not specified or an empty group is used - EMSG(_(e_invarg)); - return; - } - - if (bufarg && buf == NULL) - { - EMSG2(_("E158: Invalid buffer name: %s"), arg); - } - else if (id <= 0 && idx == SIGNCMD_PLACE) - { - if ((group == NULL) && (lnum >= 0 || sign_name != NULL)) - EMSG(_(e_invarg)); - else - // ":sign place file={fname}": list placed signs in one file - // ":sign place group={group} file={fname}" - // ":sign place group=* file={fname}" - sign_list_placed(buf, group); - } + + if (idx == SIGNCMD_PLACE) + sign_place_cmd(buf, lnum, sign_name, id, group, prio); + else if (idx == SIGNCMD_UNPLACE) + sign_unplace_cmd(buf, lnum, sign_name, id, group); else if (idx == SIGNCMD_JUMP) - { - // ":sign jump {id} file={fname}" - // ":sign jump {id} group={group} file={fname}" - if (lnum >= 0 || sign_name != NULL || buf == NULL) - EMSG(_(e_invarg)); - else if ((lnum = buf_findsign(buf, id, group)) > 0) - { /* goto a sign ... */ - if (buf_jump_open_win(buf) != NULL) - { /* ... in a current window */ - curwin->w_cursor.lnum = lnum; - check_cursor_lnum(); - beginline(BL_WHITE); - } - else - { /* ... not currently in a window */ - char_u *cmd; - - if (buf->b_fname == NULL) - { - EMSG(_("E934: Cannot jump to a buffer that does not have a name")); - return; - } - cmd = alloc((unsigned)STRLEN(buf->b_fname) + 25); - if (cmd == NULL) - return; - sprintf((char *)cmd, "e +%ld %s", (long)lnum, buf->b_fname); - do_cmdline_cmd(cmd); - vim_free(cmd); - } -# ifdef FEAT_FOLDING - foldOpenCursor(); -# endif - } - else - EMSGN(_("E157: Invalid sign ID: %ld"), id); - } - else if (idx == SIGNCMD_UNPLACE) - { - if (lnum >= 0 || sign_name != NULL) - EMSG(_(e_invarg)); - else if (id == -2) - { - if (buf != NULL) - // ":sign unplace * file={fname}" - sign_unplace(0, group, buf, 0); - else - // ":sign unplace * group=*": remove all placed signs - FOR_ALL_BUFFERS(buf) - if (buf->b_signlist != NULL) - buf_delete_signs(buf, group); - } - else - { - if (buf != NULL) - // ":sign unplace {id} file={fname}" - // ":sign unplace {id} group={group} file={fname}" - // ":sign unplace {id} group=* file={fname}" - sign_unplace(id, group, buf, 0); - else - { - if (id == -1) - { - // ":sign unplace group={group}": - // ":sign unplace group=*": - // remove sign in the current line in specified group - sign_unplace_at_cursor(group); - } - else - { - // ":sign unplace {id} group={group}": - // ":sign unplace {id} group=*": - // remove all placed signs in this group. - FOR_ALL_BUFFERS(buf) - if (buf->b_signlist != NULL) - sign_unplace(id, group, buf, 0); - } - } - } - } - /* idx == SIGNCMD_PLACE */ - else if (sign_name != NULL && buf != NULL) - sign_place(&id, group, sign_name, buf, lnum, prio); - else - EMSG(_(e_invarg)); + sign_jump_cmd(buf, lnum, sign_name, id, group); } }
--- a/src/testdir/test_signs.vim +++ b/src/testdir/test_signs.vim @@ -104,6 +104,16 @@ func Test_sign() exe 'sign jump 43 file=' . fn call assert_equal('B', getline('.')) + " Check for jumping to a sign in a hidden buffer + enew! | only! + edit foo + call setline(1, ['A', 'B', 'C', 'D']) + let fn = expand('%:p') + exe 'sign place 21 line=3 name=Sign3 file=' . fn + hide edit bar + exe 'sign jump 21 file=' . fn + call assert_equal('C', getline('.')) + " can't define a sign with a non-printable character as text call assert_fails("sign define Sign4 text=\e linehl=Comment", 'E239:') call assert_fails("sign define Sign4 text=a\e linehl=Comment", 'E239:') @@ -131,6 +141,18 @@ func Test_sign() sign define Sign4 text=\\ linehl=Comment sign undefine Sign4 + " define a sign with a leading 0 in the name + sign unplace * + sign define 004 text=#> linehl=Comment + let a = execute('sign list 4') + call assert_equal("\nsign 4 text=#> linehl=Comment", a) + exe 'sign place 20 line=3 name=004 buffer=' . bufnr('') + let a = execute('sign place') + call assert_equal("\n--- Signs ---\nSigns for foo:\n line=3 id=20 name=4 priority=10\n", a) + exe 'sign unplace 20 buffer=' . bufnr('') + sign undefine 004 + call assert_fails('sign list 4', 'E155:') + " Error cases call assert_fails("sign place abc line=3 name=Sign1 buffer=" . \ bufnr('%'), 'E474:') @@ -241,6 +263,14 @@ func Test_sign_invalid_commands() call assert_fails('sign undefine', 'E156:') call assert_fails('sign list xxx', 'E155:') call assert_fails('sign place 1 buffer=999', 'E158:') + call assert_fails('sign place 1 name=Sign1 buffer=999', 'E158:') + call assert_fails('sign place buffer=999', 'E158:') + call assert_fails('sign jump buffer=999', 'E158:') + call assert_fails('sign jump 1 file=', 'E158:') + call assert_fails('sign jump 1 group=', 'E474:') + call assert_fails('sign jump 1 name=', 'E474:') + call assert_fails('sign jump 1 name=Sign1', 'E474:') + call assert_fails('sign jump 1 line=100', '474:') call assert_fails('sign define Sign2 text=', 'E239:') " Non-numeric identifier for :sign place call assert_fails("sign place abc line=3 name=Sign1 buffer=" . bufnr('%'), 'E474:')