Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Response of LSP Requests results in wrong position in editor #15900

Closed
7 of 10 tasks
hlouzada opened this issue Jun 18, 2021 · 3 comments · Fixed by #15903
Closed
7 of 10 tasks

Response of LSP Requests results in wrong position in editor #15900

hlouzada opened this issue Jun 18, 2021 · 3 comments · Fixed by #15903

Comments

@hlouzada
Copy link
Contributor

Issue Report Checklist

  • Searched the issues page for similar reports
  • Read the relevant sections of the Spyder Troubleshooting Guide and followed its advice
  • Reproduced the issue after updating with conda update spyder (or pip, if not using Anaconda)
  • Could not reproduce inside jupyter qtconsole (if console-related)
  • Tried basic troubleshooting (if a bug/error)
    • Restarted Spyder
    • Reset preferences with spyder --reset
    • Reinstalled the latest version of Anaconda
    • Tried the other applicable steps from the Troubleshooting Guide
  • Completed the Problem Description, Steps to Reproduce and Version sections below

Problem Description

Spyder always sends transformed document text EOL to LF in LSP requests. Therefore, when the server calculates an object position, it's considering LF as EOL. However, this results in a position mismatch if the document EOL is CRLF or CR as Spyder expects the position accordingly.

This could be server-sided solved, as the server could identify the document EOL and use that to convert the position accordingly, but I don't know if this approach breaks the (Language Server Protocol)[https://microsoft.github.io/language-server-protocol/]. Besides, the Server Implementation that was tested does not result in this bug with VSCode, so it probably should be solved client-sided.

What steps reproduce the problem?

  1. Open a document with CRLF as EOL
  2. Use the go-to reference (can be any LSP request that's position sensitive)
  3. Results in the wrong position.

What is the expected output? What do you see instead?

It is expected the correct object's reference in the editor, but it leads to a wrong position instead.

Paste Traceback/Error Below (if applicable)

No traceback is applicable.

Versions

  • Spyder version: 5.0.4
  • Python version: 3.8.5
  • Qt version: 5.12.10
  • PyQt version: 5.12.3
  • Operating System name/version: Windows-10-10.0.21390-SP0

Dependencies

# Mandatory:
atomicwrites >=1.2.0          :  1.4.0 (OK)
chardet >=2.0.0               :  4.0.0 (OK)
cloudpickle >=0.5.0           :  1.6.0 (OK)
cookiecutter >=1.6.0          :  1.7.2 (OK)
diff_match_patch >=20181111   :  20200713 (OK)
intervaltree >=3.0.2          :  3.1.0 (OK)
IPython >=7.6.0               :  7.19.0 (OK)
jedi =0.17.2                  :  0.17.2 (OK)
jsonschema >=3.2.0            :  3.2.0 (OK)
keyring >=17.0.0              :  21.8.0 (OK)
nbconvert >=4.0               :  6.0.7 (OK)
numpydoc >=0.6.0              :  1.1.0 (OK)
paramiko >=2.4.0              :  2.7.2 (OK)
parso =0.7.0                  :  0.7.0 (OK)
pexpect >=4.4.0               :  4.8.0 (OK)
pickleshare >=0.4             :  0.7.5 (OK)
psutil >=5.3                  :  5.8.0 (OK)
pygments >=2.0                :  2.8.1 (OK)
pylint >=1.0                  :  2.6.0 (OK)
pyls >=0.36.2;<1.0.0          :  0.36.2 (OK)
pyls_black >=0.4.6            :  0.4.6 (OK)
pyls_spyder >=0.3.2;<0.4.0    :  0.3.2 (OK)
qdarkstyle =3.0.2             :  3.0.2 (OK)
qstylizer >=0.1.10            :  0.1.10 (OK)
qtawesome >=1.0.2             :  1.0.2 (OK)
qtconsole >=5.1.0             :  5.1.0 (OK)
qtpy >=1.5.0                  :  1.9.0 (OK)
rtree >=0.9.7                 :  0.9.7 (OK)
setuptools >=39.0.0           :  56.0.0 (OK)
sphinx >=0.6.6                :  3.4.3 (OK)
spyder_kernels >=2.0.4;<2.1.0 :  2.0.4 (OK)
textdistance >=4.2.0          :  4.2.0 (OK)
three_merge >=0.1.1           :  0.1.1 (OK)
watchdog >=0.10.3             :  1.0.2 (OK)
zmq >=17                      :  22.0.3 (OK)

# Optional:
cython >=0.21                 :  None (NOK)
matplotlib >=2.0.0            :  3.3.3 (OK)
numpy >=1.7                   :  1.20.3 (OK)
pandas >=1.1.1                :  1.2.1 (OK)
scipy >=0.17.0                :  1.6.0 (OK)
sympy >=0.7.3                 :  1.7.1 (OK)

# Spyder plugins:
spyder_f_language             :  1.3.dev (OK)
@hlouzada
Copy link
Contributor Author

I can help with this problem if needed as it breaks compatibility with my language server project and Spyder.

@hlouzada
Copy link
Contributor Author

It seems that spyder.plugins.editor.widgets.codeeditor methods document_did_open, document_did_change and document_did_save uses toPlainText method to send the document text to the language server:

text = self.toPlainText()

text = self.toPlainText()

params['text'] = self.toPlainText()

And this method always returns the text with LF as EOL. A possible solution would be using the get_text_with_eol method:

text = self.get_text_with_eol()

Although this method defined in widgets.mixins could be improved to ignore when EOL is already LF:

    def get_text_with_eol(self):
        """Same as 'toPlainText', replace '\n'
        by correct end-of-line characters"""
        utext = to_text_string(self.toPlainText())
        linesep = self.get_line_separator()
        if linesep != "\n":
            lines = utext.splitlines()
            txt = linesep.join(lines)
            if utext.endswith('\n'):
                txt += linesep
            return txt
        else:
            return utext

@ccordoba12
Copy link
Member

ccordoba12 commented Jun 18, 2021

Hey @hlouzada, thanks a lot for reporting and investigating this problem. Please submit a pull request with the fix you consider necessary for it. That way it would be easier for us to understand what you have in mind and give you feedback about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants