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

Incorrect indentation for multiline logical expression #3167

Open
jdm opened this issue Nov 5, 2018 · 3 comments
Open

Incorrect indentation for multiline logical expression #3167

jdm opened this issue Nov 5, 2018 · 3 comments

Comments

@jdm
Copy link
Contributor

jdm commented Nov 5, 2018

Before:

     // http://tools.ietf.org/html/rfc6265#section-5.1.4
     pub fn path_match(request_path: &str, cookie_path: &str) -> bool {
         // A request-path path-matches a given cookie-path if at least one of
         // the following conditions holds:
         // The cookie-path and the request-path are identical.
         request_path == cookie_path ||

         (request_path.starts_with(cookie_path) && (
             // The cookie-path is a prefix of the request-path, and the last
             // character of the cookie-path is %x2F ("/").
             cookie_path.ends_with("/") ||
             // The cookie-path is a prefix of the request-path, and the first
             // character of the request-path that is not included in the cookie-
             // path is a %x2F ("/") character.
             request_path[cookie_path.len()..].starts_with("/")
         ))
    }

After:

     // http://tools.ietf.org/html/rfc6265#section-5.1.4
     pub fn path_match(request_path: &str, cookie_path: &str) -> bool {
         // A request-path path-matches a given cookie-path if at least one of
         // the following conditions holds:
         // The cookie-path and the request-path are identical.
         request_path == cookie_path ||
            (request_path.starts_with(cookie_path) &&
                (
                    // The cookie-path is a prefix of the request-path, and the last
                    // character of the cookie-path is %x2F ("/").
                    cookie_path.ends_with("/") ||
            // The cookie-path is a prefix of the request-path, and the first
            // character of the request-path that is not included in the cookie-
            // path is a %x2F ("/") character.
            request_path[cookie_path.len()..].starts_with("/")
               ))
    }
@scampi
Copy link
Contributor

scampi commented Nov 5, 2018

It seems like knowledge about the parentheses (opened, closed, count, ...) would be needed to have a nice formatting in this case, which incidentally would also be needed for #3108 as per @nrc on discord.

For example, a formatting like below (mostly wrt closing parentheses) could be ouputted:

     // http://tools.ietf.org/html/rfc6265#section-5.1.4
     pub fn path_match(request_path: &str, cookie_path: &str) -> bool {
         // A request-path path-matches a given cookie-path if at least one of
         // the following conditions holds:
         // The cookie-path and the request-path are identical.
         request_path == cookie_path ||
            (request_path.starts_with(cookie_path) &&
                (
                    // The cookie-path is a prefix of the request-path, and the last
                    // character of the cookie-path is %x2F ("/").
                    cookie_path.ends_with("/") ||
                    // The cookie-path is a prefix of the request-path, and the first
                    // character of the request-path that is not included in the cookie-
                    // path is a %x2F ("/") character.
                    request_path[cookie_path.len()..].starts_with("/")
                )
            )
    }

Maybe that output is possible without making the rewrite-logic aware of parentheses, but I wanted to highlight another use case for it.

@nrc
Copy link
Member

nrc commented Nov 5, 2018

It seems like knowledge about the parentheses (opened, closed, count, ...)

I think that knowledge is implicit in the call stack and the Shape used for layout.

@topecongiro
Copy link
Contributor

This is because of a binary expression with comment in between:

            cookie_path.ends_with("/") ||
            // The cookie-path is a prefix of the request-path, and the first
            // character of the request-path that is not included in the cookie-
            // path is a %x2F ("/") character.
            request_path[cookie_path.len()..].starts_with("/")

rustfmt currently cannot handle this.

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

5 participants