Add the numbers format console menus, showing the validation options in 2 columns. #682

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

fitorec commented Jun 4, 2012

Add the numbers format console menus(on CotrollerTask, ModelTask), showing the validation options in 2 columns(ModelTask), Normalizing the array $ConsoleOutput::_styles.

fitorec added some commits Jun 4, 2012

@fitorec fitorec Add the numbers format console menus, showing the validation options …
…in 2 columns.

 Author:      @Fitorec <chanerec@gmail.com>
 Date:        lun jun  4 01:01:22 CDT 2012
 Description: Add the numbers format console menus(on CotrollerTask, ModelTask), showing the validation options in 2 columns(ModelTask), Normalizing the array $ConsoleOutput::_styles
 -------------------------------------------------------------------------
 lib/Cake/Console/Command/Task/ControllerTask.php |    3 +-
 lib/Cake/Console/Command/Task/ModelTask.php      |   25 +++++++++++++--------
 lib/Cake/Console/ConsoleOutput.php               |    2 +-
 3 files changed, 18 insertions(+), 12 deletions(-)
c7b821f
@fitorec fitorec Removing the case
 Author:      @Fitorec <chanerec@gmail.com>
 Date:        lun jun  4 01:20:27 CDT 2012
 Description: Removing the case in ModelTask - pull request #682
 -------------------------------------------------------------------------
 lib/Cake/Console/Command/Task/ModelTask.php |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
d39e062

@markstory markstory commented on an outdated diff Jun 4, 2012

lib/Cake/Console/Command/Task/ModelTask.php
- $prompt = '';
- for ($i = 1; $i < $defaultChoice; $i++) {
- $prompt .= $i . ' - ' . $this->_validations[$i] . "\n";
+ for ($i = 1, $m=$defaultChoice/2; $i < $m; $i++) {
+ $str_aux = sprintf("%2d. %s", $i, $this->_validations[$i]);
@markstory

markstory Jun 4, 2012

Owner

variable names should be camelBacked.

@markstory markstory commented on an outdated diff Jun 4, 2012

lib/Cake/Console/Command/Task/ModelTask.php
- $prompt = '';
- for ($i = 1; $i < $defaultChoice; $i++) {
- $prompt .= $i . ' - ' . $this->_validations[$i] . "\n";
+ for ($i = 1, $m=$defaultChoice/2; $i < $m; $i++) {
@markstory

markstory Jun 4, 2012

Owner

Missing spaces around operators.

@markstory markstory commented on an outdated diff Jun 4, 2012

lib/Cake/Console/Command/Task/ModelTask.php
foreach ($options as $i => $option) {
- $this->out($i + 1 . '. ' . $option);
+ $this->out(sprintf("%${len}d. %s", $i+1, $option));
@markstory

markstory Jun 4, 2012

Owner

spaces are required around operators.

@fitorec fitorec Adding coding style.
 Author:      @Fitorec <chanerec@gmail.com>
 Date:        lun jun  4 07:57:14 CDT 2012
 Description: Add coding style from @ markstory comments.
 -------------------------------------------------------------------------
 lib/Cake/Console/Command/Task/ModelTask.php |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)
ae4aed1
Contributor

fitorec commented Jun 4, 2012

Hi!, I just made the changes relevant to the style of coding, the curious thing is that before I pull requests I had tested with phpcs.

Contributor

fitorec commented Jun 4, 2012

The tests I did with https://github.com/jrbasso/CodeSniffer_CakePHP

These rules of spaces between operators, so camel for variables, is not the CodeSniffer_CakePHP detected?,or maybe I have it configured incorrectly.

Contributor

sitedyno commented Jun 4, 2012

Contributor

fitorec commented Jun 4, 2012

@sitedyno thanks for the info now install this version.

Owner

markstory commented Jun 5, 2012

Why do your commit messages contain the shortlog of changes made?

@markstory markstory commented on an outdated diff Jun 5, 2012

lib/Cake/Console/Command/Task/ModelTask.php
- $prompt = '';
- for ($i = 1; $i < $defaultChoice; $i++) {
- $prompt .= $i . ' - ' . $this->_validations[$i] . "\n";
+ for ($i = 1, $m = $defaultChoice / 2; $i < $m; $i++) {
+ $strAux = sprintf("%2d. %s", $i, $this->_validations[$i]);
+ strAux = $strAux.str_repeat(" ", 31 - strlen(strAux));
@markstory

markstory Jun 5, 2012

Owner

This and the line below contain parse errors.

Parse error: syntax error, unexpected '=' in lib/Cake/Console/Command/Task/ModelTask.php on line 415

@fitorec fitorec Repairing error of the variable named.
 Author:      @Fitorec <chanerec@gmail.com>
 Date:        lun jun  4 19:54:45 CDT 2012
 Description: I'm wrong and give an error of substituting the name of the variable $strAux does not add the prefix '$', so I just checked.
32a5eac
Contributor

fitorec commented Jun 5, 2012

@markstory some people like this information in the commits/pullRequests, is my hook prepare-commit-msg(is basically a git diff - stat - staged).

Member

rchavik commented Jun 5, 2012

i don't think these stats are relevant for commit messages. They are relevant for merges though.

Owner

markstory commented Jun 5, 2012

@fitorec Ah ok, I'll just take them out as they aren't necessary in CakePHP. We're not nearly as picky :)

Contributor

fitorec commented Jun 5, 2012

I'm thinking of adding support for type "Enum-> inlist" (MySQL) and "dateTime" in the ModelTask->fieldValidation.

Maybe the following instructions:

<?php
if ($metaData['null'] != 1 && !in_array($fieldName, array($primaryKey, 'created', 'modified', 'updated'))) {
    if ($fieldName == 'email') {
        $guess = $methods['email'];
    } 
    /*... */
    } elseif ($metaData['type'] == 'datetime') { //datetime support
        $guess = $methods['datetime'];
    } elseif (strrpos($metaData['type'], 'enum') === 0) { //Enum support
        $guess = $methods['inlist'];
    }
}

What do you think?

Owner

markstory commented Jun 5, 2012

Enum column types are not supported, so I don't know why they'd have validation suggestions.

Contributor

fitorec commented Jun 5, 2012

@markstory I made the change locally, in the run it to create the test files I generate a error.

I think the enum could do with the type inlist.

I would like to review in the near future, if they think the support to enum is a good change, I can try to add this support to cake 🍰

Just add the change to datetime?:

<?php
    /*... */
    } elseif ($metaData['type'] == 'datetime') { //datetime support
        $guess = $methods['datetime'];
    }
@fitorec fitorec Adding case 'datetime' for the selection of $guess
 Description: in ModelTask->fieldValidation added option "datetime"
b6e99f6

@markstory markstory added a commit that referenced this pull request Jun 11, 2012

@markstory markstory Merge branch 'fitorec-console' into 2.2
Closes #GH-682
fddcdd6
Owner

markstory commented Jun 11, 2012

I reworded the commits and merged it into 2.2

markstory closed this Jun 11, 2012

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