Hi Bram, On Tue, Jun 13, 2017 at 5:34 AM, Bram Moolenaar <[email protected]> wrote: > > Yegappan Lakshmanan wrote: > >> >> I am attaching a patch to get and set the quickfix list items using the >> >> getqflist() and setqflist() functions. After this patch, it will be easier >> >> to save and restore any quickfix list in the quickfix stack. >> >> >> >> This will also make it consistent to get or set any attributes (title, >> >> context, items) of the quickfix list using the getqflist() and setqflist() >> >> functions. >> > >> > Splitting qf_free() in two functions makes me wonder what each of them >> > is doing. >> > >> >> The qf_free_items() function frees only the entries in a quickfix list. >> Other information like the context and title are not freed. This is needed >> when replacing all the entries in a quickfix list using the setqflist() >> function with the "r" action argument. In this case, we should not free the >> context and title information. >> >> The qf_free() function frees all the entries in a list, the context >> information and the title. This function completely frees a list. > > We need to be a bit more precise to avoid confusion and making mistakes > in the future. The comment for qf_list_T says that if qf_count is zero > this means no error list. But now qf_title and qf_ctx may still be set, > thus there is an error list, but with zero entries. > > If this is a transient state, thus this only happens while changing the > list, and eventually we get to a state with qf_count non-zero, that > should be mentioned as well. > > Best place to explain it is where qf_list_T is declared, at line 50. > There is no comment there at all, thus saying a few words would be > helpful. At first it's not clear that qf_info_T contains an array of > qf_list_T which contains a list of qfline_T. >
I will add comments to these structures and send out an updated diff shortly. - Yegappan > >> > It appears qf_free() also clears the title and context. But >> > in qf_add_entries() it was previously clearing the context, and it is >> > not. This should be documented. I would also like to have a clearer >> > comment on those qf_free functions. >> >> I have updated the comments and the updated patch is attached. > -- -- 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.
