[READY] Use location list stack, instead of trampling on existing ones

[bstaletic]

PR Prelude

Thank you for working on YCM! :)

Please complete these steps and check these boxes (by putting an x inside
the brackets) before filing your PR:

• I have read and understood YCM's CONTRIBUTING document.
• I have read and understood YCM's CODE_OF_CONDUCT document.
• I have included tests for the changes in my PR. If not, I have included a
  rationale for why I haven't.
• I understand my PR may be closed if it becomes obvious I didn't
  actually perform all of these steps.

Why this change is necessary and useful

Personally, I wanted this for a looong time, but the first time I brought this up @puremourning was against it. The argument was that it would be too advanced for an average vim user to navigate the loclist stack. The other reason this wasn't implemented earlier is because the docs are really not clear how the finer details of setloclist() work!

But here's the general idea:

1. If a user is using :lgrep, we don't want to instantly trample over that location list. So we need a different one in the stack.
2. Constantly creating new lists, would quickly fill up the stack and old lists would be lost. So we should keep track of which list is ours and reuse it.
3. Populating loc list automatically in the background shouldn't open the list every time it changes. Especially if the currently active list is not ours.
4. However, if the user ran a manual update of diagnostics, we should bring that list to the front. Except vim doesn't let us reorder lists, so the current solution is to remove contents and title of our old list and create a new one. Maybe we could just focus the old one with :Nlolder?
5. Interactions with user options:
   5.1. There are two relevant options: g:ycm_always_populate_location_list and g:ycm_open_loclist_on_ycm_diags
   5.2. If the former is off and latter on, populating only happens manually and makes cursor focus the loc list.
   5.3. If both are off, even :YcmDiags won't show the list.
   5.4. If former is on, but latter is off, both automatic population and :YcmDiags will populate the loc list, but we'll never open it for the user.
   5.5. If both are on, both ways populate the loc list, but only :YcmDiags makes YCM focus the loc list.

5 is defined like that, because visiting a buffer for which filetype there's no diagnostics available at all, we should never replace :lgrep with an empty list of diags. Ever.

The PR, as is, seems to be working with clangd at least. Testing with something like Tern or libclang would be useful. Writing tests as well.

This will actually make me turn on g:ycm_always_populate_location_list.

[Please explain in detail why the changes in this PR are needed.]

---

This change is 

[codecov]

Codecov Report

Merging #3951 (2163ff0) into master (f35a30d) will decrease coverage by 0.24%.
The diff coverage is 80.76%.

@@            Coverage Diff             @@
##           master    #3951      +/-   ##
==========================================
- Coverage   91.43%   91.18%   -0.25%     
==========================================
  Files          27       27              
  Lines        3688     3699      +11     
==========================================
+ Hits         3372     3373       +1     
- Misses        316      326      +10     

[puremourning]

Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic and @puremourning)

a discussion (no related file):
So I found a case which maybe isn't quite what we might expect:

• open a file with no errors
• :lgrep something to create loc list
• create an error
• :lwindow (see the YCM errors)
• :lolder (see the grep results)
• create another error
• Expect: still see the grep results
• Actual: the YCM loc list is visible.

intentional?

---

---

python/ycm/vimsupport.py, line 323 at r1 (raw file):

  total_location_lists = int( vim.eval( f'getloclist( { window_number}, '
                                         '{ "nr": "$" } ).nr' ) )
  ycm_loc_id = None

maybe we could just store this in the window as a variable like vim.windows[ window_number ].variables[ 'ycm_loc_id' ], this is the equivalent of let w:ycm_loc_id = and saves all this searching. wdyt?

[bstaletic]

Reviewable status: 0 of 2 LGTMs obtained (waiting on @puremourning)

a discussion (no related file):
Hm... What was your g:ycm_always_populate_location_list and g:ycm_open_locationlist_on_ycm_diags?

With both set to 1 I don't see that behaviour.

• open a file with no errors
• :lgrep something to create loc list

The location list window already opens for me. Closing.

• create an error
• :lwindow (see the YCM errors)

Uhm... I still see :lgrep results.

• :lolder (see the grep results)

Returns me to YCM error list!

• create another error
• Expect: still see the grep results.

Works on my machine!

---

---

python/ycm/vimsupport.py, line 323 at r1 (raw file):

Previously, puremourning (Ben Jackson) wrote…

maybe we could just store this in the window as a variable like vim.windows[ window_number ].variables[ 'ycm_loc_id' ], this is the equivalent of let w:ycm_loc_id = and saves all this searching. wdyt?

I tried to break it, but so far couldn't. Should get some dogfooding treatment.

[puremourning]

Reviewed 2 of 8 files at r1, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @puremourning)

a discussion (no related file):

Previously, bstaletic (Boris Staletic) wrote…

Hm... What was your g:ycm_always_populate_location_list and g:ycm_open_locationlist_on_ycm_diags?

With both set to 1 I don't see that behaviour.

• open a file with no errors
• :lgrep something to create loc list

The location list window already opens for me. Closing.

• create an error
• :lwindow (see the YCM errors)

Uhm... I still see :lgrep results.

• :lolder (see the grep results)

Returns me to YCM error list!

• create another error
• Expect: still see the grep results.

Works on my machine!

Hmm both set to 1 for me.

Also, I can't seem to repro now either. Maybe I was confused the other day.

do we need to update anything in the README ?

---

---

python/ycm/vimsupport.py, line 323 at r1 (raw file):

Previously, bstaletic (Boris Staletic) wrote…

I tried to break it, but so far couldn't. Should get some dogfooding treatment.

I don't think this is broken, just inelegant. not a big deal.

[puremourning]

Reviewed 6 of 8 files at r1.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)

[bstaletic]

Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)

a discussion (no related file):

do we need to update anything in the README ?

I added a small note.

---

---

python/ycm/vimsupport.py, line 323 at r1 (raw file):

Previously, puremourning (Ben Jackson) wrote…

I don't think this is broken, just inelegant. not a big deal.

I forgot to push yesterday. Anyway, implemented what you've asked for.

[puremourning]

Reviewed 7 of 7 files at r2, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)

[puremourning]

Sweet! Nice.

Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)

[mergify]

Thanks for sending a PR!