Language Server Protocol Support for Brackets #14606

jhasubhash · 2018-12-04T13:37:13Z

This covers the first cut of Language Server Protocol Support for Brackets.

Basic core implementation to:

Initiate/Stop a Language Server
Interact with Language Server
- Completions
- Signature Help
- Diagnostics
- Jump To Dec/Decl/Impl
- Symbols
- References
- Synchronization Events (Text Document didOpen, didChange etc.)
- Custom Request/Notification (so the developer can use commands not exposed in the initial implementation)
Bidirectional Request/Notification system for communication between Brackets and Language Server
Abstraction of LSP from Brackets (as far as possible)
Creation of Language Client that works as the handle to Language Server from Brackets side
Support for all communication types: IPC, Pipe, Socket and Standard I/O

TODO:

This feature can be experienced with the reference Language Client implementation for PHP here : #14671

…ucture for client capabilities

…ode cleanup

… from client code

ingorichter

I can’t test it myself right now, but I wanted to leave a couple of comments before I’ll look into the functional details. Besides the feature implementation I recommend to run the linter and ensure that the formatting is consistent with all the remaining brackets sources (there might be inconsistencies too). Since this is a lot of new code we should ensure that it’s properly formatted and documented. Adding test cases is crucial for this sort of functionality. Adding a sample LSP client would help to demonstrate the use of the API and helps with the testing of this great new feature.

ingorichter · 2018-12-27T20:14:38Z

src/lsp/ClientTemplate.js

+define(function (require, exports, module) {
+    "use strict";
+
+    class Client{


Suggested change

class Client{

class Client {

Please ensure that the formatting is consistent across the files, e.g. white space between text and { as an example.

ingorichter · 2018-12-27T20:15:31Z

src/lsp/LSPInterface.js

@@ -0,0 +1,150 @@
+


Nit: obsolete empty line

ingorichter · 2018-12-27T20:22:57Z

src/lsp/LSPInterface.js

+     */
+    function receiveMessageFromServer(event, msgObj) {
+
+        if(msgObj.param.method && _callbackList[msgObj.serverName+msgObj.param.method]){


I would assert that msgObj is defined and param exists before checkin for method. Same applies for accessing _callbackList. There is a some potential to create a variable and assign the constructed key msgObj.serverName+msgObj.param.

ingorichter · 2018-12-27T20:24:16Z

src/lsp/LSPInterface.js

+ *
+ */
+
+ define(function (require, exports, module) {


I suggest to write unit tests for all this new functionality to ensure that edge cases and error conditions are properly excercises

ingorichter · 2018-12-27T20:28:10Z

src/lsp/ParameterHintManager.js

+     * otherwise.
+     */
+    function isHintDisplayed() {
+        return hintState.visible === true;


Suggested change

return hintState.visible === true;

return hintState.visible;

should be enough ;-)

ingorichter · 2018-12-27T20:30:17Z

src/lsp/ParameterHintManager.js

+     *
+     */
+    function dismissHint() {
+        if (hintState.visible) {


nit: you could call isHintDisplayed instead

Fixing Promise related Issues

Set Server Capability in Start call

Review Comments & Bug Fixes

petetnt

Some comments :) haven't worked with Brackets codebase in a while so there are couple of questions, couple of smaller nits and maybe one or two issues. Generally looks good to me otherwise

petetnt · 2019-03-29T16:48:09Z

src/extensions/default/JavaScriptCodeHints/ParameterHintsProvider.js

+        functionInfo = functionInfo || this.session.getFunctionInfo();
+        if (!functionInfo.inFunctionCall) {
+            this.cleanHintState();
+            result.reject(null);


Q: Should/can this return early?

Suggested change

result.reject(null);

return result.reject(null);

Fixed. Thanks for the catch. It can return early. Basically, since we were returning a promise which could be resolved only once, it was still functioning correctly.

src/extensions/default/JavaScriptCodeHints/ParameterHintsProvider.js

petetnt · 2019-03-29T17:00:53Z

src/features/PriorityBasedRegistration.js

+     *     language IDs for languages to remove the provider for. Defaults
+     *     to all languages.
+     */
+    RegistrationHandler.prototype.removeProvider = function (provider, targetLanguageId) {


Are private methods still underscored by practice? If they are this should probably be _removeProvider

Actually, the function isn't meant to be private. Fixed the documentation.

src/languageTools/DefaultEventHandlers.js

src/languageTools/DefaultProviders.js

src/languageTools/LanguageClient/package.json

src/languageTools/LanguageTools.js

petetnt · 2019-03-29T18:35:52Z

src/languageTools/PathConverters.js

@@ -0,0 +1,82 @@
+/*


This file is the same as LanguageClient/Utils.js, should they just use same file?

LanguageClient here would work in the Node Context while Utils works in the CEF/Brackets context. The function APIs and logic are the same but there are subtle differences internally wherein the former one we use node.js built-ins and browser ones in the later.

petetnt · 2019-03-29T18:38:10Z

src/languageTools/styles/default_provider_style.css

+    text-overflow: ellipsis;
+    line-height: 1em;
+    -webkit-line-clamp: 2;
+    -webkit-box-orient: vertical;


Deprecated property?

Actually currently we're running on slightly older versions of CEF 2623 in Win, 2704 on Mac and 2785 in Linux, where the property works fine. We're using this property elsewhere in brackets core as well, so I think we should do the cleanup collectively together as an issue when we upgrade CEF.

petetnt · 2019-03-29T18:40:55Z

src/languageTools/styles/default_provider_style.css

+}
+
+.highlight .hint-doc {
+    display: -webkit-box;


Should this and -webkit-box-orient just be display: flex and flex-direction: column?

See my comment above.

src/languageTools/LanguageClient/LanguageClient.js

Addressing Review Comments

shubhsnov · 2019-04-01T09:34:02Z

@petetnt Thanks for the review. I've addressed the comments.

Fix author

Fixing Author Information

swmitra

LGTM

swmitra · 2019-03-21T08:21:41Z

src/brackets.js

+    require("languageTools/DefaultProviders");
+    require("languageTools/DefaultEventHandlers");
+
+	//load language features


nit: fix indentation.

* LSP Initial set of changes * Adding comments and a bit of cleanup * Adding php client for lsp * further cleanup * removing dependency on HintUtils * removing phpClient extension from this branch * Cleanup * fixing eslint errors * Refactoring code- Removing dependency on JSUtils ANd adding basic structure for client capabilities * Bug Fix: too many listeners were getting attached to node process + code cleanup * putting null check and settign capabilities to default values * reinitializing server on workspace change and moving out capabilities from client code * cleanup * First cut for LSP support in Brackets * First cut for LSP support in Brackets * Adding client infrastructure * Adding client infrastructure * Adding handlers on Language Client Proxy, fixing eslint errors * Adding handlers on Language Client Proxy, fixing eslint errors * Fixing protocol adapter * Fixing protocol adapter * Fix typo * Fix typo * Removing older implementation * Removing older implementation * Added error checks to the auto update mechanism. So in case the auto update mechansim fails, we will now give chance to the default update process Handler to handle the update mechanism (Which is essentially taking the user to brackets.io). (#14605) * First cut for LSP support in Brackets * First cut for LSP support in Brackets * Adding client infrastructure * Adding client infrastructure * Adding handlers on Language Client Proxy, fixing eslint errors * Adding handlers on Language Client Proxy, fixing eslint errors * Fixing protocol adapter * Fixing protocol adapter * Fix typo * Fix typo * Removing older implementation * Removing older implementation * Removing custom comments * Removing custom comments * Fixing Typo * Fixing Typo * Add missing Params in function call * Add missing Params in function call * Correcting message type, handlers * Correcting message type, handlers * Minor correction on active project change * Minor correction on active project change * Correcting the message format for didChange * Correcting the message format for didChange * Changing custom notification and request handlers, correcting typo, adding catch block for Connection * Changing custom notification and request handlers, correcting typo, adding catch block for Connection * Stop Creation of Multiple Language Servers * Stop Creation of Multiple Language Servers * Make Language Client Generic, address review comments * Make Language Client Generic, address review comments * Correcting param descriptions * Correcting param descriptions * Modifying events handling logic for Language Client, add formatting option for communication params * Modifying events handling logic for Language Client, add formatting option for communication params * Add handlers for node side * Add handlers for node side * Removing explicit param creation, substituting with appropriate checks * Removing explicit param creation, substituting with appropriate checks * Fixing lint errors in MessageHandler.js * Fixing lint errors in MessageHandler.js * Messaging related cleanup * Messaging related cleanup * Adding default providers and feature managers * Adding default providers and feature managers * Adding banner and fixing lint error * Adding banner and fixing lint error * fix spacing issue * fix spacing issue * Fix spacing issues * Fix spacing issues * Add filetype checks for all events, minor server info corrections * Add filetype checks for all events, minor server info corrections * Handling Reload with Extension Scenario, minor JumpToDef provider fix * Handling Reload with Extension Scenario, minor JumpToDef provider fix * Correcting Typo * Correcting Typo * Adding bug fixes * Adding bug fixes * Adding bug fixes 2 * Adding bug fixes 2 * Addressing Review: Fixing minor typo * Addressing Review: Fixing minor typo * Minor bug fixes, functionality enhancements * Minor bug fixes, functionality enhancements * Adding tests for Language Server Support: first cut * Adding tests for Language Server Support: first cut * Adding banner, fixing lint errors * Adding banner, fixing lint errors * Adding dependency related tasks * Adding dependency related tasks * Fixing npm environment string * Fixing npm environment string * Changing handler name * Changing handler name * Changing file name to ClientLoader * Changing file name to ClientLoader * Changing variable name appropriately * Changing variable name appropriately * Grunt related changes for build * Grunt related changes for build * Adding additional requests and notifications for handling various scenarios * Adding additional requests and notifications for handling various scenarios * Adding Path Converter Utilities * Adding Path Converter Utilities * Changing Ternary operator to OR operater * Changing Ternary operator to OR operater * Addressing review comments * Addressing review comments * Removing the handler for editor change, will be handled explicitely * Removing the handler for editor change, will be handled explicitely * Patching JavaScriptCodeHints * Patching JavaScriptCodeHints * Preferences infra for LanguageTools * Preferences infra for LanguageTools * Fixing JS ParameterHints * Fixing JS ParameterHints * Fixing Default Parameter Hints Provider * Fixing Default Parameter Hints Provider * Fixing Path Converters * Fixing Path Converters * Fixing Lint in PathConverters * Fixing Lint in PathConverters * Retaining Posix Path on Win * Retaining Posix Path on Win * Fixing lint errors * Fixing lint errors * Fixing Node side Utils * Fixing Node side Utils * Fixing Promise related Issues * Fixing Promise related Issues * Set Server Capability in Start call * Set Server Capability in Start call * Review Comments & Bug Fixes * Review Comments & Bug Fixes * Addressing Review Comments * Addressing Review Comments * Fixing Lint * Fixing Lint Co-authored-by: Shubham Yadav <shubyada@adobe.com>

grire974 · 2019-05-02T18:48:49Z

Wrong place for this comment I'm sure - but I just saw this feature (looks awesome thanks!) can't find documentation anywhere on how to implement it; for example can I install any of these LSP's?
https://langserver.org/

Some of the LSP's in this list (e.g. I'm interested in Java at the moment) have examples on how to install them on VSCode for example; I looked on the brackets wiki & don't see anything there either under Extensions->Languages that makes any mention of this new functionality.

shubhsnov · 2019-05-03T06:13:24Z

@grire974 This is a new feature and we're actively working on making the documentation better and more accessible for the developers. You can look at this page to get started.

We've tried to make sure that the Server integration part isn't too far off from how it works in VSCode, so basically, any extension which works for VSCode will work for us and often in the same way in terms of code pertaining to spawning a language server or interacting with it.

Please reach out to us in case you have any queries in the form of issues. Thanks for your interest.

grire974 · 2019-05-03T20:20:21Z

Ah ok - that's pretty informative thanks. Sounds like then to answer my own question that the LSP's I linked to at langserver.org above might not work out of the box but could possibly be a starting point to modify into the brackets format (e.g. main.js etc...)? I've written a couple of basic brackets extensions before, so this could be a good next project for me!

Subhash Jha added 13 commits November 18, 2018 17:36

LSP Initial set of changes

2bfb400

Adding comments and a bit of cleanup

ac24b4c

Adding php client for lsp

0a585d5

further cleanup

58bdca0

removing dependency on HintUtils

908af27

removing phpClient extension from this branch

1d9a45f

Cleanup

27592bd

fixing eslint errors

fb1a9fc

Refactoring code- Removing dependency on JSUtils ANd adding basic str…

95d2db2

…ucture for client capabilities

Bug Fix: too many listeners were getting attached to node process + c…

1639d42

…ode cleanup

putting null check and settign capabilities to default values

c577588

reinitializing server on workspace change and moving out capabilities…

9c133bc

… from client code

cleanup

32772c0

swmitra requested review from peterflynn and ingorichter December 12, 2018 17:48

ingorichter reviewed Dec 27, 2018

View reviewed changes

shubhsnov and others added 14 commits January 30, 2019 10:15

First cut for LSP support in Brackets

b9d747e

First cut for LSP support in Brackets

0a73384

Adding client infrastructure

3fe4fb7

Adding client infrastructure

4355925

Adding handlers on Language Client Proxy, fixing eslint errors

03bfd7e

Adding handlers on Language Client Proxy, fixing eslint errors

45377bf

Fixing protocol adapter

a2ad0a5

Fixing protocol adapter

a86ceab

Fix typo

a300e1b

Fix typo

10e5e93

Merge branch 'shubham/LanguageTools' into subhash/brackets/master

0018109

Merge branch 'shubham/LanguageTools' into subhash/brackets/master

e4bb9ac

Removing older implementation

d156822

Removing older implementation

29474b1

shubhsnov and others added 6 commits March 28, 2019 15:18

Fixing Promise related Issues

e89f52c

Fixing Promise related Issues

7a55ba0

Merge pull request #33 from subhashjha333/subhashMaster

1acd762

Fixing Promise related Issues

Set Server Capability in Start call

2bb01b2

Set Server Capability in Start call

603c07d

Merge pull request #34 from subhashjha333/subhashMaster

a859b9a

Set Server Capability in Start call

swmitra requested a review from petetnt March 29, 2019 16:36

shubhsnov and others added 3 commits March 29, 2019 23:18

Review Comments & Bug Fixes

2e49aed

Review Comments & Bug Fixes

466c7f9

Merge pull request #35 from subhashjha333/subhashMaster

4b65e19

Review Comments & Bug Fixes

petetnt reviewed Mar 29, 2019

View reviewed changes

jhasubhash commented Apr 1, 2019

View reviewed changes

src/languageTools/LanguageClient/LanguageClient.js Show resolved Hide resolved

shubhsnov and others added 5 commits April 1, 2019 13:26

Addressing Review Comments

bc06314

Addressing Review Comments

c8d38aa

Fixing Lint

3b972f3

Fixing Lint

398be65

Merge pull request #36 from subhashjha333/subhashMaster

56a12b1

Addressing Review Comments

shubhsnov added 2 commits April 1, 2019 22:37

Merge pull request #37 from subhashjha333/fixAuthor

0a28890

Fix author

Merge pull request #38 from subhashjha333/subhashMaster

9f46857

Fixing Author Information

swmitra approved these changes Apr 2, 2019

View reviewed changes

swmitra merged commit df7aea4 into adobe:master Apr 2, 2019

shubhsnov mentioned this pull request Apr 2, 2019

Language Server Protocol Support for Brackets - Fix Author #14678

Merged

This was referenced Aug 30, 2021

[CLOSED] Language Server Protocol Support for Brackets brackets-archive/bracketsIssues#11952

Open

[CLOSED] Language Server Protocol Support for Brackets - Fix Author brackets-archive/bracketsIssues#11972

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Language Server Protocol Support for Brackets #14606

Language Server Protocol Support for Brackets #14606

jhasubhash commented Dec 4, 2018 •

edited by shubhsnov

Loading

ingorichter left a comment

ingorichter Dec 27, 2018

ingorichter Dec 27, 2018

ingorichter Dec 27, 2018

ingorichter Dec 27, 2018

ingorichter Dec 27, 2018

ingorichter Dec 27, 2018

petetnt left a comment

petetnt Mar 29, 2019

shubhsnov Apr 1, 2019

petetnt Mar 29, 2019

shubhsnov Apr 1, 2019

petetnt Mar 29, 2019

shubhsnov Apr 1, 2019

petetnt Mar 29, 2019

shubhsnov Apr 1, 2019 •

edited

Loading

petetnt Mar 29, 2019

shubhsnov Apr 1, 2019

shubhsnov commented Apr 1, 2019

swmitra left a comment

swmitra Mar 21, 2019

grire974 commented May 2, 2019

shubhsnov commented May 3, 2019

grire974 commented May 3, 2019

Language Server Protocol Support for Brackets #14606

Language Server Protocol Support for Brackets #14606

Conversation

jhasubhash commented Dec 4, 2018 • edited by shubhsnov Loading

This feature can be experienced with the reference Language Client implementation for PHP here : #14671

ingorichter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petetnt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shubhsnov Apr 1, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shubhsnov commented Apr 1, 2019

swmitra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grire974 commented May 2, 2019

shubhsnov commented May 3, 2019

grire974 commented May 3, 2019

jhasubhash commented Dec 4, 2018 •

edited by shubhsnov

Loading

shubhsnov Apr 1, 2019 •

edited

Loading