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

Problem with indentation if code is chained #237

Closed
krzysztof-magosa opened this issue Apr 9, 2015 · 9 comments
Closed

Problem with indentation if code is chained #237

krzysztof-magosa opened this issue Apr 9, 2015 · 9 comments

Comments

@krzysztof-magosa
Copy link

Hi,

For unknown reason -> is being aligned with = from previous line.
In PSR2 mode it should be always aligned to 4 spaces.

screen shot 2015-04-09 at 13 57 49

@syohex
Copy link
Collaborator

syohex commented Apr 10, 2015

Would you tell me where is document about this ?
I cannot find it from https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md

@krzysztof-magosa
Copy link
Author

There is point "Code MUST use 4 spaces for indenting, not tabs." in this document.
I think it clearly says that every indentation must be 4 spaces long.

@ejmr
Copy link
Collaborator

ejmr commented Apr 11, 2015

From section 2.4.:

The use of spaces also makes it easy to insert fine-grained sub-indentation for inter-line alignment.

I interpret that to me mean that aligning -> the way we do is acceptable (i.e. "fine-grained sub-indentation") so long as we only use spaces and use at least four spaces.

@metaturso
Copy link

@syohex Assuming that Krzysztof's configuration states somewhere that php-lineup-cascaded-calls is nil, then I believe what he is actually asking is to have all subsequent method calls indented of 4 spaces, instead of matching the assignment operator. E.g.:

// with php-lineup-cascaded-calls nil
$longvarname = $this->get()
    ->another();

// with php-lineup-cascaded-calls t
$bar = $this->get()
            ->another();

This is different from the proposed new behaviour in PR #239. My understanding is that this PR changes the indentation to match ->, regardless the value of emulating php-lineup-cascaded-calls when the indentation style is set to PSR-2. E.g.:

// with php-lineup-cascaded-calls t
$bar = $this->get()
            ->another();

// with php-lineup-cascaded-calls nil
$bar = $this->get()
            ->another();

I don't think that this PR answers Krzysztof request. I think a solution that would keep everybody happy is to allow this behaviour to be customised in the following way:

  1. Indent 1 level (whether this means to indent 4 spaces or 1 tab is down to Emacs to decide, based on your configuration).
  2. Indent to match = above.
  3. Indent to match -> above.
  4. Indent with a function.

Obviously when using option 3. the indentation to the -> above is done regardless of the value of php-lineup-cascaded-calls.

Finally, PSR-2 does not define how cascading method calls should be indented, so as @ejmr suggested the current php-mode behaviour is also acceptable.

@krzysztof-magosa
Copy link
Author

Hm, I've just found that symfony2 style follows PSR2 + chains methods in 4 spaces, so at the moment it resolves my issue. Majority of editors put 4 spaces in such cases, so even if using more spaces is acceptable by psr2 ability to configure as proposed by trashofmasters would be really nice. Many companies add own points to internal coding standards where psr2 lacks information.

@ejmr
Copy link
Collaborator

ejmr commented Apr 13, 2015

Those four varieties of customization are already supported via c-offset-alist, which we use to define the indentation of many things in each style. Instead of making any large changes we could consider carefully explaining how to adjust an existing style, e.g.:

(c-add-style
  "psr2"
  '("php"
    (c-offsets-alist . ((statement-cont . +)))))

And we could explain how to create a new style from scratch, what are the common things worth configuring, et cetera.

Many companies add own points to internal coding standards where psr2 lacks information.

In my opinion this is another reason we should consider expanding our documentation about how to modify/create styles like PSR-2, Symfony, Drupal, and so on.

@metaturso
Copy link

I wasn't aware of the ability to do this, but now I totally agree with
@ejmr. IMHO documenting how this behaviour can be obtained by configuring
the underlying functionality would be the best approach.

On Mon, Apr 13, 2015 at 11:22 AM, Eric James Michael Ritz <
[email protected]> wrote:

Those four varieties of customization are already supported via
c-offset-alist, which we use to define the indentation of many things in
each style. Instead of making any large changes we could consider carefully
explaining how to adjust an existing style, e.g.:

(c-add-style
"psr2"
'("php"
(c-offsets-alist . ((statement-cont . +)))))

And we could explain how to create a new style from scratch, what are the
common things worth configuring, et cetera.

Many companies add own points to internal coding standards where psr2
lacks information.

In my opinion this is another reason we should consider expanding our
documentation about how to modify/create styles like PSR-2, Symfony,
Drupal, and so on.


Reply to this email directly or view it on GitHub
#237 (comment).

@krzysztof-magosa
Copy link
Author

I also didn't know about that.
php-mode is for developers so it's absolutely fine to configure some behaviour by writing few lines of Lisp in config. But of course some note in documentation would be nice, because people without knowledge about underlying layer in emacs won't know how to find it.

@syohex
Copy link
Collaborator

syohex commented Apr 14, 2015

I have merged #239.

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

No branches or pull requests

4 participants