Neovim Universal Healthcare #4932

tjdevries · 2016-06-17T02:49:12Z

Context

This is in response to #4478. It can be thought of as an extension of #4885.

We will be attempting to create an interface that anyone can hook into to display messages about the health of their plugin, script, feature, etc.

Outline

The general outline (subject to change) is as follows:

nvim/runtime/autoload/health.vim: Sets up the health checker interface. Provides:
- g:health_checkers: The health checkers dictionary that contains all the available checkers to run. The value of the key determines whether it should be run or not (v:true and v:false)
  - Also provides a way to find health checkers where applicable.
- health#check(): Some report function that can be called. This will run all the enabled doctors, and compile the results into a nice report (which may initially be printing to a nvim buffer). This can be called by :CheckHealth<bang>
- Report functions - These will be used by people making their own health.vim files.
  - Initial function:
    - health#report_start(name): A function to begin a specific area of a report.
    - health#report_info(msg): A function to report general info from the checker
    - health#report_warn(msg, [suggestions]): A function to report warnings, with optional list of suggestions.
    - health#report_error(msg, [suggestions]): A function to report errors, with optional list of suggestions.
nvim/runtime/autoload/health/nvim.vim: This is where the current health.vim file goes. All standard Neovim gotchas / checks will go in this file. It could be split out later quite easily.
Future plugins / checkers would simply need to place a $PLUGIN/autoload/health/$PLUGIN.vim and have a function health#{plugin_name}#check(bang) that runs the desired health checks.
- These could also be made into smaller categories, such as autoload/health/{plug}/python.vim, autoload/health/{plugin}/environment.vim, etc. to allow for further customiation.
- ~~They would also need to register their plugin with health#add_checker({checker_name})~~
- Any plugin that has a file in $PLUGIN/autoload/health/**.vim will automatically be added and enabled as a health_checker.

Future work

Improved reporting functions:
- Allow for different formats of health reporting (txt, markdown, json, etc.). This would be under the same interface, just flipping a different flag.
- Color coding for reports
Whatever you can think of! 😄

Certain To Do's

Update documentation
Add tests
- Check test/functional/plugin/shada_spec.lua
- Some sort of functional test for the python configuration
Make better to do

Open Questions

Best way to determine whether a health checker function exists.
Should the check exit if health#report_error is called, or leave that up to the plugin maintainer?

ZyX-I · 2016-06-17T13:54:07Z

I think you need to add tests to your TODO, check test/functional/plugin/shada_spec.lua (I point at this file because it contains tests for autoload, syntax, ftplugin and plugin files at once).

Hettomei · 2016-06-17T17:11:50Z

Name of health checkers. We could do doctors

If I understand, a command like :doctors ? I do not want 'doctor' because of emacs and 'doctor' mode.
I don't want any mistake for people using both.

tjdevries · 2016-06-17T17:19:06Z

@Hettomei The command would remain to be :CheckHealth. I have been trying to think of what to call the individual "health checkers" since that name seems clunky to me. I am not opposed to leaving it that way though.

fmoralesc · 2016-06-17T17:22:09Z

(I find the mental image of people mistaking CheckHealth with emacs doctor mode very funny)

I think checkers is fine as a name.

ZyX-I · 2016-06-17T17:31:02Z

“Checkers” is fine, but such things were always used for troubleshooting, and I would say that moving everything to autoload/troubleshoot* is better. But :Troubleshoot looks like only slightly better name for a command then :CheckHealth (I actually do not like either, but first just sounds bad while second has a reference to a “health” concept which I have not heard of in regards to application setup).

tjdevries · 2016-06-17T18:00:04Z

The original reason for using :CheckHealth is because that's what the current naming scheme is. I am happy to change it to whatever consensus we can come to here.

justinmk · 2016-06-17T18:02:08Z

Health checking is very common in the "devops" culture, cf. AWS. Command is named :CheckHealth for parallel form with existing "check" commands: :checktime, :checkpath.

"Troubleshooting" is far too long, too many syllables, and difficult to type e.g. in IRC or Reddit. It's also less generic: I was hoping :CheckHealth will be a general entry point for sanity checks: #1202

"healthcheck" is what the checkers can be called.

wsdjeg · 2016-06-17T22:49:14Z

agree with @justinmk.

tjdevries · 2016-06-20T02:05:48Z

What does everyone think about something like this?

This is a little more involved than the original proposal, but I don't think it would be very difficult to use. It would take away a lot of the hassle that I see in the current python health checker, worrying about indentation, line wraps, etc.

""
" Inside of autoload/health/nvim.vim
""
function! health#nvim#check() abort
  " Start diagnosing a certain item
  call health#diagnose('Python 3 configuration')

  call health#symptom_check('General Python Environment')
  call health#add_note('Python 3 is in the path', 'success')
  " Say a symptom check
  call health#symptom_check('Virtual Environment')
  " Add notes about the system
  call health#add_note('Virtual env not configured.', 'warning')
  nvim_pkg_suggestions = [
        \ 'Attempt to install the Neovim pacakage with `pip3 install neovim`',
        \ 'Consult the Neovim Python FAQ page at <website stuff>'
        \ ]
  call health#add_note('Neovim package not installed', 'warning', nvim_pkg_suggestions)
endfunction

""
" After call of health#check() for this checker
""

" Prints something like this
Nvim Health Checker
  - Diagnosing: Python 3 Configuration
    - General Python Environment
      - SUCCESS: Python 3 is in the Path
    - Virtual Environment
      - WARNING: Virtual env is not configured
      - WARNING: Neovm package not installed
        - SUGGESTION: Attempt to install the Neovim pacakage with `pip3 install neovim`,
        - SUGGESTION: Consult the Neovim Python FAQ page at <website stuff>

justinmk · 2016-06-20T04:00:04Z

@tjdevries Looks like a nice API. If the names can be made more obvious, it will further help plugin authors.

health#diagnose: let's omit this and just list the function name: health#nvim#check. (Can always add this later if it's really needed.)
health#symptom_check: more-obvious name is health#report_start()
health#add_note(msg, severity): it will help plugin authors if we have separate functions for each severity level (helps with auto-complete, validation, documentation):
- health#report_ok(msg)
- health#report_info(msg)
- health#report_warn(msg, suggestions)

function! health#nvim#check() abort
  call health#report_start('General Python Environment')
  call health#report_ok('Python 3 is in the path')
  call health#report_start('Virtual Environment')
  call health#report_warn('Virtual env not configured.')
  nvim_pkg_suggestions = [
        \ 'Attempt to install the Neovim pacakage with `pip3 install neovim`',
        \ 'Consult the Neovim Python FAQ page at <website stuff>'
        \ ]
  call health#report_warn('Neovim package not installed', nvim_pkg_suggestions)
endfunction

tjdevries · 2016-06-20T04:09:26Z

@justinmk That seems to be straight forward enough for me. I like it and it seems to be pretty easy to adopt.

One thing I'm still considering is the levels. Would ok be for things that are as expected, info be for more of debug output (so environment variables, etc.) and warn be for unexpected results? Would we need any other levels? I suppose we could add them later.

justinmk · 2016-06-20T04:12:10Z

ok is "success"; info is just informative messages. What warn means, and whether we need another level, I was going to leave up to you.

tjdevries · 2016-06-20T04:13:18Z

ok ;)

I will think about it some more.

tweekmonster · 2016-06-20T04:35:16Z

whether we need another level, I was going to leave up to you

You're talking about health#report_wtf() right? 😉

But seriously, I think error would be needed to report on things that are obviously broken causing further diagnostics to be pointless/impossible.

tjdevries · 2016-06-21T03:34:09Z

Examples of some output that I've gotten from the new setup:

Checking health
Checker health#nvim#check says: true
  - Diagnosing: Python 2 Configuration
    - INFO: Using: g:python_host_prog = "/usr/bin/python"
    - INFO: Executable:/usr/bin/python
    - INFO: Python Version: 2.7.6
    - INFO: python-neovim Version: 0.1.8
    - WARNING: Latest Neovim Python client versions: 0.1.9)

  - Diagnosing: Python 3 Configuration
    - INFO: Using: g:python3_host_prog = "/usr/bin/python3"
    - INFO: Executable:/usr/bin/python3
    - INFO: Python Version: 3.4.3
    - INFO: python3-neovim Version: 0.1.8
    - WARNING: Latest Neovim Python client versions: 0.1.9)

  - Diagnosing: Remote Plugins
    - INFO: Up to date

Checking health
Checker health#nvim#check says: true
  - Diagnosing: Python 2 Configuration
    - WARNING: "g:python_host_prog" is not set.  Searching for python2 in the environment.
    - INFO: pyenv found: "/home/tj/.pyenv/libexec/pyenv"
    - WARNING: pyenv couldn't find python2.
    - WARNING: Your virtualenv is not set up optimally.
    - INFO: Executable:/usr/bin/python2
    - INFO: Python Version: 2.7.6
    - INFO: python2-neovim Version: 0.1.8
    - WARNING: Latest Neovim Python client versions: 0.1.9)

  - Diagnosing: Python 3 Configuration
    - WARNING: "g:python3_host_prog" is not set.  Searching for python3 in the environment.
    - INFO: pyenv found: "/home/tj/.pyenv/libexec/pyenv"
    - INFO: Executable:/home/tj/.pyenv/versions/3.5.1/bin/python3
    - INFO: Python Version: 3.5.1
    - INFO: python3-neovim Version: 0.1.8
    - WARNING: Latest Neovim Python client versions: 0.1.9)

  - Diagnosing: Remote Plugins
    - INFO: Up to date

tweekmonster · 2016-06-21T03:51:01Z

runtime/autoload/health/nvim.vim

+      endif
+
+      if resolve(python_bin) !~# '^'.venv_root.'/'
+        call health#report_warn('Your virtualenv is not set up optimatlly.', [


optimally is spelled wrong here.

tweekmonster · 2016-06-21T04:53:58Z

Responding to your commit:

I'm still not sure about the notes item, or what should be done with that.

I used it as a way to move the messages below what I thought were the prime diagnostic information, such as version numbers and paths. Things that could be a dead give away to a problem.

I think how your examples look now is fine, but the provider#pythonx#Detect() output can get pretty verbose. Maybe it'll be less of an issue if message labels were colored to make it easier to distinguish the type of message?

tjdevries · 2016-06-22T23:33:16Z

Sure, color coding would be pretty cool! I think maybe that will be one of the features to add in future work, for nicer formatting. Adding it to the description above.

justinmk · 2016-06-23T01:23:14Z

Best way to determine whether a health checker function exists.

https://github.com/Shougo/unite.vim has a way of doing it. @Shougo how does unite check for unite sources implemented by third-party plugins? Can you link to the relevant code?

Should the check exit if health#report_error is called, or leave that up to the plugin maintainer?

report_error is only a cosmetic effect in the report, it should not cause an actual error. Control flow is totally up to the plugin author.

tweekmonster · 2016-06-23T03:11:56Z

runtime/autoload/health.vim


+      let l:report .= capture('call ' . l:checker[0] . '()')


Should this be put in a try block to catch unforeseen exceptions or will capture() grab the exception output?

See #4697 (comment)

capture('foo') will capture the exception and display it to the user

can be wrapped in try if you need to handle the exception in some way

capture('silent! foo') will capture the exception output, without showing it to the user

Thanks! Then, it would only make sense to use try if we want a checker to have an exit code (I can't think of a reason where we would). Otherwise, I think capture('silent! foo') would be the best way to go.

Shougo · 2016-06-24T05:05:42Z

https://github.com/Shougo/unite.vim has a way of doing it. @Shougo how does unite check for unite sources implemented by third-party plugins? Can you link to the relevant code?

This.
https://github.com/Shougo/unite.vim/blob/master/autoload/unite/init.vim#L545

unite.vim just search from 'runtimepath'.

Shougo · 2016-06-24T08:17:59Z

runtime/autoload/health/nvim.vim

+function! s:download(url) abort
+  let content = ''
+  if executable('curl')
+    let content = system('curl -sL "'.a:url.'"')


I think using list is better like this system(['curl', '-sL', a:url]).

I moved it to the new location of the documentation. Now the tags point to the correct place.

brcolow · 2016-07-20T05:31:45Z

runtime/doc/pi_health.txt

+Health.vim aims to solve this problem in two ways for both core and plugin
+maintainers.
+
+The way this is done is to provide an interface for that users will know to


remove extraneous "for"

justinmk · 2016-07-20T13:45:41Z

Let's not get tied up in the help doc, I'll tweak before merge. Is this functionally complete?

tjdevries · 2016-07-20T13:56:24Z

@justinmk Yes. I think so. The tests that I wrote are working as expected. It seems to find any correctly structured helper that is in the runtimepath. The output seems to go nicely straight to markdown, which will be good for pasting into github / other markdown supported places.

If there are any particular concerns, I will try and address those as well, but it seems functional to me.

Realized that I messed something up when I was trying to add something... I'll try and add it in a different PR later. Just moved the original call back to the function. Also fixed a systemlist problem

The old way would always enable all the checkers when you would run :CheckHealth, which is bad if people wanted to eliminate some checkers. So I made the interface consistent.

Fixed most of the suggestions that were given.

tjdevries · 2016-07-24T03:26:41Z

@justinmk Are you looking for anything else in particular currently? Should I change this to [RFC]?

justinmk · 2016-08-10T06:56:37Z

@tjdevries Working on merging this (#5205).

I can't find where we discussed the concept of "disabling" a checker. What is the use-case for that?

tjdevries · 2016-08-10T14:25:13Z

@justinmk I guess there were two main use cases:

Remove extraneous output (for example, if someone has 100 plugins, still a lot to try and sort through). This one is not that important
Remove a checker that might take a very long time to run it's checks, but is unrelated. For example, if the user has an older machine and a check for some plugin/feature/nvim check takes multiple minutes to run, you might want to disable that checker while trying to debug locally. This way you don't have to wait five minutes every time you run check health while you're trying to fix the local settings.

It's primarily because we add all the checker by default, I figured we should have a way to turn them off if someone desires.

justinmk · 2016-08-22T01:21:26Z

It's primarily because we add all the checker by default, I figured we should have a way to turn them off if someone desires.

Addressed this in #5205 by letting :CheckHealth take optional list of checkers (plugin names) to run.

:CheckHealth deoplete nvimux ...

tjdevries changed the title ~~Neovim Universal Healthcare~~ [WIP] Neovim Universal Healthcare Jun 17, 2016

marvim added the WIP label Jun 17, 2016

tjdevries force-pushed the neo-healthcare branch from 264a5ad to d0db84e Compare June 20, 2016 15:41

tweekmonster reviewed Jun 21, 2016
View reviewed changes

tweekmonster reviewed Jun 23, 2016
View reviewed changes

Shougo reviewed Jun 24, 2016
View reviewed changes

Move old reference to checkhealth

c2538f8

I moved it to the new location of the documentation. Now the tags point to the correct place.

tjdevries force-pushed the neo-healthcare branch from 7f75ae3 to c2538f8 Compare July 20, 2016 04:35

brcolow reviewed Jul 20, 2016
View reviewed changes

Small alignment adjustment

73e3e0b

tjdevries added 4 commits July 20, 2016 13:08

Move original call back to file

121fdc8

Realized that I messed something up when I was trying to add something... I'll try and add it in a different PR later. Just moved the original call back to the function. Also fixed a systemlist problem

Simplify interface.

7f77e0c

The old way would always enable all the checkers when you would run :CheckHealth, which is bad if people wanted to eliminate some checkers. So I made the interface consistent.

Add test for disabling checkers

26d13df

[WIP] Add more documentation

ff45d75

Fixed most of the suggestions that were given.

tjdevries force-pushed the neo-healthcare branch from dcba0a2 to ff45d75 Compare July 20, 2016 17:17

Finish up my thought on the docs

fa722d3

justinmk self-assigned this Jul 24, 2016

justinmk mentioned this pull request Aug 10, 2016

[RFC] CheckHealth #5205

Merged

justinmk changed the title ~~[WIP] Neovim Universal Healthcare~~ Neovim Universal Healthcare Aug 22, 2016

justinmk closed this in 2cc523c Aug 22, 2016

justinmk removed the WIP label Aug 22, 2016

tjdevries deleted the neo-healthcare branch May 26, 2017 02:57

justinmk mentioned this pull request Aug 26, 2018

Create a more convenient workflow to create error reports/dumps for plugins #4478

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Neovim Universal Healthcare #4932

Neovim Universal Healthcare #4932

tjdevries commented Jun 17, 2016 •

edited

ZyX-I commented Jun 17, 2016

Hettomei commented Jun 17, 2016

tjdevries commented Jun 17, 2016

fmoralesc commented Jun 17, 2016

ZyX-I commented Jun 17, 2016 •

edited

tjdevries commented Jun 17, 2016

justinmk commented Jun 17, 2016 •

edited

wsdjeg commented Jun 17, 2016

tjdevries commented Jun 20, 2016

justinmk commented Jun 20, 2016

tjdevries commented Jun 20, 2016

justinmk commented Jun 20, 2016

tjdevries commented Jun 20, 2016

tweekmonster commented Jun 20, 2016

tjdevries commented Jun 21, 2016 •

edited

tweekmonster Jun 21, 2016

tweekmonster commented Jun 21, 2016

tjdevries commented Jun 22, 2016

justinmk commented Jun 23, 2016 •

edited

tweekmonster Jun 23, 2016

justinmk Jun 23, 2016 •

edited

tweekmonster Jun 23, 2016

Shougo commented Jun 24, 2016

Shougo Jun 24, 2016

brcolow Jul 20, 2016

justinmk commented Jul 20, 2016

tjdevries commented Jul 20, 2016

tjdevries commented Jul 24, 2016

justinmk commented Aug 10, 2016

tjdevries commented Aug 10, 2016

justinmk commented Aug 22, 2016 •

edited

Neovim Universal Healthcare #4932

Neovim Universal Healthcare #4932

Conversation

tjdevries commented Jun 17, 2016 • edited

Context

Outline

Future work

Certain To Do's

Open Questions

ZyX-I commented Jun 17, 2016

Hettomei commented Jun 17, 2016

tjdevries commented Jun 17, 2016

fmoralesc commented Jun 17, 2016

ZyX-I commented Jun 17, 2016 • edited

tjdevries commented Jun 17, 2016

justinmk commented Jun 17, 2016 • edited

wsdjeg commented Jun 17, 2016

tjdevries commented Jun 20, 2016

justinmk commented Jun 20, 2016

tjdevries commented Jun 20, 2016

justinmk commented Jun 20, 2016

tjdevries commented Jun 20, 2016

tweekmonster commented Jun 20, 2016

tjdevries commented Jun 21, 2016 • edited

tweekmonster Jun 21, 2016

Choose a reason for hiding this comment

tweekmonster commented Jun 21, 2016

tjdevries commented Jun 22, 2016

justinmk commented Jun 23, 2016 • edited

tweekmonster Jun 23, 2016

Choose a reason for hiding this comment

justinmk Jun 23, 2016 • edited

Choose a reason for hiding this comment

tweekmonster Jun 23, 2016

Choose a reason for hiding this comment

Shougo commented Jun 24, 2016

Shougo Jun 24, 2016

Choose a reason for hiding this comment

brcolow Jul 20, 2016

Choose a reason for hiding this comment

justinmk commented Jul 20, 2016

tjdevries commented Jul 20, 2016

tjdevries commented Jul 24, 2016

justinmk commented Aug 10, 2016

tjdevries commented Aug 10, 2016

justinmk commented Aug 22, 2016 • edited

tjdevries commented Jun 17, 2016 •

edited

ZyX-I commented Jun 17, 2016 •

edited

justinmk commented Jun 17, 2016 •

edited

tjdevries commented Jun 21, 2016 •

edited

justinmk commented Jun 23, 2016 •

edited

justinmk Jun 23, 2016 •

edited

justinmk commented Aug 22, 2016 •

edited