boolean operators should be at the start of a line

[autonomousapps]

I don't know if this is a feature request or a bug. ktfmt prefers complex conditionals to look like this:

Current behavior

.artifactView { view ->
  view.componentFilter { id ->
    id is ProjectComponentIdentifier &&
      id.projectPath !in excludedLocalPaths &&
      (includedLocalPaths.isEmpty() || id.projectPath in includedLocalPaths)
  }
}

or if I add comments

.artifactView { view ->
  view.componentFilter { id ->
    // Must be a local project.
    id is ProjectComponentIdentifier
    // must not be in the set of exclusions.
    &&
      id.projectPath !in excludedLocalPaths
      // If the set of inclusions is not empty, must be in that set.
      &&
      (includedLocalPaths.isEmpty() || id.projectPath in includedLocalPaths)
  }
}

Expected (desired) behavior:

.artifactView { view ->
  view.componentFilter { id ->
    id is ProjectComponentIdentifier
      && id.projectPath !in excludedLocalPaths
      && (includedLocalPaths.isEmpty() || id.projectPath in includedLocalPaths)
  }
}

I think the expected behavior is better because it is much more clear when reading that each additional line is "anded" or "or'd", and in practical terms, in the IDE, I can easily comment out a whole line (condition) with a single keystroke. With the current behavior, when the operator is dangling at the end of the preceding line, it is not as easy to just comment out one of the conditions.

Happy to discuss or contribute if maintainers agree.

[hick209]

I believe opening a discussion around this is a good idea as I don't believe this stands as an issue, but a matter of preference.

One could just as easily say that it's easier to comment out the first line in the current approach without breaking anything, when your suggestion would require some extra handling. Meaning that both have their tradeoffs and that's why I believe it's something worth discussing.

[autonomousapps]

Thanks for the reply! I agree this could be considered more a matter of preference. I think I was making an implicit assumption (which I'm now realizing) that I was treating the first part of the conditional as "more important", and the subsequent portions as the ones I'm more likely to comment out on the fly to test things. That does seem more like a personal style choice, and in the absence of large amounts of data I don't know if I'm in the minority or the majority.

Continuing to speculate, I feel like having the operator at the start of the line vs the end makes it easier to read, since the column (line-start) will be the same for each subsequent line. When at the end, the column (line-end) for the location of each operator will be variable.

Kind of tangential, ktfmt clearly has a preference for inserting a line break between the operator and the operand:

    // must not be in the set of exclusions.
    &&
      id.projectPath !in excludedLocalPaths

which feels strange to me. I like inserting comments for each line of a conditional, but doing so in the presence of this tool is mangling the source code a bit.

[nreid260]

There's a technical limitation that requires binary operators to be on the same line as the left operand, at least some of the time. And that makes it easier/preferable for it to happen all the time.

Unary operators can be ambiguous:

val x = y + z

parses differently than

val x = y
  + z

The latter is two statements: a trivial assignment, followed but a unary-plus call on z.

[autonomousapps]

I think that makes sense and I can understand why, from an implementation perspective, you might prefer to leave things as they are.

Is there a world where we can gather feedback and see if there might be a preponderance of support for my proposal? Because again, while I agree with what you're saying, we can at least say that the boolean operators (other than !) are never unary and so could be treated separately from operators that are unary or binary.

[nreid260]

I'm not an owner (although I have made substantial contributions) so I don't actually have the power to enforce an outcome.

I don't personally see an aesthetic benefit to the proposal, not that the current approach is prettier, rather that they seem equal to me. Yes, it's gross with the comments, but the comments are adversarially positioned. Given that, I'm in favour of the status quo, since that incurs no additional complex, unpredictability, or churn.

IME, the best way to get something this size done is to do it yourself. Nothing argues louder than a working prototype. And you'll learn a lot about the tradeoffs getting it working.

[autonomousapps]

IME, the best way to get something this size done is to do it yourself. Nothing argues louder than a working prototype. And you'll learn a lot about the tradeoffs getting it working.

I'm certainly open to that, and I even mentioned that in my original comment. But I wasn't about to put in such work without first seeing if it had the possibility to be merged.

[nreid260]

Sorry, didn't mean to imply anything negative. I should say, "the fastest way".

[autonomousapps]

This is another example of why I think my preference is "objectively better":

val foo = !a && 
          (b || (c && !d) || e || (f && !g))

It's a complex boolean. Could be refactored I suppose, but as-is it's impossible to easily comment out either of the lines with a single keystroke (like cmd-/ in Intellij).

When I get a chance I may try to propose a code change that would change the behavior, for consideration by repo maintainers.

[ZacSweers]

I don't think this can be a preference thing. Per correctness issue described here, it has to be trailing. ktfmt would need to special-case some operators but not others, vs just standardize on one pattern for all operators.

[autonomousapps]

ktfmt would need to special-case some operators but not others

That's exactly what I'm proposing, yes. I haven't had any time to invest in this though. I do keep running into cases where I wish my desired behavior was the actual behavior, so if enough of those accumulate over a sufficiently short span of time, perhaps I'll find the necessary motivation.