There are no documented standards for how to handle when an object method is wrapped onto a second line.

e.g.:

  $query = db_select('node');
  $query->condition('type', 'article')
    ->condition('status', 1)
    ->execute();

vs

  $query = db_select('node');
  $query->condition('type', 'article')
      ->condition('status', 1)
      ->execute();

vs

  $query = db_select('node');
  $query->condition('type', 'article')
        ->condition('status', 1)
        ->execute();

The first example appears to be the convention, but it is not standardized.

Once an agreement is reached we'll need to update the Coder rules (#2690325: Indentation incorrect on wrapped method calls). Done, as per comment #20.

Suggested text:

Chained Function Calls

In case you have a fluid interface for a class, and the code spans more than one line, the method calls should be indented with 2 spaces:

$query = db_select('node')
  ->condition('type', 'article')
  ->condition('status', 1)
  ->execute();

Comments

DamienMcKenna created an issue. See original summary.

Perignon’s picture

+1. I have always used the first example of two spaces.

klausi’s picture

Some people rather like

  $query = db_select('node');
  $query->condition('type', 'article')
        ->condition('status', 1)
        ->execute();

I'm not sure we should make up rules for this and just let people use whatever they want?

Although Drupal core mostly uses 2 spaces indentation I believe, so it seems to be sort of a convention already.

DamienMcKenna’s picture

Title: Standardize indenting on stacked method calls » [policy, no patch] Standardize indenting on stacked method calls
Issue summary: View changes
Status: Active » Needs review

@klausi: The same could be said of *all* Drupal's coding standards ;-)

So would anyone else agree to making the first example the standard?

jhodgdon’s picture

+1 for 2 spaces, as it's consistent with our other indentation standards.

drunken monkey’s picture

+1 for 2 spaces. It's also what is used in the DB layer documentation – probably the foremost example of such chained calls.

DamienMcKenna’s picture

Title: [policy, no patch] Standardize indenting on stacked method calls » [policy, no patch] Standardize indenting on chained method calls
dawehner’s picture

Issue summary: View changes

I agree with the 2 spaces idea, its simply consistent.

Here is a suggested text:

# Nested Function Calls

In case you have a fluid interface for a class, the method calls should be indented with 2 spaces:

$query = db_select('node');
  ->condition('type', 'article')
  ->condition('status', 1)
  ->execute();
NerOcrO’s picture

+1 for 2 spaces only!

tizzo’s picture

Issue tags: +final discussion
alexpott’s picture

+1 for 2 spaces.

Crell’s picture

"Me too!" (2 space indent.)

The 3rd variant has some logic to it, but when the first line is not a single variable the alignment point could get very far intended. 2-spaces is nice and consistent and readable.

tizzo’s picture

Project: Coding Standards » Drupal core
Version: » 8.2.x-dev
Component: Coding Standards » other

The TWG has reviewed and is ready to approve this issue. Passing to the core queue for core committer sign-off. If this looks good to core please tag it "Core Committer Approved" and pass back to the TWG queue for documentation updates as per the documented process.

klausi’s picture

Status: Needs review » Reviewed & tested by the community

Setting to RTBC to make it visible to core committers.

xjm’s picture

Status: Reviewed & tested by the community » Needs review

This rule could be tested programmatically; is there coverage for it in coder?

klausi’s picture

No, there is no coverage in Coder yet. Please open an issue in the Coder queue.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

klausi’s picture

Ah sorry, of course the Coder issue exists: #2690325: Indentation incorrect on wrapped method calls

xjm’s picture

Status: Needs review » Postponed

Thanks @klausi! Postponing on that issue.

klausi’s picture

Status: Postponed » Needs review
Issue tags: -final discussion +Needs issue summary update

The Coder rule is implemented now.

The next step is to come up with a wording of this rule in the issue summary and where in the coding standards document we are going to put it.

I think no core committer has signed off explicitly yet, although xjm probably did that by asking for the Coder rule? @xjm: can you confirm and add the "Core Committer Approved" tag?

pfrenssen’s picture

@dawehner has a proposal for the wording in #8.

dawehner’s picture

Issue summary: View changes
pfrenssen’s picture

Issue summary: View changes
pfrenssen’s picture

Status: Needs review » Reviewed & tested by the community

Marking this RTBC for core committer approval. We can still hash out the details of the wording while awaiting approval.

catch’s picture

Project: Drupal core » Drupal Technical Working Group
Version: 8.3.x-dev »
Component: other » Code
Issue tags: +Core Committer Approved

This was +1'd by alexpott, xjm's points have been done, I'm also +1, so tagging and sending back to TWG.

I'd use 'chained method calls' rather than 'nested function calls' in the docs. Nested function calls I think of as array_keys(array_intersect_key()).

klausi’s picture

Project: Drupal Technical Working Group » Coding Standards
Component: Code » Coding Standards

Coding Standards is the correct queue now.

jthorson’s picture

Issue summary: View changes

Updating issue summary to reflect suggestion to use 'chained' instead of 'nested', and clarifying that this applies specifically in the multi-line case.

jthorson’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs issue summary update, -Core Committer Approved +Approved Coding Standard

Ratified.

Documentation added to the 'Chaining' section on the Object Oriented code page of the coding standards.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

tim.plunkett’s picture

Except it's "fluent", not "fluid".
Should I just go change the docs page directly?

klausi’s picture

Sure, typo fixes are always allowed :)