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

Align arrow in arrays ? #118

Closed
mathieutu opened this issue Feb 23, 2018 · 21 comments
Closed

Align arrow in arrays ? #118

mathieutu opened this issue Feb 23, 2018 · 21 comments

Comments

@mathieutu
Copy link

I took the habit of aligning all the arrows in a array.
Phpstorm do that automatically, and I find that a lot clearer.

-        'description'  => 'description',
-        'excerpt'      => 'extrait',
-        'date'         => 'date',
-        'time'         => 'heure',
-        'available'    => 'disponible',
-        'size'         => 'taille',
-        'already_paid' => 'déjà payé',
-    ],
+        'description' => 'description',
+        'excerpt' => 'extrait',
+        'date' => 'date',
+        'time' => 'heure',
+        'available' => 'disponible',
+        'size' => 'taille',
+        'already_paid' => 'déjà payé'
+   ]

What do you think about that?

@alexander-akait
Copy link
Member

alexander-akait commented Feb 23, 2018

@mathieutu by default no align, we should keep open issue and get community opinion

  • 👍 Add option
  • 👎 Don't add option

@nicoder
Copy link
Contributor

nicoder commented Feb 23, 2018

in my team we agreed not to align, one advantage is that there is less noise in git diffs.

@alexander-akait
Copy link
Member

@nicoder thanks for response, feel free to write you opinion, just don't forget vote

@mathieutu
Copy link
Author

Adding an option don't force you to use it! 😉

@nicoder
Copy link
Contributor

nicoder commented Feb 23, 2018

sure, I did not see you mentioning an option at the beginning, then @evilebottnawi chimed in :)

@czosel
Copy link
Collaborator

czosel commented Feb 23, 2018

Please also see #120 😄

@czosel
Copy link
Collaborator

czosel commented Feb 25, 2018

I'd suggest we follow the advice here #120 (comment) and not implement alignment.

@czosel
Copy link
Collaborator

czosel commented Feb 26, 2018

Closing since there seems to be consensus 😄

@czosel czosel closed this as completed Feb 26, 2018
@alexander-akait
Copy link
Member

alexander-akait commented Feb 26, 2018

@czosel very early to close, WordPress is very big community, without this option they can't adapt prettier

@alexander-akait
Copy link
Member

alexander-akait commented Feb 26, 2018

php != js 😄

@czosel
Copy link
Collaborator

czosel commented Feb 26, 2018

I don't think that this should be a blocker for any given codebase. But we can leave the issue open if you like!

@alexander-akait
Copy link
Member

@czosel let's leave open 👍

@mathieutu
Copy link
Author

I think I'll still use prettier without that, but I will stay very sad.. 😢

😇

@alexander-akait
Copy link
Member

@mathieutu After stable release we will consider an option, you can help us by applying the prettier on your code base and create issue about problems 👍

@mathieutu
Copy link
Author

mathieutu commented Feb 26, 2018

@evilebottnawi Yeah I'm actually doing it, and I've already opened several issues.

@cmancone
Copy link

cmancone commented Mar 30, 2018

As someone who has always aligned my arrows and everything else as well, I'd vote no on this one, and probably even no on providing an option for it. I love my aligned arrows but the problem is that I've found that even how to align things causes disagreement among developers. As a relevant example, we have a large number of definitions of multi-dimensional arrays. I understand this is not the norm, but I'm sure we're not the only ones who run into this. How do you line up something like this (actual code snippet from our code base)?

        return [
            'status_id'     => [
                'class'         => 'status',
                'parent_class'  => status::class,
                'validators' => [ 'required' ],
            ],
            'created_by_id' => [
                'class'         => 'created_by_membership',
            ],
            'customer_id'   => [
                'class'         => 'belongs_to',
                'parent_class'  => customer::class,
                'multi_tenant'  => true,
            ],
            'job_id'        => [
                'class'         => 'belongs_to',
                'parent_class'  => job::class,
                'multi_tenant'  => true,
            ],
            'total'         => [
                'class'         => 'price',
                'editable'      => false,
            ],
            'total_man_minutes' => [
                'class'         => 'integer',
            ],
            'fully_quoted'  => [
                'class'         => 'yes_no',
                'editable'      => false,
            ],
            'vendor_approved' => [
                'class'         => 'yes_no',
                'editable'      => false,
            ],
            'created'       => [
                'class'         => 'created',
            ],
            'updated'       => [
                'class'         => 'updated',
            ],
            'tasks'         => [
                'class'         => 'has_many_writeable',
                'child_class'   => estimate_task::class,
            ],
        ]

For which I see two problems: the "business" question of "What does it mean to line up nested data structures?" and then the code to actually do that. I'm sure it isn't an impossible problem, but especially at this phase of the process I don't think it should be a high priority. It wouldn't surprise me if you ended up with a couple options (even prettier has one or two), and this might be a good one to choose, but it is definitely a less is more sort of situation, and I'm okay with not having any options at all at the beginning.

@mathieutu
Copy link
Author

I'm closing this, as you all convinced me 😋...

And some reading to take away about that: http://mnapoli.fr/approaching-coding-style-rationally/

@mathieutu
Copy link
Author

or we can leave it open for later and other users, if you want @evilebottnawi..

@alexander-akait
Copy link
Member

@mathieutu let's left open

@czosel
Copy link
Collaborator

czosel commented Dec 1, 2018

Closing due to inactivity.

@czosel czosel closed this as completed Dec 1, 2018
@nikita-komissarov
Copy link

This is the only reason why I use phpfmt and not Prettier. Really looking forward to having this feature in Prettier!

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

No branches or pull requests

6 participants