Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong wrapping condition in array formater #45

Closed
zeleznypa opened this issue Jun 21, 2019 · 8 comments · Fixed by #47
Closed

Wrong wrapping condition in array formater #45

zeleznypa opened this issue Jun 21, 2019 · 8 comments · Fixed by #47

Comments

@zeleznypa
Copy link
Contributor

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
Copy link

This would be very neat, indeed. Hopefully this’ll get implemented.

@ttomdewit
Copy link

ttomdewit commented Jul 18, 2019

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
Copy link

Thank you!

dg pushed a commit that referenced this issue Nov 20, 2019
dg pushed a commit that referenced this issue Nov 20, 2019
dg pushed a commit that referenced this issue Nov 20, 2019
dg pushed a commit that referenced this issue Nov 20, 2019
dg pushed a commit that referenced this issue Nov 20, 2019
dg pushed a commit that referenced this issue Nov 20, 2019
dg pushed a commit that referenced this issue Nov 20, 2019
dg pushed a commit that referenced this issue Nov 20, 2019
dg pushed a commit that referenced this issue Nov 20, 2019
@duxthefux
Copy link

@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
Copy link

@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
Copy link
Contributor Author

@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
Copy link

@zeleznypa just created an MR #56

@zeleznypa
Copy link
Contributor Author

@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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants