Skip to content

Conversation

ccordoba12
Copy link
Member

@ccordoba12 ccordoba12 commented Jan 11, 2023

Description of Changes

  • Completions were not introduced correctly when one was selected in the completion widget after they were triggered automatically next to a dot.
  • Fix introducing completions for files and directories that start with a dot (they were not introduced correctly either).
  • Fix introducing completions for files and directories in empty strings, i.e. you can hit Tab in a empty string ("<Tab>") and Jedi will provide file completions in that case. The problem was the selected entry was not placed correctly in the editor either.
  • Fix introducing file and directory completions next to a path separator (i.e. os.sep).
  • Expand tests to cover all those cases.
  • Depends on Improve Jedi file completions for directories python-lsp/python-lsp-server#337

Issue(s) Resolved

Fixes #20331

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
Copy link
Member Author

ccordoba12 commented Jan 11, 2023

@mrclary, the conda-based installer for Mac is failing (I don't understand why). Could you check that? Thanks!

@mrclary
Copy link
Contributor

mrclary commented Jan 11, 2023

@mrclary, the conda-based installer for Mac is failing (I don't understand why). Could you check at that? Thanks!

It looks like the constructor was updated from 4.0b5 to 4.0b7. I'll investigate and submit a PR asap.

@ccordoba12
Copy link
Member Author

Great! Thanks a lot for your help with that 👍🏽

@mrclary
Copy link
Contributor

mrclary commented Jan 11, 2023

@ccordoba12, looks like work I already had in the pipeline (PR #20319) will fix the failing conda-based installers.

@ccordoba12
Copy link
Member Author

Ok, great news! I'll review it right away so we can include it in 5.4.2 then.

@ccordoba12 ccordoba12 force-pushed the issue-20331 branch 3 times, most recently from 052c352 to 3306dd7 Compare January 12, 2023 04:26
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 ! Checking this locally the only thing I manage to see that I'm not totally sure if it is correct is that the dot completion for filenames gives back the filename again instead of the extension for filename completions:

completion

Also, I left a suggestion in a comment for a typo and as we discussed, this needs a rebase to get the fixes done to the conda-based installers workflows

@ccordoba12
Copy link
Member Author

Checking this locally the only thing I manage to see that I'm not totally sure if it is correct is that the dot completion for filenames gives back the filename again instead of the extension for filename completions

Good catch @dalthviz! I fixed that use case in my last commit and added a test for it.

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.

Checked again but seems like there are still some cases not being correctly handle like a double dot for files:

completion2

@ccordoba12
Copy link
Member Author

@mrclary, I don't understand why the conda-based installers started to fail again. Could you take a look at that?

@mrclary
Copy link
Contributor

mrclary commented Jan 15, 2023

@mrclary, I don't understand why the conda-based installers started to fail again. Could you take a look at that?

So, the real issue is that I should be using git diff instead of git format-patch to generate the patch. I'll submit a PR for the change. I don't know why this only plagued Linux and not macOS... 🤷🏼‍♂️

@mrclary
Copy link
Contributor

mrclary commented Jan 15, 2023

@ccordoba12 #20376 should resolve the failing conda-installers

@mrclary
Copy link
Contributor

mrclary commented Jan 15, 2023

Actually, that resolved the initial patch issue when building Spyder. But now I see a package conflict error when building the installer...

@mrclary
Copy link
Contributor

mrclary commented Jan 15, 2023

Nevermind, I was normalizing the version, which apparently is not good.
So, all should be good now.

Also, expand test to check that an entry selected in the completion
widget when it's shown after a dot is introduced as expected.
- This correctly introduces completions for files that start with a dot
and when completions are requested at the beginning of a string.
- Expand tests to cover those cases.
…rver.git --branch=improve-file-completions --update --force external-deps/python-lsp-server

subrepo:
  subdir:   "external-deps/python-lsp-server"
  merged:   "dbf75cf19"
upstream:
  origin:   "https://github.com/ccordoba12/python-lsp-server.git"
  branch:   "improve-file-completions"
  commit:   "dbf75cf19"
git-subrepo:
  version:  "0.4.3"
  origin:   "https://github.com/ingydotnet/git-subrepo"
  commit:   "2f68596"
…rver.git --branch=develop --update --force external-deps/python-lsp-server

subrepo:
  subdir:   "external-deps/python-lsp-server"
  merged:   "11b54415d"
upstream:
  origin:   "https://github.com/python-lsp/python-lsp-server.git"
  branch:   "develop"
  commit:   "11b54415d"
git-subrepo:
  version:  "0.4.3"
  origin:   "https://github.com/ingydotnet/git-subrepo"
  commit:   "2f68596"
@ccordoba12
Copy link
Member Author

ccordoba12 commented Jan 17, 2023

@dalthviz, this is ready for another review. I fixed the last problem you reported, also fixed several issues with file completions next to path separators (i.e. os.sep), and added os.sep at the end of directory completions in the pylsp to ease introducing additional file completions.

@dalthviz
Copy link
Member

dalthviz commented Jan 17, 2023

@ccordoba12 checked again and the last issue is fixed 👍, however, with further testing, there is still a strange behavior for the completion of files names:

completion3

@ccordoba12
Copy link
Member Author

however, with further testing, there is still a strange behavior for the completion of files names

Yeah, I found that too while testing but there's no easy solution for it. The thing is we select the word under the cursor (using Qt) and send it to pylsp for completions. But in this case:

part.0.parq<Tab>

the selected word is parq because Qt considers that a word goes up to a dot. And pylsp can find no completions for it.

To solve this, we should override Qt and extend the search for current words to go up to part (the file name). But that's not so simple and it could introduce more corner cases and regressions. So, I think for now this is the best we can do to provide good enough file completions.

@ccordoba12 ccordoba12 merged commit 8ce602c into spyder-ide:5.x Jan 17, 2023
@ccordoba12 ccordoba12 deleted the issue-20331 branch January 17, 2023 22:52
ccordoba12 added a commit that referenced this pull request Jan 17, 2023
@luistrotta19
Copy link

Buenas tardes, qusiera que spyder proporcione detalles de los metodos al colocar el cursor encima del objeto, como lo hace VSC, antes se podía instalar kite, pero ahora no está disponible, hay algún sustituto?, gracias, saludos

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.

4 participants