Skip to content
This repository has been archived by the owner on Sep 25, 2019. It is now read-only.

[RFC] Add handler for unknown commands #362

Merged
merged 2 commits into from
Jun 5, 2017

Conversation

abdulhannanali
Copy link
Contributor

Summary

Right now, as @pirelenito mentioned in #360 we need a handler for unknown actions, which cause the sagui to exit with non zero code, alerting the developers about the problems if any in their setup or sagui. This adds a very minimalistic solution in sagui.

However, I do have a few suggestions, we can use to improve this even further. I hope @pirelenito can look into it too after vacation.

  • Add helpful Did you mean messages, in order to tell the user, about the nearest matching option if any
  • Maybe it should automatically redirect to --help

If the current solution is good enough, it's okay too. 👍 I am using exit code 1 in this case.

@abdulhannanali abdulhannanali force-pushed the unknown-action-handler branch from 5595ec9 to c415593 Compare May 31, 2017 20:54
@abdulhannanali abdulhannanali changed the title Add handler for unknown commands [RFC] Add handler for unknown commands May 31, 2017
@xaviervia
Copy link
Member

Thanks a lot! This is a great first step. I think redirecting to --help is a good idea; of course adding suggestions would be really nice, but I don’t know what the complexity if that would be. Maybe there is a simple library to handle that.

@Kadrei
Copy link

Kadrei commented Jun 2, 2017

one could easily calculate the levenstheindistance and suggest the one command with the least.

@xaviervia
Copy link
Member

Cool, I did find a couple of Levenshtein distance calculator functions:

I assume one can run whatever command the user sent and compute the distance to all the possible commands, and if one goes below a threshold (say, 3) output that one as a suggestion; otherwise, show the --help

Makes sense?

@pirelenito
Copy link
Member

I think that the PR is great as it is, given this fixes the more urgent problem.

Regarding @xaviervia's suggestion, could we have it as a separated PR?

Wdyt?

Thanks again @abdulhannanali.

@xaviervia
Copy link
Member

I agree with you @pirelenito , I think this is good as it is. Refinements can come as separate releases. Go for it if you want.

@pirelenito
Copy link
Member

@abdulhannanali please update your branch from v9, I did a fix that should make your build green (#363)

@abdulhannanali
Copy link
Contributor Author

abdulhannanali commented Jun 5, 2017

@pirelenito Sounds good, I will do it as a separate PR. Thanks a lot @Kadrei for suggestions. I am going to use leven in order to calculate the distance.

Thanks @xaviervia for a great outlook on this problem. I will update with progress on a separate PR

@abdulhannanali
Copy link
Contributor Author

@pirelenito Did that, hope the build passes.

@pirelenito pirelenito merged commit a4264b9 into saguijs:v9 Jun 5, 2017
@abdulhannanali abdulhannanali deleted the unknown-action-handler branch June 5, 2017 23:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants