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

PR: Preserve EOL characters when formatting text (Editor) #17555

Merged
merged 7 commits into from
Mar 28, 2022

Conversation

ccordoba12
Copy link
Member

@ccordoba12 ccordoba12 commented Mar 26, 2022

Description of Changes

  • We were not formatting text correctly for eol's different than LF.
  • This requires new versions of python-lsp-server (1.4.1+, already released) and python-lsp-black (1.2.0+, not released yet but working on it).
  • This also expands our test suite to cover those cases.

Issue(s) Resolved

Fixes #17143

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct:

@ccordoba12 ccordoba12 added this to the v5.3.0 milestone Mar 26, 2022
@ccordoba12 ccordoba12 requested a review from dalthviz March 26, 2022 19:53
@ccordoba12 ccordoba12 self-assigned this Mar 26, 2022
@pep8speaks
Copy link

pep8speaks commented Mar 26, 2022

Hello @ccordoba12! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 80:80: E501 line too long (85 > 79 characters)
Line 83:80: E501 line too long (94 > 79 characters)

Line 67:80: E501 line too long (85 > 79 characters)
Line 70:80: E501 line too long (94 > 79 characters)

Comment last updated at 2022-03-28 17:11:18 UTC

This is necessary to fix formatting with eol's different than LF for
Autopep8 and Yapf.
…cordoba12/python-lsp-server.git external-deps/python-lsp-server

subrepo:
  subdir:   "external-deps/python-lsp-server"
  merged:   "a38339494"
upstream:
  origin:   "https://github.com/ccordoba12/python-lsp-server.git"
  branch:   "fix-yapf-crlf"
  commit:   "a38339494"
git-subrepo:
  version:  "0.4.3"
  origin:   "https://github.com/ingydotnet/git-subrepo"
  commit:   "2f68596"
Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ccordoba12 ! Checked this locally and seems like this fixes #17143 👍

However, checking #16549 , the breakpoints still disappear with the changes here:

break

Should we remove this as a fix for #16549 ? Otherwise this LGTM 👍

@ccordoba12
Copy link
Member Author

ccordoba12 commented Mar 28, 2022

Should we remove this as a fix for #16549 ? Otherwise this LGTM

If that's the case, then sure (I'll move that issue to the 5.3.1 milestone). I didn't bother to check because I thought almost surely this problem was connected to that issue, but thanks for double checking.

Let me add one last commit to restore the subrepo to its default branch, then you can merge this one.

…lsp/python-lsp-server.git external-deps/python-lsp-server

subrepo:
  subdir:   "external-deps/python-lsp-server"
  merged:   "62cdac598"
upstream:
  origin:   "https://github.com/python-lsp/python-lsp-server.git"
  branch:   "develop"
  commit:   "62cdac598"
git-subrepo:
  version:  "0.4.3"
  origin:   "https://github.com/ingydotnet/git-subrepo"
  commit:   "2f68596"

[ci skip]
@dalthviz dalthviz merged commit d0603f6 into spyder-ide:5.x Mar 28, 2022
@ccordoba12 ccordoba12 deleted the issue-17143 branch March 28, 2022 17:22
dalthviz added a commit that referenced this pull request Mar 28, 2022
@dalthviz dalthviz mentioned this pull request Mar 28, 2022
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants