patch 8.0.1752: qf_set_properties() is to long

Problem:    qf_set_properties() is to long.
Solution:   Refactor the function.  Define INVALID_QFIDX. (Yegappan
            Lakshmanan, closes #2812)

diff --git a/src/quickfix.c b/src/quickfix.c
index 6142825e3be09e3dbdde235e372aa865d51aed0f..c4b63879341fbf7da6ebaf8a641f17962b5de5b7 100644 (file)
--- a/src/quickfix.c
+++ b/src/quickfix.c
@@ -46,6 +46,7 @@ struct qfline_S
  * There is a stack of error lists.
  */
 #define LISTCOUNT   10
+#define INVALID_QFIDX (-1)
 
 /*
  * Quickfix/Location list definition
@@ -5085,7 +5086,7 @@ qf_getprop_qfidx(qf_info_T *qi, dict_T *what)
            {
                qf_idx = di->di_tv.vval.v_number - 1;
                if (qf_idx < 0 || qf_idx >= qi->qf_listcount)
-                   qf_idx = -1;
+                   qf_idx = INVALID_QFIDX;
            }
        }
        else if (di->di_tv.v_type == VAR_STRING
@@ -5094,7 +5095,7 @@ qf_getprop_qfidx(qf_info_T *qi, dict_T *what)
            /* Get the last quickfix list number */
            qf_idx = qi->qf_listcount - 1;
        else
-           qf_idx = -1;
+           qf_idx = INVALID_QFIDX;
     }
 
     if ((di = dict_find(what, (char_u *)"id", -1)) != NULL)
@@ -5109,7 +5110,7 @@ qf_getprop_qfidx(qf_info_T *qi, dict_T *what)
                qf_idx = qf_id2nr(qi, di->di_tv.vval.v_number);
        }
        else
-           qf_idx = -1;
+           qf_idx = INVALID_QFIDX;
     }
 
     return qf_idx;
@@ -5251,7 +5252,7 @@ qf_get_properties(win_T *wp, dict_T *what, dict_T *retdict)
        qf_idx = qf_getprop_qfidx(qi, what);
 
     /* List is not present or is empty */
-    if (qi == NULL || qi->qf_listcount == 0 || qf_idx == -1)
+    if (qi == NULL || qi->qf_listcount == 0 || qf_idx == INVALID_QFIDX)
        return qf_getprop_defaults(qi, flags, retdict);
 
     if (flags & QF_GETLIST_TITLE)
@@ -5405,19 +5406,19 @@ qf_add_entries(
     return retval;
 }
 
+/*
+ * Get the quickfix list index from 'nr' or 'id'
+ */
     static int
-qf_set_properties(qf_info_T *qi, dict_T *what, int action, char_u *title)
+qf_setprop_get_qfidx(
+       qf_info_T       *qi,
+       dict_T          *what,
+       int             action,
+       int             *newlist)
 {
     dictitem_T *di;
-    int                retval = FAIL;
-    int                qf_idx;
-    int                newlist = FALSE;
-    char_u     *errorformat = p_efm;
+    int                qf_idx = qi->qf_curlist;    /* default is the current list */
 
-    if (action == ' ' || qi->qf_curlist == qi->qf_listcount)
-       newlist = TRUE;
-
-    qf_idx = qi->qf_curlist;           /* default is the current list */
     if ((di = dict_find(what, (char_u *)"nr", -1)) != NULL)
     {
        /* Use the specified quickfix/location list */
@@ -5434,118 +5435,179 @@ qf_set_properties(qf_info_T *qi, dict_T *what, int action, char_u *title)
                 * non-available list and add the new list at the end of the
                 * stack.
                 */
-               newlist = TRUE;
-               qf_idx = qi->qf_listcount - 1;
+               *newlist = TRUE;
+               qf_idx = qi->qf_listcount > 0 ? qi->qf_listcount - 1 : 0;
            }
            else if (qf_idx < 0 || qf_idx >= qi->qf_listcount)
-               return FAIL;
+               return INVALID_QFIDX;
            else if (action != ' ')
-               newlist = FALSE;        /* use the specified list */
+               *newlist = FALSE;       /* use the specified list */
        }
        else if (di->di_tv.v_type == VAR_STRING
-                       && di->di_tv.vval.v_string != NULL
-                       && STRCMP(di->di_tv.vval.v_string, "$") == 0)
+               && di->di_tv.vval.v_string != NULL
+               && STRCMP(di->di_tv.vval.v_string, "$") == 0)
        {
            if (qi->qf_listcount > 0)
                qf_idx = qi->qf_listcount - 1;
-           else if (newlist)
+           else if (*newlist)
                qf_idx = 0;
            else
-               return FAIL;
+               return INVALID_QFIDX;
        }
        else
-           return FAIL;
+           return INVALID_QFIDX;
     }
 
-    if (!newlist && (di = dict_find(what, (char_u *)"id", -1)) != NULL)
+    if (!*newlist && (di = dict_find(what, (char_u *)"id", -1)) != NULL)
     {
        /* Use the quickfix/location list with the specified id */
-       if (di->di_tv.v_type == VAR_NUMBER)
-       {
-           qf_idx = qf_id2nr(qi, di->di_tv.vval.v_number);
-           if (qf_idx == -1)
-               return FAIL;        /* List not found */
-       }
-       else
-           return FAIL;
-    }
+       if (di->di_tv.v_type != VAR_NUMBER)
+           return INVALID_QFIDX;
 
-    if (newlist)
-    {
-       qi->qf_curlist = qf_idx;
-       qf_new_list(qi, title);
-       qf_idx = qi->qf_curlist;
+       return qf_id2nr(qi, di->di_tv.vval.v_number);
     }
 
-    if ((di = dict_find(what, (char_u *)"title", -1)) != NULL)
-    {
-       if (di->di_tv.v_type == VAR_STRING)
-       {
-           vim_free(qi->qf_lists[qf_idx].qf_title);
-           qi->qf_lists[qf_idx].qf_title =
-               get_dict_string(what, (char_u *)"title", TRUE);
-           if (qf_idx == qi->qf_curlist)
-               qf_update_win_titlevar(qi);
-           retval = OK;
-       }
-    }
+    return qf_idx;
+}
 
-    if ((di = dict_find(what, (char_u *)"items", -1)) != NULL)
-    {
-       if (di->di_tv.v_type == VAR_LIST)
-       {
-           char_u *title_save = vim_strsave(qi->qf_lists[qf_idx].qf_title);
+/*
+ * Set the quickfix list title.
+ */
+    static int
+qf_setprop_title(qf_info_T *qi, int qf_idx, dict_T *what, dictitem_T *di)
+{
+    if (di->di_tv.v_type != VAR_STRING)
+       return FAIL;
 
-           retval = qf_add_entries(qi, qf_idx, di->di_tv.vval.v_list,
-                   title_save, action == ' ' ? 'a' : action);
-           if (action == 'r')
-           {
-               /*
-                * When replacing the quickfix list entries using
-                * qf_add_entries(), the title is set with a ':' prefix.
-                * Restore the title with the saved title.
-                */
-               vim_free(qi->qf_lists[qf_idx].qf_title);
-               qi->qf_lists[qf_idx].qf_title = vim_strsave(title_save);
-           }
-           vim_free(title_save);
-       }
-    }
+    vim_free(qi->qf_lists[qf_idx].qf_title);
+    qi->qf_lists[qf_idx].qf_title =
+       get_dict_string(what, (char_u *)"title", TRUE);
+    if (qf_idx == qi->qf_curlist)
+       qf_update_win_titlevar(qi);
+
+    return OK;
+}
+
+/*
+ * Set quickfix list items/entries.
+ */
+    static int
+qf_setprop_items(qf_info_T *qi, int qf_idx, dictitem_T *di, int action)
+{
+    int                retval = FAIL;
+    char_u     *title_save;
 
-    if ((di = dict_find(what, (char_u *)"efm", -1)) != NULL)
+    if (di->di_tv.v_type != VAR_LIST)
+       return FAIL;
+
+    title_save = vim_strsave(qi->qf_lists[qf_idx].qf_title);
+    retval = qf_add_entries(qi, qf_idx, di->di_tv.vval.v_list,
+           title_save, action == ' ' ? 'a' : action);
+    if (action == 'r')
     {
-       if (di->di_tv.v_type != VAR_STRING || di->di_tv.vval.v_string == NULL)
-           return FAIL;
-       errorformat = di->di_tv.vval.v_string;
+       /*
+        * When replacing the quickfix list entries using
+        * qf_add_entries(), the title is set with a ':' prefix.
+        * Restore the title with the saved title.
+        */
+       vim_free(qi->qf_lists[qf_idx].qf_title);
+       qi->qf_lists[qf_idx].qf_title = vim_strsave(title_save);
     }
+    vim_free(title_save);
 
-    if ((di = dict_find(what, (char_u *)"lines", -1)) != NULL)
+    return retval;
+}
+
+/*
+ * Set quickfix list items/entries from a list of lines.
+ */
+    static int
+qf_setprop_items_from_lines(
+       qf_info_T       *qi,
+       int             qf_idx,
+       dict_T          *what,
+       dictitem_T      *di,
+       int             action)
+{
+    char_u     *errorformat = p_efm;
+    dictitem_T *efm_di;
+    int                retval = FAIL;
+
+    /* Use the user supplied errorformat settings (if present) */
+    if ((efm_di = dict_find(what, (char_u *)"efm", -1)) != NULL)
     {
-       /* Only a List value is supported */
-       if (di->di_tv.v_type == VAR_LIST && di->di_tv.vval.v_list != NULL)
-       {
-           if (action == 'r')
-               qf_free_items(qi, qf_idx);
-           if (qf_init_ext(qi, qf_idx, NULL, NULL, &di->di_tv, errorformat,
-                       FALSE, (linenr_T)0, (linenr_T)0, NULL, NULL) > 0)
-               retval = OK;
-       }
-       else
+       if (efm_di->di_tv.v_type != VAR_STRING ||
+               efm_di->di_tv.vval.v_string == NULL)
            return FAIL;
+       errorformat = efm_di->di_tv.vval.v_string;
     }
 
-    if ((di = dict_find(what, (char_u *)"context", -1)) != NULL)
-    {
-       typval_T        *ctx;
+    /* Only a List value is supported */
+    if (di->di_tv.v_type != VAR_LIST || di->di_tv.vval.v_list == NULL)
+       return FAIL;
 
-       free_tv(qi->qf_lists[qf_idx].qf_ctx);
-       ctx =  alloc_tv();
-       if (ctx != NULL)
-           copy_tv(&di->di_tv, ctx);
-       qi->qf_lists[qf_idx].qf_ctx = ctx;
+    if (action == 'r')
+       qf_free_items(qi, qf_idx);
+    if (qf_init_ext(qi, qf_idx, NULL, NULL, &di->di_tv, errorformat,
+               FALSE, (linenr_T)0, (linenr_T)0, NULL, NULL) > 0)
        retval = OK;
+
+    return retval;
+}
+
+/*
+ * Set quickfix list context.
+ */
+    static int
+qf_setprop_context(qf_info_T *qi, int qf_idx, dictitem_T *di)
+{
+    typval_T   *ctx;
+
+    free_tv(qi->qf_lists[qf_idx].qf_ctx);
+    ctx =  alloc_tv();
+    if (ctx != NULL)
+       copy_tv(&di->di_tv, ctx);
+    qi->qf_lists[qf_idx].qf_ctx = ctx;
+
+    return OK;
+}
+
+/*
+ * Set quickfix/location list properties (title, items, context).
+ * Also used to add items from parsing a list of lines.
+ * Used by the setqflist() and setloclist() VimL functions.
+ */
+    static int
+qf_set_properties(qf_info_T *qi, dict_T *what, int action, char_u *title)
+{
+    dictitem_T *di;
+    int                retval = FAIL;
+    int                qf_idx;
+    int                newlist = FALSE;
+
+    if (action == ' ' || qi->qf_curlist == qi->qf_listcount)
+       newlist = TRUE;
+
+    qf_idx = qf_setprop_get_qfidx(qi, what, action, &newlist);
+    if (qf_idx == INVALID_QFIDX)       /* List not found */
+       return FAIL;
+
+    if (newlist)
+    {
+       qi->qf_curlist = qf_idx;
+       qf_new_list(qi, title);
+       qf_idx = qi->qf_curlist;
     }
 
+    if ((di = dict_find(what, (char_u *)"title", -1)) != NULL)
+       retval = qf_setprop_title(qi, qf_idx, what, di);
+    if ((di = dict_find(what, (char_u *)"items", -1)) != NULL)
+       retval = qf_setprop_items(qi, qf_idx, di, action);
+    if ((di = dict_find(what, (char_u *)"lines", -1)) != NULL)
+       retval = qf_setprop_items_from_lines(qi, qf_idx, what, di, action);
+    if ((di = dict_find(what, (char_u *)"context", -1)) != NULL)
+       retval = qf_setprop_context(qi, qf_idx, di);
+
     if (retval == OK)
        qf_list_changed(qi, qf_idx);
 

diff --git a/src/testdir/test_quickfix.vim b/src/testdir/test_quickfix.vim
index 12bfe32278fe348f30a9c8170bf829a5f5d41534..f12f4f21a7d69bb597db4b64db0b838620575f39 100644 (file)
--- a/src/testdir/test_quickfix.vim
+++ b/src/testdir/test_quickfix.vim
@@ -1795,6 +1795,9 @@ func Xproperty_tests(cchar)
     call assert_equal(0, s)
     let d = g:Xgetlist({"title":1})
     call assert_equal('Sample', d.title)
+    " Try setting title to a non-string value
+    call assert_equal(-1, g:Xsetlist([], 'a', {'title' : ['Test']}))
+    call assert_equal('Sample', g:Xgetlist({"title":1}).title)
 
     Xopen
     call assert_equal('Sample', w:quickfix_title)
@@ -1943,6 +1946,9 @@ func Xproperty_tests(cchar)
     call g:Xsetlist([], 'a', {'items' : [{'filename':'F1', 'lnum':10}]})
     call assert_equal(10, g:Xgetlist({'items':1}).items[0].lnum)
 
+    " Try setting the items using a string
+    call assert_equal(-1, g:Xsetlist([], ' ', {'items' : 'Test'}))
+
     " Save and restore the quickfix stack
     call g:Xsetlist([], 'f')
     call assert_equal(0, g:Xgetlist({'nr':'$'}).nr)

diff --git a/src/version.c b/src/version.c
index ac6e0e8ffd907b1ff86e9d721d72686bef27212e..fda91e2938a61ac8da349e943c62c971caa75dc9 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -761,6 +761,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1752,
 /**/
     1751,
 /**/