nette / php-generator

šŸ˜ Generates neat PHP code for you. Supports new PHP 8.3 features.
https://doc.nette.org/php-generator
Other
2.11k stars 138 forks source link

Wrong wrapping condition in array formater #45

Closed zeleznypa closed 4 years ago

zeleznypa commented 5 years ago

Version: 3.2.*

Bug Description

Wrapping condition ignore the length of the property name and needed characters $ = [] https://github.com/nette/php-generator/blob/master/src/PhpGenerator/Helpers.php#L97

Steps To Reproduce

Actual generator produce code:

    /** @var int[] */
    public $terminalsValues = ['true' => 3, 'false' => 4, 'null' => 5, '{' => 6, '}' => 7, ',' => 8, ':' => 9, '[' => 10, ']' => 11];

Expected Behavior

Generate code:

    /** @var int[] */
    public $terminalsValues = [
        'true' => 3, 
        'false' => 4, 
        'null' => 5, 
        '{' => 6, 
        '}' => 7, 
        ',' => 8, 
        ':' => 9, 
        '[' => 10, 
        ']' => 11
    ];

Possible Solution

Add possibility to modify Helper::WRAP_LENGTH by custom length that will be filled here by str_len($property->getName()) + 6

ttomdewit commented 5 years ago

This would be very neat, indeed. Hopefully thisā€™ll get implemented.

ttomdewit commented 5 years ago

Having changed \Nette\PhpGenerator\Helpers::WRAP_LENGTH to, for example, 10, allowed to me to create the following array:

/**
 * The attributes that should be mutated to dates.
 *
 * @var array $dates
 */
public $dates = [
    self::CREATED_AT,
    self::UPDATED_AT,
];

What normally happens is the following:

/**
 * The attributes that should be mutated to dates.
 *
 * @var array $dates
 */
public $dates = [self::CREATED_AT, self::UPDATED_AT];

I personally prefer the former over the latter and I'd like to be able to configure this setting.

Thanks for your time!

Tom

ttomdewit commented 4 years ago

Thank you!

duxthefux commented 4 years ago

@ttomdewit I currently struggle with the same issue, where I also would prefer the former version where each item of the array is in separate line. How do I get there with the latest version?

I would need to update Dumper::$wrapLength but that's simply not possible afaik.

Do you have any suggestion?

ttomdewit commented 4 years ago

@ttomdewit I currently struggle with the same issue, where I also would prefer the former version where each item of the array is in separate line. How do I get there with the latest version?

I would need to update Dumper::$wrapLength but that's simply not possible afaik.

Do you have any suggestion?

@duxthefux I havenā€™t worked on my project for a very long time so Iā€™m afraid Iā€™m not able to help a ton. As soon as Iā€™m back on it Iā€™ll try and see if this works now.

zeleznypa commented 4 years ago

@duxthefux The former version of the generator also decide to wrap the code or not, but the algorithm counts the length of line without the length of the property name.

This fix just changes the counting mechanism to wrap the code more frequently.

To add the possibility of "wrap always", it will be necessary to add public static variable $wrapAlways into the Dumper class and use it here https://github.com/nette/php-generator/blame/master/src/PhpGenerator/Dumper.php#L115

Will you prepare the feature request and pull request?

duxthefux commented 4 years ago

@zeleznypa just created an MR https://github.com/nette/php-generator/pull/56

zeleznypa commented 4 years ago

@duxthefux Thanks for the contribution, but: 1) It is not up to me. I'm not the owner of the project 2) It is not related to this "closed" issue 3) Code is not consistent with another part of code, where the access to the property is direct (no getter/setter)

But as I said, it is not up to me...