Code formatting patterns (was: [squeak-dev] The Trunk: Compiler-cmm.275.mcz)

On 2013-10-04, at 17:45, Chris Muller [hidden email] wrote:

  

Compare

       [ undeclared at: sym put: nil ]
               on: AttemptToWriteReadOnlyGlobal
               do: [ :noti | noti resume: true ]

to:

       [ undeclared
               at: sym
               put: nil ]
               on: AttemptToWriteReadOnlyGlobal
               do: [ :noti | noti resume: true ]

Isn't the first one much more "rectangular" and easier to read? I find the
indentation in the second one irritating.
        

Yes, I agree with you for that one case.  RB, Indented Control Flow,
etc. do not specify how to handle keywords sent to Blocks.  One
suggestion is to add a blank line, as in:

        [ undeclared
                at: sym
                put: nil ]

                on: AttemptToWriteReadOnlyGlobal
                do: [ :noti | noti resume: true ]

Or, an extra indention:

        [ undeclared
                at: sym
                put: nil ]
                     on: AttemptToWriteReadOnlyGlobal
                     do: [ :noti | noti resume: true ]

The important thing to handle the _general_ case, so the computer can
do at least 97% of the formatting.  If we had a larger keyword
message, your rule would look like this:

          [ mySoundRecorder copyTo: resultBuf from: j to: (j + n - 1)
from: buf startingAt: firstInBuf normalize: nFactor dcOffset: dcOffset
]
                  on: AttemptToWriteReadOnlyGlobal
                  do: [ :noti | noti resume: true ]
    

You're misinterpreting my "rule" ;)

My rule is more like: If it clearly fits on one line then do not spread it over multiple lines. Otherwise, use your best judgement. :)

For your example I would probably end up with something like

	[ mySoundRecorder
                copyTo: resultBuf from: j to: (j + n - 1)
                from: buf startingAt: firstInBuf
                normalize: nFactor dcOffset: dcOffset
	]
		on: AttemptToWriteReadOnlyGlobal
                do: [ :noti | noti resume: true ]

even if it's not "agile". Source code is read way more often than written, so IMHO it is worth spending some time formatting it to make the meaning as obvious as possible. 

That said, I could live with your version below, in particular if it was automatically produced. It just doesn't feel as nice as the hand-formatted one which even takes into account the semantics of keywords to find suitable breaks.

  

which, clearly, is a mess compared to:

          [ mySoundRecorder
                copyTo: resultBuf
                from: j
                to: (j + n - 1)
                from: buf
                startingAt: firstInBuf
                normalize: nFactor
                dcOffset: dcOffset ]
                     on: AttemptToWriteReadOnlyGlobal
                     do: [ :noti | noti resume: true ]

I would LOVE to make pretty print use RB formatting in a way that's
acceptable to the wider community, so we can be developers and not
formatting slaves.
    

- Bert -

  

	[ 	mySoundRecorder
                	copyTo: resultBuf from: j to: (j + n - 1)
	                from: buf startingAt: firstInBuf
        	        normalize: nFactor dcOffset: dcOffset
	]	on: AttemptToWriteReadOnlyGlobal do: 
		[ 	:noti |
			noti someOtherMethod. 
			noti resume: true 
		]

cheers -ben

Bert, I like your vertical alignment of square braces for blocks.  For
myself, this is my only absolute formatting rule, excepting only when
opposing braces fit the same line.  Unfortunately most Smalltalk code puts
the closing brace as soon as possible at the end of the last statement of
the block.  Sometimes three or four closing braces pile up together at the
end of a line, and it is slow to isolate the matching open braces. (Maybe
    

Not if you use Shout.  Each level of parentheses and blocks specifies
its own color.
  

Nor if you simply click in the inner edge of the bracket.  Or Command+Space it.
  

Most Smalltlak code employs Rectangular Block because it's an
object-oriented format.  Like a text-version of Scratch tiles.
  

that indicates other architectural issues with the code, but the reality is
these situations occur.)  As a slight twist of my own to your last example,
I indent the first statement one tabstop.

[ mySoundRecorder
                copyTo: resultBuf from: j to: (j + n - 1)
               from: buf startingAt: firstInBuf
               normalize: nFactor dcOffset: dcOffset
] on: AttemptToWriteReadOnlyGlobal do:
[ :noti |
noti someOtherMethod.
noti resume: true
]
    

The above, by contrast, looks and feels procedural, with the brackets
"delimiting" the starts and ends of code segments.


  

Ah, there's no war like a code formatting war. To throw my twig into
the fire,