Skip to content

feat(ssh-keyscan): new completion #532

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

Merged
merged 2 commits into from
Jul 6, 2021
Merged

feat(ssh-keyscan): new completion #532

merged 2 commits into from
Jul 6, 2021

Conversation

scop
Copy link
Owner

@scop scop commented Jun 4, 2021

No description provided.

@scop scop force-pushed the feat/ssh-keyscan branch 2 times, most recently from 4f72f6b to 621e0f4 Compare June 6, 2021 06:55
@scop scop force-pushed the feat/ssh-keyscan branch from 621e0f4 to d07e980 Compare June 30, 2021 06:53
@scop scop requested a review from akinomyoga June 30, 2021 10:35
@akinomyoga
Copy link
Collaborator

Thank you for updating the PR. I have tested the new commit and found that the fields before the comma is removed when there are multiple completions for the string after the comma. For example, in the following cases, it works as expected:

$ ssh-keyscan -t rsa,TAB TAB
dsa      ecdsa    ed25519     # <- the list is shown
$ ssh-keyscan -t rsa,dTAB
$ ssh-keyscan -t rsa,dsa     # <- correctly completed

But in the following ambiguous case, the fields before comma is deleted by TAB.

$ ssh-keyscan -t rsa,eTAB
$ ssh-keyscan -t e     # <- "rsa," is lost

@scop
Copy link
Owner Author

scop commented Jul 5, 2021

Seems what you describe happens with set show-all-if-ambiguous off only, I have it set to on and it works as expected for me (Ubuntu 20.04, bash 5.0.17(1)-release). Opened #552 for that, I suggest we don't wait for that here.

@akinomyoga
Copy link
Collaborator

OK, fine! I now approve this PR for merging.

@scop scop merged commit a00b719 into master Jul 6, 2021
@scop scop deleted the feat/ssh-keyscan branch July 6, 2021 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants