Repository: spyder-ide/spyder
Pull request: PR: Add option to connect to external PyLS servers
<!--- Make sure to read the Contributing Guidelines:                   --->
<!--- https://github.com/spyder-ide/spyder/blob/master/CONTRIBUTING.md --->
<!--- and follow PEP 8, PEP 257 and Spyder's code style:               --->
<!--- https://github.com/spyder-ide/spyder/wiki/Dev:-Coding-Style      --->

## Description of Changes

* [ ] Wrote at least one-line docstrings (for any new functions)
* [ ] Added unit test(s) covering the changes (if testable)
* [x] Included a screenshot (if affecting the UI)

<!--- Explain what you've done and why --->

![imagen](https://user-images.githubusercontent.com/16781833/56710483-04c75c80-66ec-11e9-8fe3-ed46c0f0d2e4.png)

Properly set the `external` property for the lsp-server

### Issue(s) Resolved

<!--- List the issue(s) below, in the form "Fixes #1234"; one per line --->

Fixes #9195 


### Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the [Developer Certificate of Origin](https://developercertificate.org)
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

<!--- TYPE YOUR USER/NAME AFTER THE FOLLOWING: --->
I certify the above statement is true and correct: dalthviz

<!--- Thanks for your help making Spyder better for everyone! --->


Problem statement:
Cannot connect to an external PyLS server
<!--- **PLEASE READ:** When submitting here, please ensure you've completed the following checklist and checked the boxes to confirm. Issue reports without it may be closed. Thanks! --->
## Problem Description

The new menu does not allow to connect to an external PyLS server. 

### What steps reproduce the problem?

1. Run spyder-pyls server on one terminal with the command `python -m pyls -vvv --port 2089 --tcp`
2. Run spyder on another terminal
3. Try to connect with the running server by changing the port in Preferences > Completion and linting > Advanced 
4. After giving apply, there's no new connection received by the local pyls server.
* The same happens after trying to setup a new server for python.

### What is the expected output? What do you see instead?
Connect to an external PyLS server running locally.

## Versions
<!--- You can get this information from Help > About Spyder...
or (if Spyder won't launch) the "conda list" command
from the Anaconda Prompt/Terminal/command line. --->

* Spyder version: Spyder 4.0.0.dev0 (Commit: ba26876de) 
* Python version: 3.6.8
* Qt version: 5.12.2
* PyQt version: 5.12.1 
* Operating System name/version: Darwin 18.2.0 

### Dependencies
<!--- Please go to the menu entry Help > Dependencies,
press the Copy to clipboard button and paste below --->

```
pygments >=2.0    :  2.3.1 (OK)
qdarkstyle >=2.6.4:  2.6.5 (OK)
sphinx >=0.6.6    :  1.8.5 (OK)
psutil >=0.3      :  5.6.1 (OK)
pyls >=0.19.0     :  0.21.5 (OK)
nbconvert >=4.0   :  5.4.1 (OK)
pandas >=0.13.1   :  None (NOK)
numpy >=1.7       :  1.16.2 (OK)
sympy >=0.7.3     :  None (NOK)
cython >=0.21     :  None (NOK)
qtconsole >=4.2.0 :  4.4.3 (OK)
IPython >=4.0     :  7.4.0 (OK)
matplotlib >=2.0.0:  None (NOK)
pylint >=0.25     :  2.3.1 (OK)
```

Hints from issue discussion:
@dalthviz, I was wrong when I instructed you to assume that external servers can only live in external machines. As @steff456 pointed out, external servers are simply servers started outside Spyder.

So to fix this we need to add a new checkbox in `Completion and linting > Advanced Setttings` (below the current options) with this text:

    [ ] This is an external server

associated to the option `advanced/external` and being `False` by default. Then we need to use that option to set the value of `self.python_config['external']` in `update_python_config` of `LSPManager`.

Commit message: LSP: Fix lsp-client restart after changing options for python

Patch to review:
diff --git a/spyder/config/main.py b/spyder/config/main.py
index 0948fd1cbe2..52659a26dcf 100755
--- a/spyder/config/main.py
+++ b/spyder/config/main.py
@@ -698,6 +698,7 @@
               'advanced/command_launch': 'pyls',
               'advanced/host': '127.0.0.1',
               'advanced/port': 2087,
+              'advanced/external': False
              })
             ]
 
diff --git a/spyder/plugins/editor/lsp/manager.py b/spyder/plugins/editor/lsp/manager.py
index 847ee526894..bb6e9ddf762 100644
--- a/spyder/plugins/editor/lsp/manager.py
+++ b/spyder/plugins/editor/lsp/manager.py
@@ -82,7 +82,7 @@ def get_language_config(self, language):
         """Get language configuration options from our config system."""
         if language == 'python':
             self.update_python_config()
-            return self.python_config
+            return self.python_config.copy()
         else:
             return self.get_option(language)
 
@@ -301,14 +301,16 @@ def update_python_config(self):
             'follow_imports': self.get_option('jedi_definition/follow_imports')
         }
 
+        # Advanced
+        advanced_external = self.get_option('advanced/external')
+
         # Setup options in json
         self.python_config['cmd'] = cmd
         if host in self.LOCALHOST:
             self.python_config['args'] = '--host {host} --port {port} --tcp'
-            self.python_config['external'] = False
         else:
             self.python_config['args'] = ''
-            self.python_config['external'] = True
+        self.python_config['external'] = advanced_external
         self.python_config['host'] = host
         self.python_config['port'] = port
 
diff --git a/spyder/preferences/languageserver.py b/spyder/preferences/languageserver.py
index 029286c2426..3aa56a17247 100644
--- a/spyder/preferences/languageserver.py
+++ b/spyder/preferences/languageserver.py
@@ -862,6 +862,9 @@ def setup_page(self):
             word_wrap=False)
         advanced_port = self.create_spinbox(
             ":", "", 'advanced/port', min_=1, max_=65535, step=1)
+        advanced_external = self.create_checkbox(
+            _("This is an external server"),
+            'advanced/external')
 
         # Advanced layout
         advanced_g_layout = QGridLayout()
@@ -879,6 +882,7 @@ def setup_page(self):
         advanced_layout = QVBoxLayout()
         advanced_layout.addWidget(advanced_label)
         advanced_layout.addLayout(advanced_g_layout)
+        advanced_layout.addWidget(advanced_external)
         advanced_widget.setLayout(advanced_layout)
 
         # --- Other servers tab ---


Code review comments:
diff_hunk: @@ -82,7 +82,7 @@ def get_language_config(self, language):
         """Get language configuration options from our config system."""
         if language == 'python':
             self.update_python_config()
-            return self.python_config
+            return self.python_config.copy()
path: spyder/plugins/editor/lsp/manager.py
text: @user1:
Why is this needed? I mean, it shouldn't be necessary.

@author:
While testing the changes, the restart was not being triggered (the `current_config` and `new_config` in `update_server_list` were the same). This was caused because the client for `python` was being affected for the change in `self.python_config` (adding the `copy` prevents this to happen)

@user1:
Ok, I understand what you mean now and that's a serious problem because by using a shared `self.python_config` attribute we are modifying the configuration of the existing Python client and of future clients too.

To avoid that, please do the following:

1. Remove `self.python_config` from `LSPManager`
2. Rename `update_python_config` to `generate_python_config`
3. Use a local variable inside that method called `python_config`, like this:

    ```python
    def generate_python_config(self):
    """
    Generate Python server configuration with the options saved in our
    config system
    """
    python_config = PYTHON_CONFIG.copy()
    ...
    ...
    return python_config
    ```

    and use `python_config` instead of `self.python_config` in that method.

diff_hunk: @@ -301,14 +301,16 @@ def update_python_config(self):
             'follow_imports': self.get_option('jedi_definition/follow_imports')
         }
 
+        # Advanced
+        advanced_external = self.get_option('advanced/external')
path: spyder/plugins/editor/lsp/manager.py
text: @user1:
Let's change this to

    external_server = self.get_option('advanced/external')

@user1:
You changed the names of other variables, but forgot to change precisely this name ;-)