1338109 - [clang-format] Should lines be cut to fit into 80 columns, it shouldn't continue to fit as much as it can in 80 columns

Status: NEW

(Developer Infrastructure :: Lint and Formatting, defect)

Description

[Jean-Yves Avenard [:jya]]

This is not something currently specified in the mozilla coding style but I believe it would make things much more readable with no confusion possible.

Currently, clang-format attempt to always fill as much as possible within 80 columns.

example it will format the line as:
  return (aDecoder.HasPromise() || aDecoder.mTimeThreshold.isSome()) &&
         !aDecoder.HasPendingDrain() && !aDecoder.HasFatalError() &&
         !aDecoder.mDemuxRequest.Exists() && !aDecoder.mOutput.Length() &&
         !aDecoder.HasInternalSeekPending() &&
         !aDecoder.mDecodeRequest.Exists();

ignore the fact that the && operator is incorrectly placed at the end of the line (see bug 1338105)

To be spec compliant it should be written:
  return (aDecoder.HasPromise() || aDecoder.mTimeThreshold.isSome()) 
         && !aDecoder.HasPendingDrain() && !aDecoder.HasFatalError() 
         && !aDecoder.mDemuxRequest.Exists() && !aDecoder.mOutput.Length() 
         && !aDecoder.HasInternalSeekPending() 
         && !aDecoder.mDecodeRequest.Exists();

But this is difficult to read, because it has placed more than one operation on a line as it fitted within 80 columns.

and it would be more readable written as:
  return
    (aDecoder.HasPromise() || aDecoder.mTimeThreshold.isSome())
    && !aDecoder.HasPendingDrain()
    && !aDecoder.HasFatalError()
    && !aDecoder.mDemuxRequest.Exists()
    && !aDecoder.mOutput.Length()
    && !aDecoder.HasInternalSeekPending()
    && !aDecoder.mDecodeRequest.Exists();

The first:
(aDecoder.HasPromise() || aDecoder.mTimeThreshold.isSome()) can be written on a single line, because it is one logical statement, not involving the other operators and has higher priority.

same with:
  if ((decoder.mWaitingForData
       && (!decoder.mTimeThreshold || decoder.mTimeThreshold.ref().mWaiting))
      || (decoder.mWaitingForKey && decoder.mDecodeRequest.Exists())) {
  }

the above is correct and preferred.