[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Enhancement: Internal ledger lines. (issue1855056)
From: |
hanwenn |
Subject: |
Enhancement: Internal ledger lines. (issue1855056) |
Date: |
Wed, 04 Aug 2010 21:44:58 +0000 |
what's the plan for this patch/functionality ?
the code looks a bit out of style.
would it be possible to group all the internal-ledger code together?
Without wanting to trample on feet, this looks like a feature that could
easily bitrot due to lack of use, so it's good to make it easy to remove
again.
http://codereview.appspot.com/1855056/diff/1/2
File lily/ledger-line-spanner.cc (right):
http://codereview.appspot.com/1855056/diff/1/2#newcode52
lily/ledger-line-spanner.cc:52: Internal_ledgers () : halfspace_ (1)
we usually do
= 1
in the body.
http://codereview.appspot.com/1855056/diff/1/2#newcode63
lily/ledger-line-spanner.cc:63: *
is this a new coding style? - if not follow the rest of the code?
http://codereview.appspot.com/1855056/diff/1/2#newcode87
lily/ledger-line-spanner.cc:87: return ledger_extent_.contains (pos /
halfspace_);
? should be * rather than /
http://codereview.appspot.com/1855056/diff/1/2#newcode146
lily/ledger-line-spanner.cc:146: Real &left_shorten);
IIRC, we use pointers rather than refs for modifying arguments.
http://codereview.appspot.com/1855056/show
- Enhancement: Internal ledger lines. (issue1855056),
hanwenn <=