could you split this into 2 (or more) separate patches?
- some of your comments are good, and could have been pushed a month ago. - the macro changes are highly problematic and probably won't be accepted for at least a few months. I never like seeing good changes postponed due to them being squashed together with questionable ones. http://codereview.appspot.com/5975054/diff/1/lily/accidental-placement.cc File lily/accidental-placement.cc (right): http://codereview.appspot.com/5975054/diff/1/lily/accidental-placement.cc#newcode116 lily/accidental-placement.cc:116: //TODO: add comment to this struct this still doesn't add any useful information. I mean, of course it would be nice to explain stuff in the code. But there's no point going around adding // TODO: comment this to every single struct/class/method/function. http://codereview.appspot.com/5975054/diff/1/lily/note-collision.cc File lily/note-collision.cc (right): http://codereview.appspot.com/5975054/diff/1/lily/note-collision.cc#newcode404 lily/note-collision.cc:404: for_UP_and_DOWN (d) On 2012/03/31 21:04:35, Keith wrote:
If you write the 'for' loop directly in the code, then neither humans
nor
auto-indent programs need to learn a macro: for (Direction d = UP; d; d = (UP == d)? DOWN : CENTER)
Ouch. I don't find that for loop to be particularly easy to understand; it would be much nicer if there was a macro for this. I wonder if we could ask astyle to add a --custom-loop-commands="for_UP_and_DOWN" option? it would take a while for that to reach an official release, but at least there would be some hope of simplifying this. I agree that we shouldn't switch to a macro if that ruins the indentation. http://codereview.appspot.com/5975054/diff/1/lily/note-collision.cc#newcode577 lily/note-collision.cc:577: for_UP_and_DOWN (d) // please, make a comment to this loop (better than the above one...) adding a comment to say "please comment this" does not help http://codereview.appspot.com/5975054/ _______________________________________________ lilypond-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/lilypond-devel
