patch 8.1.1098: quickfix code duplication

Problem: Quickfix code duplication. Solution: Refactor the qf_init_ext() function. (Yegappan Lakshmanan, closes #4193)
2019-03-31 15:31:59 +02:00
parent 4fc8e2f8bb
commit 95946f1209
3 changed files with 74 additions and 59 deletions
--- a/src/quickfix.c
+++ b/src/quickfix.c
@@ -195,9 +195,11 @@ static qf_info_T *ll_get_or_alloc_list(win_T *);
 */
 #define GET_LOC_LIST(wp) (IS_LL_WINDOW(wp) ? wp->w_llist_ref : wp->w_llist)

+// Macro to loop through all the items in a quickfix list
+// Quickfix item index starts from 1, so i below starts at 1
 #define FOR_ALL_QFL_ITEMS(qfl, qfp, i) \
-		    for (i = 0, qfp = qfl->qf_start; \
-			    !got_int && i < qfl->qf_count && qfp != NULL; \
+		    for (i = 1, qfp = qfl->qf_start; \
+			    !got_int && i <= qfl->qf_count && qfp != NULL; \
 			    ++i, qfp = qfp->qf_next)

 /*
@@ -1584,6 +1586,47 @@ qf_cleanup_state(qfstate_T *pstate)
 	convert_setup(&pstate->vc, NULL, NULL);
 }

+/*
+ * Process the next line from a file/buffer/list/string and add it
+ * to the quickfix list 'qfl'.
+ */
+    static int
+qf_init_process_nextline(
+	qf_list_T	*qfl,
+	efm_T		*fmt_first,
+	qfstate_T	*state,
+	qffields_T	*fields)
+{
+    int		    status;
+
+    // Get the next line from a file/buffer/list/string
+    status = qf_get_nextline(state);
+    if (status != QF_OK)
+	return status;
+
+    status = qf_parse_line(qfl, state->linebuf, state->linelen,
+	    fmt_first, fields);
+    if (status != QF_OK)
+	return status;
+
+    return qf_add_entry(qfl,
+		qfl->qf_directory,
+		(*fields->namebuf || qfl->qf_directory != NULL)
+		? fields->namebuf
+		: ((qfl->qf_currfile != NULL && fields->valid)
+		    ? qfl->qf_currfile : (char_u *)NULL),
+		fields->module,
+		0,
+		fields->errmsg,
+		fields->lnum,
+		fields->col,
+		fields->use_viscol,
+		fields->pattern,
+		fields->enr,
+		fields->type,
+		fields->valid);
+}
+
 /*
 * Read the errorfile "efile" into memory, line by line, building the error
 * list.
@@ -1676,39 +1719,14 @@ qf_init_ext(
    // Try to recognize one of the error formats in each line.
    while (!got_int)
    {
-	// Get the next line from a file/buffer/list/string
-	status = qf_get_nextline(&state);
+	status = qf_init_process_nextline(qfl, fmt_first, &state, &fields);
 	if (status == QF_NOMEM)		// memory alloc failure
 	    goto qf_init_end;
 	if (status == QF_END_OF_INPUT)	// end of input
 	    break;
-
-	status = qf_parse_line(qfl, state.linebuf, state.linelen,
-							fmt_first, &fields);
 	if (status == QF_FAIL)
 	    goto error2;
-	if (status == QF_NOMEM)
-	    goto qf_init_end;
-	if (status == QF_IGNORE_LINE)
-	    continue;

-	if (qf_add_entry(qfl,
-			qfl->qf_directory,
-			(*fields.namebuf || qfl->qf_directory != NULL)
-			    ? fields.namebuf
-			    : ((qfl->qf_currfile != NULL && fields.valid)
-				? qfl->qf_currfile : (char_u *)NULL),
-			fields.module,
-			0,
-			fields.errmsg,
-			fields.lnum,
-			fields.col,
-			fields.use_viscol,
-			fields.pattern,
-			fields.enr,
-			fields.type,
-			fields.valid) == FAIL)
-	    goto error2;
 	line_breakcheck();
    }
    if (state.fd == NULL || !ferror(state.fd))
@@ -2013,7 +2031,7 @@ check_quickfix_busy(void)

 /*
 * Add an entry to the end of the list of errors.
- * Returns OK or FAIL.
+ * Returns QF_OK or QF_FAIL.
 */
    static int
 qf_add_entry(
@@ -2035,7 +2053,7 @@ qf_add_entry(
    qfline_T	**lastp;	// pointer to qf_last or NULL

    if ((qfp = (qfline_T *)alloc((unsigned)sizeof(qfline_T))) == NULL)
-	return FAIL;
+	return QF_FAIL;
    if (bufnum != 0)
    {
 	buf_T *buf = buflist_findnr(bufnum);
@@ -2050,7 +2068,7 @@ qf_add_entry(
    if ((qfp->qf_text = vim_strsave(mesg)) == NULL)
    {
 	vim_free(qfp);
-	return FAIL;
+	return QF_FAIL;
    }
    qfp->qf_lnum = lnum;
    qfp->qf_col = col;
@@ -2061,7 +2079,7 @@ qf_add_entry(
    {
 	vim_free(qfp->qf_text);
 	vim_free(qfp);
-	return FAIL;
+	return QF_FAIL;
    }
    if (module == NULL || *module == NUL)
 	qfp->qf_module = NULL;
@@ -2070,7 +2088,7 @@ qf_add_entry(
 	vim_free(qfp->qf_text);
 	vim_free(qfp->qf_pattern);
 	vim_free(qfp);
-	return FAIL;
+	return QF_FAIL;
    }
    qfp->qf_nr = nr;
    if (type != 1 && !vim_isprintc(type)) // only printable chars allowed
@@ -2101,7 +2119,7 @@ qf_add_entry(
 	qfl->qf_ptr = qfp;
    }

-    return OK;
+    return QF_OK;
 }

 /*
@@ -2167,7 +2185,7 @@ copy_loclist_entries(qf_list_T *from_qfl, qf_list_T *to_qfl)
 		    from_qfp->qf_pattern,
 		    from_qfp->qf_nr,
 		    0,
-		    from_qfp->qf_valid) == FAIL)
+		    from_qfp->qf_valid) == QF_FAIL)
 	    return FAIL;

 	// qf_add_entry() will not set the qf_num field, as the
@@ -2551,7 +2569,7 @@ is_qf_entry_present(qf_list_T *qfl, qfline_T *qf_ptr)
 	if (qfp == qf_ptr)
 	    break;

-    if (i == qfl->qf_count) // Entry is not found
+    if (i > qfl->qf_count) // Entry is not found
 	return FALSE;

    return TRUE;
@@ -3554,21 +3572,11 @@ qf_list(exarg_T *eap)

    if (qfl->qf_nonevalid)
 	all = TRUE;
-    qfp = qfl->qf_start;
-    for (i = 1; !got_int && i <= qfl->qf_count; )
+    FOR_ALL_QFL_ITEMS(qfl, qfp, i)
    {
 	if ((qfp->qf_valid || all) && idx1 <= i && i <= idx2)
-	{
-	    if (got_int)
-		break;
-
 	    qf_list_entry(qfp, i, i == qfl->qf_index);
-	}

-	qfp = qfp->qf_next;
-	if (qfp == NULL)
-	    break;
-	++i;
 	ui_breakcheck();
    }
 }
@@ -4915,7 +4923,7 @@ qf_get_cur_valid_idx(exarg_T *eap)
    static int
 qf_get_nth_valid_entry(qf_list_T *qfl, int n, int fdo)
 {
-    qfline_T	*qfp = qfl->qf_start;
+    qfline_T	*qfp;
    int		i, eidx;
    int		prev_fnum = 0;

@@ -4923,8 +4931,8 @@ qf_get_nth_valid_entry(qf_list_T *qfl, int n, int fdo)
    if (qfl->qf_count <= 0 || qfl->qf_nonevalid)
 	return 1;

-    for (i = 1, eidx = 0; i <= qfl->qf_count && qfp != NULL;
-	    i++, qfp = qfp->qf_next)
+    eidx = 0;
+    FOR_ALL_QFL_ITEMS(qfl, qfp, i)
    {
 	if (qfp->qf_valid)
 	{
@@ -5330,7 +5338,7 @@ vgr_match_buflines(
 			0,	    // nr
 			0,	    // type
 			TRUE	    // valid
-			) == FAIL)
+			) == QF_FAIL)
 	    {
 		got_int = TRUE;
 		break;
@@ -6434,7 +6442,7 @@ qf_add_entries(

 	retval = qf_add_entry_from_dict(qfl, d, li == list->lv_first,
 								&valid_entry);
-	if (retval == FAIL)
+	if (retval == QF_FAIL)
 	    break;
    }

@@ -6744,14 +6752,18 @@ qf_free_stack(win_T *wp, qf_info_T *qi)
 	// If the location list window is open, then create a new empty
 	// location list
 	qf_info_T *new_ll = qf_alloc_stack(QFLT_LOCATION);
-	new_ll->qf_bufnr = qfwin->w_buffer->b_fnum;

-	// first free the list reference in the location list window
-	ll_free_all(&qfwin->w_llist_ref);
+	if (new_ll != NULL)
+	{
+	    new_ll->qf_bufnr = qfwin->w_buffer->b_fnum;

-	qfwin->w_llist_ref = new_ll;
-	if (wp != qfwin)
-	    win_set_loclist(wp, new_ll);
+	    // first free the list reference in the location list window
+	    ll_free_all(&qfwin->w_llist_ref);
+
+	    qfwin->w_llist_ref = new_ll;
+	    if (wp != qfwin)
+		win_set_loclist(wp, new_ll);
+	}
    }
 }

@@ -7203,7 +7215,7 @@ hgr_search_file(
 			0,	// nr
 			1,	// type
 			TRUE	// valid
-			) == FAIL)
+			) == QF_FAIL)
 	    {
 		got_int = TRUE;
 		if (line != IObuff)