[READY] JavaScript completer using Tern.js #272

puremourning · 2015-12-12T17:31:31Z

Overview

Adds support for a JavaScript completer using Tern.js.

The completer supports:

Intelligent auto-completion
Go to definition, references (GoTo, etc.)
Semantic type information for identifiers (GetType)
View documentation comments for identifiers (GetDoc)
Management of tern server instance

This provides (for Vim at least) a replacement, for tern_for_vim automagically in ycmd.

Demo

Completions:

Subcommands:

Installation

Run install.py --tern-completer

Caveats and differences vs. tern_for_vim

Currently, to use it there must be a .tern-project configuration file somewhere in the directory tree above Vim's working directory when the first JavaScript file is opened (i.e. when the server starts). I believe this is also true for tern_for_vim.
Does not support refactoring (rename) which tern_for_vim does
Methods from Object.prototype are always included. Tern has the ability to only include them when they match some number of typed characters, as they are otherwise "noise". This is incompatible with ycmd's cache and LCS matching.

Status

I believe this to be fully working with the above caveats. I think the first one is the most frustrating, but it isn't obvious how to avoid it.

Prototype supports: - completion - GetType - GetDoc - GoTo and GoToDefinition - automatic server management

…gging. Write logs to files to work with test suite

…his is needed

…l broken due to tern.js behaviour

This also seems to resolve issues around completions with a query string. \o/ Add more tests for completions and a basic test for subcommands (before rebasing on master where this approach has been changed) Improve some of the test utilities to allow specifying unsaved buffer contents and to check documentation, etc. on completions. Tidy up documentation output for completions and for GetDoc. Mimic tern_for_vim for the former, but use our own style for the latter.

Fix a bug where these would return spurious JSON error when run without an identifier - catch the HTTP error that tern returns. Also add GoToReferences which returns a list of the refs

The "installation" requires running 'npm install --production' in the third-party/tern directory. We detect the lack of the tern_modules directory and disable our completer. This ensures that anyone not specifying '--tern-completer' doesn't get it and therefore it won't conflict with existing users' tern_for_vim installations.

…lter than for them to never appear

puremourning · 2015-12-12T18:32:08Z

HMMMMMM.

The appveyor builds are failing, unable to find npm in build.py.

I've tried two approaches:

here
and here

@micbou any clues why that might be?

I can see that npm must be there because appveyor.yaml has uses it

puremourning · 2015-12-12T18:55:09Z

I think I got it. it's npm.cmd not npm.exe

puremourning · 2015-12-12T19:08:36Z

Grumble. Very grumble.

The problem is https://bugs.python.org/issue2200

i.e. the extension .cmd cannot be used with distutils.spawn.find_executable so we have to do its job for it.

micbou · 2015-12-12T19:43:26Z

Welcome to the joy of Windows.

The problem is https://bugs.python.org/issue2200
i.e. the extension .cmd cannot be used with distutils.spawn.find_executable so we have to do its job for it.

You can reuse the FindExecutable function in ycmd/utils.py.

I am not reviewing the tests because they will change with PR #270.

Review status: 0 of 27 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

ycmd/completers/javascript/tern_completer.py, line 164 [r1] (raw file):
Using localhost on Windows makes the request to the server very slow (at least on my configuration). Replace it by 127.0.0.1. Should always work.

ycmd/completers/javascript/tern_completer.py, line 199 [r1] (raw file):
Same comment as above. Maybe add a method to obtain the target (_ServerLocation for example).

ycmd/completers/javascript/tern_completer.py, line 280 [r1] (raw file):
Unfortunately, this will fail on Windows. You need to specifically call the Tern binary with the node executable.

Comments from the review on Reviewable.io

puremourning · 2015-12-12T20:17:22Z

Thanks @micbou 'preciate it!

I've fired up my Windows VM to get this to work. :)

Review status: 0 of 27 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

Comments from the review on Reviewable.io

puremourning · 2015-12-12T21:31:25Z

So strange.

Now i'm getting strange EPERM errors when tern is started by my completer, but not when run manually.

The completer runs it with the following command line:

2015-12-12 21:17:10,700 - DEBUG - Starting tern with the following command: C:\Program Files\nodejs\node.exe C:\Users\Ben\vimfiles\bundle\YOUCOM~1\THIRD_~1\ycmd\ycmd\completers\javascript\..\..\..\third_party\tern\bin\tern --port 49867 --host localhost --persistent --no-port-file --verbose

But the process dies immediately with:

events.js:142
      throw er; // Unhandled 'error' event
      ^

Error: EPERM: operation not permitted, read
    at Error (native)

If I run the same command from the command line, it works. If I run it in python interactive shell, it works too.

>>> node
'C:\\Program Files\\nodejs\\node.exe'
>>> tern = os.path.dirname( 'ycmd\\completers\\javascript\\tern_completer.py' )
>>> tern
'C:\\Users\\Ben\\vimfiles\\bundle\\YouCompleteMe-Clean\\third_party\\ycmd\\third_party\\tern\\bin\\tern'
>>> command = [ node, tern, '--host', 'localhost', '--persistent', '--verbose' ]
>>> import subprocess
>>> subprocess.Popen( command )
<subprocess.Popen object at 0x000000000241AF98>
>>> Listening on port 49881

The only difference seems to be the "shortened" path name with the tilde ~ in it. Could that be related?

Clues? Tips? ;_;

puremourning · 2015-12-12T21:38:20Z

Even with the same path in command line python it works:

C:\Users\Ben\vimfiles\bundle\YouCompleteMe-Clean\third_party\ycmd>python
Python 2.7.10 (default, May 23 2015, 09:44:00) [MSC v.1500 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import os, subprocess
>>> import ycmd.utils
>>> node = ycmd.utils.FindExecutable( 'node' )
>>> tern = 'C:\\Users\\Ben\\vimfiles\\bundle\\YOUCOM~1\\THIRD_~1\\ycmd\\ycmd\\completers\\javascript\\..\\..\\..\\third_party\\tern\\bin\\tern'
>>> tern
'C:\\Users\\Ben\\vimfiles\\bundle\\YOUCOM~1\\THIRD_~1\\ycmd\\ycmd\\completers\\javascript\\..\\..\\..\\third_party\\tern\\bin\\tern'
>>> command = [ node, tern, '--verbose' ]
>>> subprocess.Popen( command )
<subprocess.Popen object at 0x00000000024842B0>
>>> Listening on port 49907

^C
C:\Users\Ben\vimfiles\bundle\YouCompleteMe-Clean\third_party\ycmd>

I must be going mad. I even tried with raw Popen rather than utils.SafePopen - no joy.

More grumble :p

micbou · 2015-12-12T21:40:47Z

Let me try your tern-completer-locking-windows branch.

Review status: 0 of 27 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

Comments from the review on Reviewable.io

puremourning · 2015-12-12T21:46:18Z

i pushed the fixed to build.py just now

puremourning · 2015-12-12T21:46:32Z

thanks, btw. sorry to be a pain :(

micbou · 2015-12-12T22:13:19Z

Redirecting stdout and stderr to the logfiles makes the server crash. Need to understand why.

Review status: 0 of 27 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

Comments from the review on Reviewable.io

puremourning · 2015-12-12T22:14:32Z

Interesting. It works if i reduce it all into one file and run it manually:

import os
import sys

sys.path.append( os.path.join( os.path.dirname( __file__ ), '..', '..', '..' ) )

print sys.path

from ycmd.utils import ( PathToFirstExistingExecutable, SafePopen,
        GetUnusedLocalhostPort, PathToTempDir )


PATH_TO_TERNJS_BINARY = os.path.abspath( 
    os.path.join(
      os.path.dirname( __file__ ),
      '..',
      '..',
      '..',
      'third_party',
      'tern',
      'bin',
      'tern' ) )


node_path = PathToFirstExistingExecutable( [ 'node' ] )

if not node_path:
  raise RuntimeError( 'Unable to find node executable' )
else:
  print ( 'Using node binary from: ' + node_path )


server_port = GetUnusedLocalhostPort()

if True:
  extra_args = [ '--verbose' ]
else:
  extra_args = []

command = [ node_path,
            PATH_TO_TERNJS_BINARY,
            '--port',
            str( server_port ),
            '--host',
            '127.0.0.1',
            '--persistent',
            '--no-port-file' ] + extra_args

print( 'Starting tern with the following command: ' + ' '.join( command ) )

logfile_format = os.path.join( PathToTempDir(), u'tern_{port}_{std}.log' )

server_stdout = logfile_format.format(
    port = server_port,
    std = 'stdout' )

server_stderr = logfile_format.format(
    port = server_port,
    std = 'stderr' )

try:
  with open( server_stdout, 'w' ) as stdout:
    with open( server_stderr, 'w' ) as stderr:
      server_handle = SafePopen( command,
                                 stdout = stdout,
                                 stderr = stderr )
except Exception:
  print( 'Unable to start Tern.js server' )
  server_port = 0


if ( server_port > 0 and server_handle is not None 
    and server_handle.poll() is None ):
  print ( 'Tern.js Server started with pid: ' +
                str( server_handle.pid ) +
                ' listening on port ' +
                str( server_port ) )
  print ( 'Tern.js Server log files are: ' +
                server_stdout +
                ' and ' +
                server_stderr )

server_handle.wait()

Then i can use postman in chrome to talk to the server.

puremourning · 2015-12-12T22:19:16Z

Hmmm. Yes. Removing stdout= and stderr= from Popen call fixes it, but the output from the server is lost in the ether. subprocess.PIPE also causes it to crash (and also loses the output).

puremourning · 2015-12-12T22:30:18Z

I even hacked something within waitress/bottle and that works, so it isn't that either.

Valloric · 2015-12-13T20:13:03Z

Reviewed 1 of 2 files at r4, 2 of 2 files at r5.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

build.py, line 30 [r3] (raw file):
I remember there was a good reason why we didn't want to import stuff in the scripts, though I can't remember why.

The duplication is shitty but we'll live with it.

Comments from the review on Reviewable.io

puremourning · 2015-12-13T20:22:21Z

I've marked this as [READY], though AppVeyor is my nemesis today - seemingly impossibly slow. I'll see if I can massage it.

Also, do you want me to rebase this into one commit ?

Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks broke.

Comments from the review on Reviewable.io

micbou · 2015-12-13T20:26:35Z

I think there is an issue with AppVeyor. All tests pass but the run does not finish. It could be that the Tern server is still running.

Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks broke.

Comments from the review on Reviewable.io

puremourning · 2015-12-13T20:27:08Z

that's what i was just thinking. I will check it out.

micbou · 2015-12-13T20:28:10Z