To: vim_dev@googlegroups.com Subject: Patch 8.1.0669 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.1.0669 Problem: The ex_sign() function is too long. Solution: Refactor the function. Add a bit more testing. (Yegappan Lakshmanan, closes #3745) Files: src/testdir/test_signs.vim, src/ex_cmds.c *** ../vim-8.1.0668/src/testdir/test_signs.vim 2018-12-29 21:00:20.953498877 +0100 --- src/testdir/test_signs.vim 2018-12-31 21:51:47.815155503 +0100 *************** *** 104,109 **** --- 104,119 ---- 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,136 **** --- 141,158 ---- 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,246 **** --- 263,276 ---- 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:') *** ../vim-8.1.0668/src/ex_cmds.c 2018-12-29 18:53:07.843607433 +0100 --- src/ex_cmds.c 2018-12-31 21:57:40.300234709 +0100 *************** *** 7897,7902 **** --- 7897,7905 ---- 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,7937 **** --- 7935,8283 ---- } /* + * 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,7949 **** sign_T *sp; buf_T *buf = NULL; ! /* Parse the subcommand. */ p = skiptowhite(arg); idx = sign_cmd_idx(arg, p); if (idx == SIGNCMD_LAST) --- 8289,8295 ---- sign_T *sp; buf_T *buf = NULL; ! // Parse the subcommand. p = skiptowhite(arg); idx = sign_cmd_idx(arg, p); if (idx == SIGNCMD_LAST) *************** *** 7955,7966 **** if (idx <= SIGNCMD_LIST) { ! /* ! * Define, undefine or list signs. ! */ if (idx == SIGNCMD_LIST && *arg == NUL) { ! /* ":sign list": list all defined signs */ for (sp = first_sign; sp != NULL && !got_int; sp = sp->sn_next) sign_list_defined(sp); } --- 8301,8310 ---- if (idx <= SIGNCMD_LIST) { ! // Define, undefine or list signs. if (idx == SIGNCMD_LIST && *arg == NUL) { ! // ":sign list": list all defined signs for (sp = first_sign; sp != NULL && !got_int; sp = sp->sn_next) sign_list_defined(sp); } *************** *** 7969,7981 **** 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". */ p = skiptowhite(arg); if (*p != NUL) *p++ = NUL; --- 8313,8321 ---- else { char_u *name; ! // 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,8042 **** 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); ! } else if (idx == SIGNCMD_LIST) ! /* ":sign list {name}" */ sign_list_by_name(name); else ! /* ":sign undefine {name}" */ sign_undefine_by_name(name); vim_free(name); --- 8324,8335 ---- name = vim_strsave(arg); if (idx == SIGNCMD_DEFINE) ! sign_define_cmd(name, p); else if (idx == SIGNCMD_LIST) ! // ":sign list {name}" sign_list_by_name(name); else ! // ":sign undefine {name}" sign_undefine_by_name(name); vim_free(name); *************** *** 8050,8279 **** 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); ! 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); ! } ! 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)); } } --- 8343,8360 ---- char_u *sign_name = NULL; char_u *group = NULL; int prio = SIGN_DEF_PRIO; ! // Parse command line arguments ! if (parse_sign_cmd_args(idx, arg, &sign_name, &id, &group, &prio, ! &buf, &lnum) == FAIL) return; ! 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_cmd(buf, lnum, sign_name, id, group); } } *** ../vim-8.1.0668/src/version.c 2018-12-31 21:02:58.334464236 +0100 --- src/version.c 2018-12-31 21:54:07.105999672 +0100 *************** *** 801,802 **** --- 801,804 ---- { /* Add new patch number below this line */ + /**/ + 669, /**/ -- From "know your smileys": (:-# Said something he shouldn't have /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///