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

🐛 Fix PowerShell completion with incomplete word #360

Merged
merged 10 commits into from
Aug 24, 2024

Conversation

patricksurry
Copy link
Contributor

@patricksurry patricksurry commented Feb 23, 2022

Fixes #359

Standard example in documentation failed because incomplete word was not removed from cwords, so typer thought we were adding an additional parameter which wouldn't be allowed. Completing on a List[] parameter appeared to work because then a subsequent parameter would be allowed.

Also handles the case where we are completing somewhere other than the end of the line, by ignoring text past the cursor position and then completing normally.

@patricksurry patricksurry changed the title Fixes powershell completion with incomplete word 🐚 Fixes powershell completion with incomplete word Feb 23, 2022
@github-actions
Copy link

📝 Docs preview for commit 148e1b2 at: https://639cea8c57a0d303631b5efc--typertiangolo.netlify.app

@github-actions
Copy link

github-actions bot commented Jun 5, 2023

📝 Docs preview for commit 4e8ff9d at: https://647d42deaf2e9e11716f1c45--typertiangolo.netlify.app

@svlandeg svlandeg added bug Something isn't working p2 labels Mar 6, 2024
Copy link
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

I can confirm the bug with Powershell: there are no auto-completion suggestions when typing "Ca" or "Se" on master. This PR resolves the issue.

Two points I'd like to address/look into before we could merge this PR:

  • Is the _TYPER_CURSOR_POSITION variable actually necessary? The buggy behaviour is still solved on my system if I remove the if cursor lines and only keep the fix of the args definition
  • Ideally we'd have a unit test that captures this behaviour and fails on master. I'm looking into this.

@svlandeg svlandeg self-assigned this Jul 9, 2024
@svlandeg
Copy link
Member

svlandeg commented Jul 16, 2024

Update:

  • I've removed the _TYPER_CURSOR_POSITION variable from this PR. With the current PR, Powershell completes in the middle of the line even without it, so for now I think we should go with the most minimal fix and potentially reconsider building on top of it if we do run into more issues afterwards
  • I've added a unit test that fails on master and succeeds on this PR.

With that, I think this is good to merge.

@svlandeg svlandeg removed their assignment Jul 16, 2024
@svlandeg svlandeg changed the title 🐚 Fixes powershell completion with incomplete word 🐛 Fix powershell completion with incomplete word Jul 16, 2024
@tiangolo tiangolo changed the title 🐛 Fix powershell completion with incomplete word 🐛 Fix PowerShell completion with incomplete word Aug 24, 2024
@tiangolo
Copy link
Member

Great, thank you for the contribution (and the patience) @patricksurry! 🍰

And thank you @svlandeg for the thorough review, tweaks, and help. 🙇 ☕

This will be available in Typer 0.12.5 released in the next few hours. 🎉

@tiangolo tiangolo merged commit 70ffd68 into fastapi:master Aug 24, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p2 shell / powershell
Projects
None yet
Development

Successfully merging this pull request may close these issues.

powershell completion fails with partial word because it includes the incomplete word with completed args
3 participants