Add diagnostic_enable_location_list to enable/disable loclist

[Furkanzmc]

I use Neomake for linting as well and I found this option to be useful. But maybe instead of replacing the loclist, I can append to it. But I'm not sure how well that would work.

I think this is a useful addition, but let me know If it's not inline with how you see your plugin moving forward.

[haorenW1025]

Hmm a problem is right now I'm using location list as a driver for the :NextDiagnostic, :PrevDiagnostic stuff, and disable location list will break those things. So this should be way more complicated and might have to rewrite some of the logic in the callback function.

[Furkanzmc]

You are right, I didn't think about that. I'm not sure what the right implementation for this would be then.

I could remove those commands when the location list is disabled, but If the user has a mapping to those commands things would break.

Maybe those commands can be no-ops when location is disabled?

So this should be way more complicated and might have to rewrite some of the logic in the callback function.

Are you referring to M.modifyCallback()?

[haorenW1025]

Yeah, if we want both neomake's linting and lsp diagnostic work at the same time without any conflict, one way is not using location list at all and implement our own kind of jumping, which I think would be too much work..Another approach is maybe use quickfix window instead? So the option would be

let g:diagnostic_list = 'location' " or 'quickfix'

[Furkanzmc]

I honestly can't see a case where having the diagnostic information in the quickfix list would be useful. I think loclist is the perfect place for it. At least for my use case, I would not benefit from having the diagnostic information in quickfix because that's where I keep my build log or run log.

I'm not that familiar with NeoVim's LSP API, but looking through the documentation, I found this:

                                         *vim.lsp.util.diagnostics_by_buf*
diagnostics_by_buf
    A table containing diagnostics grouped by buf.

    {<bufnr>: {diagnostics}}

    {diagnostics} is an array of diagnostics.

    By default this is populated by the
    `textDocument/publishDiagnostics` callback via
    |vim.lsp.util.buf_diagnostics_save_positions|.

    It contains entries for active buffers. Once a buffer is
    detached the entries for it are discarded.

Would this be useful to get the appropriate loclist item?

As far as I understand, your implementation for jumping to the next one is similar to :lnext but the navigation starts from the current line. Is that correct?

[haorenW1025]

As far as I understand, your implementation for jumping to the next one is similar to :lnext but the navigation starts from the current line. Is that correct?

Yes, exactly.

I think the diagnostics_by_buf function gets a list of diagnostic without parsing, which I already have similar functionality. If not using location list, I guess the only way should be parsing the items but not setting it to location list, store it as a variable and use it to jump around.

[Furkanzmc]

If not using location list, I guess the only way should be parsing the items but not setting it to location list, store it as a variable and use it to jump around.

This makes sense.

In diagnostic.lua

if vim.api.nvim_get_var('diagnostic_enable_location_list') == 1 then
    vim.lsp.util.set_loclist(vim.lsp.util.locations_to_items(local_result.diagnostics))
end

So, even when g:diagnostic_enable_location_list is set to 1, we'd have the list of diagnostic information separate from the loclist and use it when navigating. Correct?

If you approve this approach, I can work on it. It'd be good to stretch my lua muscles.

[haorenW1025]

Yes, just don't use the set_loclist function. Maybe you'll have to look at runtime/lua/vim/lsp/util.lua of neovim's source code, those util function (locations_to_items and set_loclist) are defined in there.

[Furkanzmc]

Just a heads up, I don't think I'll be able to work on this any time soon. Work schedule is a bit hectic.