Address #4165: provide a highlight group for the detailed diagnostic popup

[Aster89]

This seems to work, but I have a few questions

• What tests should I look at to understand how to write one for this change?
• Are you happy with the name YcmErrorPopup?
• What should I set as 'priority', 'combine', 'override'?

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

See #4165 and conversation on gitter.

---

This change is 

[Aster89]

As regards the test, I've searched for ErrorMsg in the test directory, thinking I would find one asserting that's the highlight group used for the popup, but I found no match.

[puremourning]

I guess search the tests for the option that makes this a popup? I'm not entirely sure we have tests for this tbh.

[puremourning]

Are you happy with the name YcmErrorPopup?

Yep, though maybe 'Diagnostic' as it might not be an error (it might be a warning).

What should I set as 'priority', 'combine', 'override'?

I dunno. Check vims docs. I'm never entirely sure.

[puremourning]

This LGTM tbh

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

---

autoload/youcompleteme.vim line 517 at r1 (raw file):

  if !hlexists( 'YcmErrorPopup' )
    if hlexists( 'ErrorMsg' )

is there a reason this wouldn't exist?

[codecov]

Codecov Report

Merging #4166 (eddba4f) into master (4f1dcf4) will increase coverage by 0.09%.
The diff coverage is 100.00%.

❗ Current head eddba4f differs from pull request most recent head f742ecb. Consider uploading reports for the commit f742ecb to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4166      +/-   ##
==========================================
+ Coverage   88.82%   88.92%   +0.09%     
==========================================
  Files          34       34              
  Lines        4395     4397       +2     
==========================================
+ Hits         3904     3910       +6     
+ Misses        491      487       -4     

[Aster89]

What else was to be done here?

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

---

autoload/youcompleteme.vim line 517 at r1 (raw file):

Previously, puremourning (Ben Jackson) wrote…

is there a reason this wouldn't exist?

I don't remember anymore why I did it :'(

[Aster89]

is there a reason this wouldn't exist?

But I guess since ErrorMsg hl group exists, maybe this if was really a blunder of mine :/

[puremourning]

Yeah I don't like the if unless we know why it's needed. Maybe ErrorMsg is optional?

[puremourning]

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

[Aster89]

Yeah I don't like the if unless we know why it's needed. Maybe ErrorMsg is optional?

Looking at the other file, you were giving for granted that ErrorMsg existed, so I don't understand why I should do otherwise. Happy to remove it.

[Aster89]

Don't you merge this?

[puremourning]

Sorry I had to click to approve the CI run

[mergify]

Thanks for sending a PR!

[puremourning]

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

a discussion (no related file):
Please update docs.

---

[bstaletic]

@Aster89 The docs should be updated in the README.md too.

Usually, YCM the README.md gets updated and then vimdocs get auto-generated with a script (and a bot).

[Aster89]

@Aster89 The docs should be updated in the README.md too.

Usually, YCM the README.md gets updated and then vimdocs get auto-generated with a script (and a bot).

Good now?

[bstaletic]

Reviewed 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @Aster89 and @puremourning)

---

autoload/youcompleteme.vim line 520 at r4 (raw file):

  endif
  if s:PropertyTypeNotDefined( 'YcmErrorProperty' )
    call prop_type_add( 'YcmErrorProperty', {

Is it just me or can YCM never reach this line?

The property type named YcmErrorProperty was first defined on line 439.

[Aster89]

Reviewed 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @Aster89 and @puremourning)

autoload/youcompleteme.vim line 520 at r4 (raw file):

  endif
  if s:PropertyTypeNotDefined( 'YcmErrorProperty' )
    call prop_type_add( 'YcmErrorProperty', {

Is it just me or can YCM never reach this line?

The property type named YcmErrorProperty was first defined on line 439.

Removed

[bstaletic]

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

[Aster89]

Hi, do I need to do anything else here?

[puremourning]

Reviewed 1 of 1 files at r3, 1 of 1 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained (and 1 stale) (waiting on @Aster89)

[bstaletic]

Reviewed all commit messages.
Reviewable status: complete! 2 of 2 LGTMs obtained (waiting on @Aster89)

[mergify]

Thanks for sending a PR!