Should delimiters of different priorities be extra-indented?

[yarinb]

Black takes an interesting decision regarding how to treat strings as dictionary values.

Following dict has a very long string:

 params = {
        "prompt": "consent",
        "response_type": "code",
        "access_type": "offline",
        "client_id": google_client_id,
        "scope": "profile email https://www.googleapis.com/auth/contacts.readonly https://www.googleapis.com/auth/drive.readonly",
        "redirect_uri": google_callback_uri,
        "state": state,
    }

As Black won't split the string, I manually edited it to this:

  params = {
        "prompt": "consent",
        "response_type": "code",
        "access_type": "offline",
        "client_id": google_client_id,
        "scope": "profile "
                  + "email https://www.googleapis.com/auth/contacts.readonly "
                  + "https://www.googleapis.com/auth/drive.readonly",
        "redirect_uri": google_callback_uri,
        "state": state,
    }

But Black than changes to this:

    params = {
        "prompt": "consent",
        "response_type": "code",
        "access_type": "offline",
        "client_id": google_client_id,
        "scope": "profile "
        + "email https://www.googleapis.com/auth/contacts.readonly "
        + "https://www.googleapis.com/auth/drive.readonly",
        "redirect_uri": google_callback_uri,
        "state": state,
    }

I think the indentation here is important for readability, as one might mistake the broken line as dict pairs.

======================================
Operating system: macOS
Python version: 3.6.4
Black version: 18.4a2
Does also happen on master: yes

[ambv]

Workaround until I figure out what to do here: wrap the string in parentheses. Then you don't even have to explicitly concatenate it with +.

[ambv]

We will put optional parentheses in this situation.

[ambv]

I thought about this some more and I'm not sure that parentheses are the right answer here. At the same time, Black currently doesn't have any rule that performs additional indentation based on delimiter priority.

[zooba]

+1 for indenting as in #345

[peterjc]

Just using implicit string concatenation the example becomes:

params = {
    "prompt": "consent",
    "response_type": "code",
    "access_type": "offline",
    "client_id": google_client_id,
    "scope": "profile "
    "email https://www.googleapis.com/auth/contacts.readonly "
    "https://www.googleapis.com/auth/drive.readonly",
    "redirect_uri": google_callback_uri,
    "state": state,
}

I find that is undesirable. Adding more brackets does seem OK:

params = {
    "prompt": "consent",
    "response_type": "code",
    "access_type": "offline",
    "client_id": google_client_id,
    "scope": (
        "profile "
        "email https://www.googleapis.com/auth/contacts.readonly "
        "https://www.googleapis.com/auth/drive.readonly"
    ),
    "redirect_uri": google_callback_uri,
    "state": state,
}

It isn't explicit, but is the suggestion here (and in #345 which was marked as a duplicate) the following extra indentation?:

params = {
    "prompt": "consent",
    "response_type": "code",
    "access_type": "offline",
    "client_id": google_client_id,
    "scope":
        "profile "
        "email https://www.googleapis.com/auth/contacts.readonly "
        "https://www.googleapis.com/auth/drive.readonly",
    "redirect_uri": google_callback_uri,
    "state": state,
}

[ambv]

What we will do is this:

• we won't be changing anything when there are existing delimiters;
• in the special case of implicitly concatenated strings, we will be wrapping with extra parentheses;
• in the special case of a long lambda (see Lambda wrapped without indent #332), we will be wrapping with extra parentheses.

[SwampFalc]

I would like to point out an edge case: when the value in and of itself would fit on a single line, but key and value together go over.

The following code looks fine in my own eyes:

def dataset_configuration():
    return {
        "internal_mount": "/mnt_external",
        "external_mount": "/home/steria/mnt_external_prd",
        "dataexport_directory": "/home/steria/mnt_external_prd/batches/dataexport",
        "dataexport_log":
            "/home/steria/mnt_external_prd/batches/dataexport/logs/dataexport.log",
        "dataexport_script": 
            "/home/steria/mnt_external_prd/batches/dataexport/bin/run.sh",
    }

No line over 88 characters, clear distinction between keys and values.

But black reformats this to this, which is longer than 88 characters:

def dataset_configuration():
    return {
        "internal_mount": "/mnt_external",
        "external_mount": "/home/steria/mnt_external_prd",
        "dataexport_directory": "/home/steria/mnt_external_prd/batches/dataexport",
        "dataexport_log": "/home/steria/mnt_external_prd/batches/dataexport/logs/dataexport.log",
        "dataexport_script": "/home/steria/mnt_external_prd/batches/dataexport/bin/run.sh",
    }

Because apparently putting keys and values on the same line is a lot more important than sticking to the line length...

But if we're adding parentheses anyway in related cases, why not prefer consistence with this new rule and have this as output instead?

def dataset_configuration():
    return {
        "internal_mount": "/mnt_external",
        "external_mount": "/home/steria/mnt_external_prd",
        "dataexport_directory": "/home/steria/mnt_external_prd/batches/dataexport",
        "dataexport_log": (
            "/home/steria/mnt_external_prd/batches/dataexport/logs/dataexport.log"
        ),
        "dataexport_script": (
            "/home/steria/mnt_external_prd/batches/dataexport/bin/run.sh"
        ),
    }

I mean... I would personally still prefer the original and I don't quite understand why black insists on making a change to begin with, but at least this change stays within the line length.

[tolomea]

I'm going to put in a vote for different types of splitting get different indents
or put another way a multiline statement in a list like should have an extra level of indenting
I see the split string version got fixed, but I still hit stuff like this

        return queryset.filter(
            Q(lifecycle_status_sent_at=None) | Q(lifecycle_status_sent_at__lt=self.now - Consumer.STATUS_INTERVAL),
            lifecycle_updated_at__gt=self.now - Consumer.ALERT_HOLD_OFF,
        )

which currently gets reformatted to

        return queryset.filter(
            Q(lifecycle_status_sent_at=None)
            | Q(lifecycle_status_sent_at__lt=self.now - Consumer.STATUS_INTERVAL),
            lifecycle_updated_at__gt=self.now - Consumer.ALERT_HOLD_OFF,
        )

I would prefer

        return queryset.filter(
            (
                Q(lifecycle_status_sent_at=None)
                | Q(lifecycle_status_sent_at__lt=self.now - Consumer.STATUS_INTERVAL)
            ),
            lifecycle_updated_at__gt=self.now - Consumer.ALERT_HOLD_OFF,
        )

which black (22.8.0 with preview) will persist if it's manually done, but not do automatically

[tolomea]

there's a similar issue with inline conditionals where

        data = {
            "topic_type": self.fields["topic_type"].initial,
            "target_type": self.fields["target_type"].initial,
            "override_context": self.fields["override_context"].initial if "override_context" in self.fields else None,
        }

gets reformatted as

        data = {
            "topic_type": self.fields["topic_type"].initial,
            "target_type": self.fields["target_type"].initial,
            "override_context": self.fields["override_context"].initial
            if "override_context" in self.fields
            else None,
        }

[JelleZijlstra]

This was implemented in #3440. On current main:

% python -m black --preview -c '''params = {
        "prompt": "consent",
        "response_type": "code",
        "access_type": "offline",
        "client_id": google_client_id,
        "scope": "profile email https://www.googleapis.com/auth/contacts.readonly https://www.googleapis.com/auth/drive.readonly",
        "redirect_uri": google_callback_uri,
        "state": state,
    }
data = {
            "topic_type": self.fields["topic_type"].initial,
            "target_type": self.fields["target_type"].initial,
            "override_context": self.fields["override_context"].initial if "override_context" in self.fields else None,
        }
'''
params = {
    "prompt": "consent",
    "response_type": "code",
    "access_type": "offline",
    "client_id": google_client_id,
    "scope": (
        "profile email https://www.googleapis.com/auth/contacts.readonly"
        " https://www.googleapis.com/auth/drive.readonly"
    ),
    "redirect_uri": google_callback_uri,
    "state": state,
}
data = {
    "topic_type": self.fields["topic_type"].initial,
    "target_type": self.fields["target_type"].initial,
    "override_context": (
        self.fields["override_context"].initial
        if "override_context" in self.fields
        else None
    ),
}