Skip to content

add an example of finding close matches #1152

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

Open
wants to merge 30 commits into
base: main
Choose a base branch
from
Open

add an example of finding close matches #1152

wants to merge 30 commits into from

Conversation

phlptp
Copy link
Collaborator

@phlptp phlptp commented Apr 21, 2025

continue discussion on #1149
Adds subcommand prefix matching as a modifier to CLI.
Adds and example of close matching logic for further exploration.

@jzakrzewski
Copy link

That looks OK, but I'd really love some simple API to enable that behaviour.
As a bonus, similar thing prefix matching would be great (for example myapp i, myapp ins, myapp install would all do the same).

@jzakrzewski
Copy link

I like that. Something like this could be a great help to the user of our apps.

I'm wondering, however, if the prefix match could also be implemented for the parser itself - basically as long as the prefix is unambiguous, the corresponding command would be used. Kinda-sorta what NPM does, but better (npm in still runs npm install, where it should error out because there's npm init).

@phlptp
Copy link
Collaborator Author

phlptp commented Apr 30, 2025

I added prefix matching modifier to the app class, That will work for prefix matching on subcommands. I will add some more documentation and a few more tests yet, but review comments/suggestions would be appreciated.

@jzakrzewski
Copy link

Well, for me that looks beautiful!
Thank you :)

@phlptp
Copy link
Collaborator Author

phlptp commented May 2, 2025

I integrated prefix matching and the example has the close matching output. @henryiii what do you think of the prefix matching. I am not ready to integrate the close matching as a standard feature available that will take a different PR. But I think the prefix matching is ready or close to merging.

@phlptp phlptp requested a review from henryiii May 7, 2025 12:44
@phlptp
Copy link
Collaborator Author

phlptp commented May 9, 2025

@henryiii need your thoughts on this one

@cxw42
Copy link

cxw42 commented May 11, 2025

@phlptp Thanks for taking this up so quickly! Just to check, should this PR be prefix-match only?

Based on your comment, should the "close match"/Levenshtein material be removed?

@phlptp
Copy link
Collaborator Author

phlptp commented May 12, 2025

Right now the close match/levenshtein matching is in an example and the test is against that example. The comment was referring to trying to get that capability into the main files, which I am not ready to try yet. But I am fine with it being in an example of how that might be done. I want to get the prefix matching in first then start thinking about what the best way of including the close match stuff into the main header files, or whether or not that should be included at all.

@phlptp
Copy link
Collaborator Author

phlptp commented May 16, 2025

@henryiii Going to merge this in two days unless you have further comments.

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.

3 participants