Skip to content

Conversation

@rsenden
Copy link
Contributor

@rsenden rsenden commented Nov 17, 2021

This PR fixes #1466

@remkop
Copy link
Owner

remkop commented Nov 17, 2021

This is great, thank you!
Can you add a test for this, or modify one of the existing tests?

@remkop remkop added this to the 4.6.3 milestone Nov 17, 2021
@remkop remkop added theme: auto-completion An issue or change related to auto-completion type: bug 🐛 labels Nov 17, 2021
@rsenden
Copy link
Contributor Author

rsenden commented Nov 17, 2021

I just added aliases to the existing AutoComplete tests, and updated the expected outputs. The only thing that I'm not sure about is why in AutoCompleteTest#testComplete() I need to repeat each alias three times. Any idea?

@remkop
Copy link
Owner

remkop commented Nov 18, 2021

The only thing that I'm not sure about is why in AutoCompleteTest#testComplete() I need to repeat each alias three times. Any idea?

I will have to take a look in the debugger to see what is going on. It may take some time before I can get to this.

@remkop
Copy link
Owner

remkop commented Nov 24, 2021

I finally had some time to take a look at this.
It appears that the AutoComplete#filterAndTrimMatchingPrefix method does not filter out duplicates, even though it should. This can be fixed by modifying the first line of this method to use a Set instead of a List:

    private static void filterAndTrimMatchingPrefix(String prefix, List<CharSequence> candidates) {
        Set<CharSequence> replace = new HashSet<CharSequence>();
        ...

The expectations in testComplete can then be changed to also have only one occurrence of each of the subcommand aliases, which makes much more sense.

Can you change the PR accordingly?

Copy link
Owner

@remkop remkop left a comment

Choose a reason for hiding this comment

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

I also found some minor things with variables used in the test result templates that should be preserved. Can you take a look?

@rsenden
Copy link
Contributor Author

rsenden commented Nov 24, 2021

PR has been updated with all requested changes

@remkop remkop merged commit ff96cc9 into remkop:master Nov 24, 2021
@remkop
Copy link
Owner

remkop commented Nov 24, 2021

Merged.
Thank you for the contribution!

I will update the release notes when I get to my PC.

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

Labels

theme: auto-completion An issue or change related to auto-completion type: bug 🐛

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Autocompletion script doesn't properly handle aliases

2 participants