Skip to content
This repository has been archived by the owner on Mar 27, 2018. It is now read-only.

Allow dash in command name #56

Closed
wants to merge 1 commit into from
Closed

Conversation

Hywan
Copy link
Member

@Hywan Hywan commented Nov 9, 2014

We just remove dashes in command name. For example, for the Hoa\Devtools\Bin\Requiresnapshot command, we can write:

$ hoa devtools:require-snapshot

instead of

$ hoa devtools:requiresnapshot

The first one is far more readable.

Of course, we can also write:

$ hoa devtools:r-e-q-u-i-r-e-s-n-a-p-s-h-o-t--------

and it will work.

Pro: help to read long commands.
Con: several “names” for the same command, which is not… good :-/.

Thoughts?

/cc @hoaproject/hoackers

@Hywan Hywan self-assigned this Nov 9, 2014
@Hywan
Copy link
Member Author

Hywan commented Nov 10, 2014

As a reminder: command names are class names. That's why it does not support dash so far.

@stephpy
Copy link
Member

stephpy commented Nov 10, 2014

Imo, command should have a name, a strict name.

We could implement alias system and user could use them for theses cases (requiresnapshot and require-snapshot)

@jubianchi
Copy link
Member

@Hywan I think handling several names for one command is not really a good thing as you say.

One thing that could be good would be to support several names based on a convention: command names are computed from the command classname. So why not splitting the name on camel-case:

  • Hoa\Devtools\Bin\Requiresnapshot => requiresnapshot
  • Hoa\Devtools\Bin\RequireSnapshot => require-snapshot and requiresnapshot

@jubianchi
Copy link
Member

👍 for @stephpy suggestion for an alias system and/or a fuzzy logic: requiresnap would translate to requiresnapshot if there is only one match

@stephpy
Copy link
Member

stephpy commented Nov 10, 2014

Hoa\Devtools\Bin\Requiresnapshot => requiresnapshot
Hoa\Devtools\Bin\RequireSnapshot => require-snapshot and requiresnapshot

Is easy to implement and a good thing.

@jubianchi There is not at this moment a CommandRepository where we could implements a alias|fuzzy logic without browsing all classes on Hoa*_\Bin_*.

@jubianchi
Copy link
Member

@stephpy the fuzzy logic is the extra-thing/bonus here :)

@Hywan
Copy link
Member Author

Hywan commented Nov 10, 2014

Fuzzy logic?

+1 @jubianchi but how to do that in a single preg_replace call?

@jubianchi
Copy link
Member

@Hywan fuzzy logic: http://blog.mageekbox.net/?post/2011/10/12/Docteur-Levenshtein%2C-je-pr%C3%A9sume

@Hywan preg_replace_callback should do the trick in a single pass ;)

@Hywan
Copy link
Member Author

Hywan commented Nov 10, 2014

We already such an approach for ambiguous options: https://github.com/hoaproject/Console/blob/bc13787e92dc3e1eae33932a8c96ee162e08f48f/GetOption.php. It is based on Hoa\String. By the way, this is not the topic.

I can't use preg_replace_callback because I would like to describe that behviors with the zFormat:

$out = preg_replace('#' . $l . '#', $r, $out);
.

@jubianchi
Copy link
Member

@Hywan zFormat ?! WTF ? :)

@Hywan
Copy link
Member Author

Hywan commented Nov 10, 2014

@jubianchi Read the docs ;-).

@Hywan
Copy link
Member Author

Hywan commented Nov 10, 2014

Ok, so I would like to use \U, \u, \L, \l, \E and \e but it is not suppported by PHP… Too bad for us. We have to implement this in the zFormat I think…

@Jir4
Copy link

Jir4 commented Feb 1, 2016

Still revelant ?

@Hywan
Copy link
Member Author

Hywan commented Feb 1, 2016

Yup! See #57. But we should move this in the… Hoa\Cli repository.

@Hywan
Copy link
Member Author

Hywan commented Feb 1, 2016

This issue was moved to hoaproject/Cli#14

@Hywan Hywan closed this Feb 1, 2016
@Hywan Hywan removed the in progress label Feb 1, 2016
@Hywan
Copy link
Member Author

Hywan commented Feb 1, 2016

Done.

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

Successfully merging this pull request may close these issues.

None yet

4 participants