Continuing my journey on getting that si command working... long options require the equal sign after them while short options do not even work if you add an equal sign after them. This is awfully confusing.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | options.diff | 9.91 KB | moshe weitzman |
Comments
Comment #1
chx commentedFor example, using
--sites-subdir example.comcreates a sites subdir called 1. Srsly.Comment #2
greg.1.anderson commented#0 is fairly standard in *nix (c.f. git).
#1 could be fixed with better error handling.
Comment #3
moshe weitzman commentedRight, this is standard in Unix AFAIK. I agree that it is confusing and masochistic even. But deviating from Unix standard seems pretty unwise.
Comment #4
chx commentedWell, this is never a problem with git because the manpage always gives you the option fully. Say,
git checkout [-p|--patch] [<tree-ish>] [--] <pathspec>...no equal sign is needed.--author=<pattern>, --committer=<pattern>. Equal sign is needed. It beats you with a cluestick, basically.Comment #5
greg.1.anderson commentedIn a drush command record, we should add:
Then we could make drush output --sites-subdir= instead of --sites-subdir. Then drush could speak softly and carry a big cluestick too.
The only reason this is going to e a tedious rather than trivial change is that mode help options today use the short form, so they're all going to have to be converted into arrays. I will try to carve out time to do this soon, but I might not be successful in the next week.
Comment #6
greg.1.anderson commentedOh, there are some options where the = is optional. For example, --drupal-project-rename is the same as --drupal-project-rename=drupal. The help should be --drupal-project-rename=, but we do not want to complain if the value is not provided.
Perhaps we need:
'value-description' => 'directory_name'
and
'value' => 'optional'
Valid values for the 'value' help option item could be 'required', 'optional', and 'none'. Default could be 'none' if there is no value-description, and 'required' if value-description is specified.
Comment #7
moshe weitzman commentedThis will be a lot of work to review existing options and code them properly but I really like the idea. These sorts of improvements go a long long way.
Comment #8
jonhattanI've think several times in validating accepted values for command options. Here's a proposal to validate options and its values:
'default-value' is only useful if 'value' is optional. Even in that case it may be NULL / not provided, meaning that the value is to be calculated later (e.g:
si --admin-passis randomly generated orsql-dump --result-filecreate a date based filename). An alternative is to provide instead a 'default-value-callback' key.'valid-values' allow to specify an array of valid values (e.g. pm-download profile --variant=full | project |"profile only"). The callback alternative could be useful for example to validate hostnames in the case of site-dir.
Comment #9
greg.1.anderson commentedI don't think it would be advisable to change the function signature for drush_get_option at this stage of drush-5. That means that default values would have to be implemented in terms of drush_set_option on the 'default' context. I don't think I'm excited about attaching callbacks to items in a drush context. I therefore suggest that callbacks is a drush-6 feature, at best. I don't know that I'm excited about doing it there, either.
Comment #10
greg.1.anderson commentedI suppose we could call the callback functions as soon as the command record is parsed, and stash the result in the default context. That wouldn't be too hard to manage.
Comment #11
moshe weitzman commentedI'd like to focus on chx example from #4. We need a way to show the equal when needed and add a [<treeish>] for example. This issue is mostly about better help, IMO
Comment #12
greg.1.anderson commentedYes, let's keep this issue about better help. I'm mildly negative on the callbacks still, but that can be discussed elsewhere.
Comment #13
moshe weitzman commentedWe actually already have support for example-value. This patch adds 'value' key defaults to 'required' but can also be 'optional'. Optional keys get square braces and required get angle backets. Patch adds support in core.drush.inc and global options.
I think this is an improvement, but it does duplicate a lot of info already found in description and/or in examples section.
Here is some output:
Comment #14
greg.1.anderson commentedAwesome. I like it.
Comment #15
moshe weitzman commentedCommitted in 7dca167. I took a quick run through the rest of the commands and expanded the options that I thought would benefit from example values.
I'm still on the fence about whether we do as chx asked and warn when an option requires a value. This is a pretty big change. We might even have to just mandate that options stop using simple name => description pairs and start using name => array() so that we know for sure which ones take values. I'm thinking that this is a drush6 thing. I'm open to discussion about next steps. Leaving this open for now.
Comment #16
jonhattanThis commit was causing a lot of
fixed in http://drupalcode.org/project/drush.git/commit/c1e02f6
Comment #17
greg.1.anderson commentedThis issue was marked
closed (won't fix)because Drush has moved to Github.If this feature is still desired, you may copy it to our Github project. For best results, create a Pull Request that has been updated for the master branch. Post a link here to the PR, and please also change the status of this issue to
closed (duplicate).Please ask support questions on Drupal Answers.