ternjs / tern_for_sublime

Sublime Text package adding Tern support
MIT License
803 stars 54 forks source link

Fix crash on completion due to incorrect str/list concatenation #140

Closed ronjouch closed 7 years ago

ronjouch commented 7 years ago

Triggering completion used to cause a

Traceback (most recent call last):
  File "/opt/sublime_text/sublime_plugin.py", line 591, in on_query_completions
    res = callback.on_query_completions(v, prefix, locations)
  File "/home/ronj/.config/sublime-text-3/Packages/tern_for_sublime/tern.py", line 63, in on_query_completions
    completions, fresh = ensure_completions_cached(pfile, view)
  File "/home/ronj/.config/sublime-text-3/Packages/tern_for_sublime/tern.py", line 415, in ensure_completions_cached
    data = run_command(view, {"type": "completions", "types": True, "includeKeywords": True})
  File "/home/ronj/.config/sublime-text-3/Packages/tern_for_sublime/tern.py", line 294, in run_command
    port, port_is_old = server_port(pfile.project)
  File "/home/ronj/.config/sublime-text-3/Packages/tern_for_sublime/tern.py", line 160, in server_port
    started = start_server(project)
  File "/home/ronj/.config/sublime-text-3/Packages/tern_for_sublime/tern.py", line 173, in start_server
    proc = subprocess.Popen(tern_command + tern_arguments, cwd=project.dir, env=env,
TypeError: Can't convert 'list' object to str implicitly

Sublime Text 3 build 3125, Ubuntu Linux 16.04.1LTS

marijnh commented 7 years ago

The problem is that you set tern_command to a string in your config, whereas it is documented to be an array of strings.

ronjouch commented 7 years ago

The problem is that you set tern_command to a string in your config

Aaah, <facepalm>, should have checked the docs before proposing this naive patch.

whereas it is documented to be an array of strings.

@marijnh but maybe there's a problem here. The convention in the Sublime ecosystem is to self-document settings by listing their default values in the packages Settings - Default, and let the user copy+modify things into Settings - User. But tern_for_sublime doesn't follow this pattern, the Settings - Default just list three settings, omitting tern_command and tern_arguments:

{
    "tern_argument_hints": false,
    "tern_output_style": "tooltip",
    "tern_argument_completion": false
}

If "tern_command": ["<default>", "<value>"] had been present in this list, I would have understood its type and would have followed it. Instead, I just tried something that would fix my Exception.

Also, Sublime's pseudo-JSON parser accepting comments, it's a common practice to add a one-liner actually documenting the setting. Following this and the documentation I can extract from the README, tern_for_sublime's Settings - Default would become:

// Place your settings in tern_for_sublime's "Settings - User",
// which overrides the settings in here.
{
    // whether to show argument hints (may impact responsiveness on slow machines or big projects).
    "tern_argument_hints": false,
    // status, panel, tooltip. Defaults to tooltip when available, otherwise status
    "tern_output_style": "tooltip",
    // auto-complete function arguments (similar to IDEs like eclipse).
    "tern_argument_completion": false,
    // the command to execute to start a Tern server. Use if your node/tern is not in $PATH
    "tern_command": ["node", "/path/to/tern/bin/tern"],
    // extra set of arguments to pass to the Tern server. For example, --no-port-file to suppress creation of .tern-port files
    "tern_arguments": []
}

This makes for concise documentation that is at your fingertips when you need it (touch a setting). There's one nit: "path/to/tern/bin/tern" is invalid, maybe it could be stored as a magic constant and set at startup by tern.py only if still equals to this default invalid value.

What do you think?


Off-topic, thanks for Eloquent Javascript :)