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. > > 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. -- The only backup you need is the one that you didn't have time for. (Murphy) /// Bram Moolenaar -- [email protected] -- 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 /// -- -- 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.
