item.cc: make is_non_musical() conform to its name

This function was renamed from is_breakable() in commit   e03b2923c10e1c but still returned false for line-broken non musical Items.

So far, only one use the function seems to have suffered from the poor name, that in hairpin.cc leading to the problem in issue 2060 and 2057.

[pkx166h, 2011-11-29 08:23:42]

passes make and make check

james

[Keith, 2011-11-29 08:23:42]

LGTM  (I have to say that, even though I'm the author, or my comment will raise
the "patch-needs-work" flag.)

http://codereview.appspot.com/5373048/diff/3018/lily/hairpin.cc
File lily/hairpin.cc (right):

http://codereview.appspot.com/5373048/diff/3018/lily/hairpin.cc#newcode178
lily/hairpin.cc:178: if (bound->is_non_musical (bound)
is_non_musical() explicitly excludes Items that were already broken.  This made
made more sens when it was originally named is_breakable().  The name change
happened in a big commit e03b2923c10e1c that I cannot unravel.

Here, we are looking for the case of hairpins bounded by a breakable, or broken,
column.  So, include both cases.

[Keith, 2011-11-30 07:38:25]

New, completely different patch

[MikeSol, 2011-11-30 08:49:50]

LGTM.

I like the fact that this brings the function name in line with the function's
function - it'll make it easier on future users of the function to understand
what's going on.

I won't have time to test it today, but I'm assuming that the change doesn't
have adverse effects in other places where non_musical is used.

Cheers,
MS

[Keith, 2011-11-30 20:10:05]

On 2011/11/30 08:49:50, MikeSol wrote:

> I won't have time to test it today, but I'm assuming that 
> the change doesn't
> have adverse effects in other places where non_musical is used.

Yes, I tested all the uses of the changed function.
Except for the one case where I added a test for original() in this patch, all
are okay with the function meaning "non-musical" as opposed to
"breakable-and-not-a-clone"

lily/break-align-engraver.cc:76:  
 if (!Item::is_non_musical (inf.item ())) // from Fix #1695, wants non-musical
lily/break-align-engraver.cc:100:
 if (!Item::is_non_musical item)) // Fine either way, as the distinction is not
yet made; tested with assert( 0 == original() )
lily/paper-column-engraver.cc:237:
  Grob *col = Item::is_non_musical (elem) ? command_column_ : musical_column_;//
conceptually "non-musical"
lily/script-column-engraver.cc:68:
lily/script-row-engraver.cc:74:
  if (!Item::is_non_musical (thing)) // conceptually "non-musical"
lily/separating-line-group-engraver.cc:87:
 if (Item::is_non_musical (it) // This used to be "ib", standing for
is-breakable, but it should not matter.  tested with tested with assert( 0 ==
original() )
lily/system.cc:144:
 if (it && Item::is_non_musical (it)) // could go either way; tested with
assert( 0 == original() )

[pkx166h, 2011-12-01 17:30:19]

Passes Make and I get no reg test diffs.

James