LilyIssues / Issues / #1193 Enhancement: internal ledger lines

Google Importer - 2010-08-01

Originally posted by: n.putt...@gmail.com

(No comment was entered for this change.)

Summary: [PATCH] Enhancement: internal ledgers

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2010-08-01

Originally posted by: n.putt...@gmail.com

(No comment was entered for this change.)

Summary: [PATCH] Enhancement: internal ledger lines

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2010-08-04

Originally posted by: n.putt...@gmail.com

Cleaned up patch: http://codereview.appspot.com/1855056/show

I've removed the code associated with chromatic staves, since Kevin Dalley produced a separate patch for that (which was committed: it introduced the context property `staffLineLayoutFunction').

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2010-08-04

Originally posted by: n.putt...@gmail.com

(No comment was entered for this change.)

internal-legers.png

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2010-08-06

Originally posted by: percival.music.ca@gmail.com

(No comment was entered for this change.)

Labels: Priority-Medium

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2010-10-16

Originally posted by: v.villenave

Where are we wrt this patch? Is it only a matter of coding style before it gets accepted, or does it lack testing/documentation as well?

Neil, do you think that I could at least mark this issue as "Started"?

Status: Accepted

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2010-10-24

Originally posted by: n.putt...@gmail.com

> Where are we wrt this patch? Is it only a matter of coding style before it gets accepted, or does it lack testing/documentation as well?

It needs some refactoring (I only tidied it up to the extent that it compiled in current master) and documentation.

> Neil, do you think that I could at least mark this issue as "Started"?

Nope. That would require an owner.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-01-22

Originally posted by: percival.music.ca@gmail.com

There appears to be no action on this patch, so I'm moving it into the "abandoned" category.

Labels: -Patch Abandoned_patch

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-01-29

Originally posted by: percival.music.ca@gmail.com

(No comment was entered for this change.)

Labels: -Abandoned_patch Patch-abandoned

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-05

Originally posted by: pkx1...@gmail.com

Neil is this patch really abandoned? The rietveld is still open so we'd like to just tie up the loose ends.

http://codereview.appspot.com/1855056/

James

Owner: n.putt...@gmail.com

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-05

Originally posted by: n.putt...@gmail.com

I'm not working on this patch, it was simply a case of ensuring it's visible, since the original version was completely ignored. At least if anbody wants to work on it, we have a record of it (even if it doesn't currently apply to master).

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-10

Originally posted by: pkx1...@gmail.com

According to this

http://codereview.appspot.com/4974075/

this addresses this. This passes make and reg tests

Summary: Enhancement: internal ledger lines
Owner: ---
Labels: -Patch-abandoned -Priority-Medium Patch-review

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-10

Originally posted by: ColinPKC...@gmail.com

@12:
James, is this this this this or this one?

@11:
Neil, should http://codereview.appspot.com/1855056/ be closed, then?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-10

Originally posted by: n.putt...@gmail.com

> Neil, should http://codereview.appspot.com/1855056/ be closed, then?

I guess so.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-10

Originally posted by: pkx1...@gmail.com

Colin,

http://codereview.appspot.com/4974075/ is for this issue (as well as issue 1292)

James

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-10

Originally posted by: ColinPKC...@gmail.com

@Neil and Piers: Can we merge issues 1193 (Internal Leger Lines) and 1292 (Twelve-Notation Support)? It seems to me that 1292 is a special case of which 1193 is the general.

Cheers,
Colin

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-14

Originally posted by: ColinPKC...@gmail.com

Changes requested; see Rietveld.

Labels: -Patch-review Patch-needs_work

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-17

Originally posted by: pkx1...@gmail.com

Passes make and reg tests

(making Janek owner as Piers is not available as an 'Owner')

Labels: -Patch-needs_work Patch-review
Owner: janek.li...@gmail.com

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-20

Originally posted by: ColinPKC...@gmail.com

(No comment was entered for this change.)

Labels: -Patch-review Patch-countdown

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-21

Originally posted by: ColinPKC...@gmail.com

(No comment was entered for this change.)

Labels: -Patch-countdown Patch-push

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-23

Originally posted by: piersti...@gmail.com

@Colin: yes, I guess they can be merged, or could have been. I guess now it's just a matter of pushing the patch and closing them all.
Also https://code.google.com/p/lilypond/issues/detail?id=1878 is for the same patch, but even more general.

Cheers,
Piers

(sorry for the slow response, I wasn't notified on this issue)

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-23

Originally posted by: pkx1...@gmail.com

I think we need some documentation for the Notation Reference or at least an entry into the 'changes' doc.

If someone wants to write 'something' up for the Notation Reference I can make a patch and an example.

We could probably use the example in comment 4 right?

James

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-23

Originally posted by: piersti...@gmail.com

I added a regression test, which is a good example for custom external ledger lines. It should probably also be added to the documentation. See staff-ledger-positions.ly

As for internal ledger lines, they are now shown by default, I attached an example that shows this, and explains how to omit them. See staff-ledger-internal.ly

The behaviour of the example in comment 4 is a bit strange in my opinion, but can be accomplished with \override Staff.StaffSymbol #'ledger-positions = #'(-12 -10 -8 -6 -4 -2 (0 2) 4 6 8 10 12). The brackets around (0 2) link them together, without the brackets it would be the same as the default behaviour.

Changes:
Support for custom ledger positions added.
Internal ledger lines are now shown when custom line positions are set with a gap of 4 steps or more.

Piers

staff-ledger-internal.ly

staff-ledger-internal.preview.png

staff-ledger-positions.ly

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-25

Originally posted by: janek.li...@gmail.com

Hi,

i see that i'm the owner of this issue (and i suppose that Piers doesn't have git push ability), so i guess that it's for me to push. However, i don't understand some things:
- what exactly shall i push?
- is Rietveld issue up-to-date? I see that http://codereview.appspot.com/4974075/ was last modified week ago, while the discussion here is more recent (for example i see staff-ledger-internal.ly attached to the previous comment, but it's not in Rietveld)
- if the patch was indeed modified after the countdown, should it have another countdown before pushing?

cheers,
Janek

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-09-25

Originally posted by: pkx1...@gmail.com

Well Janek, the Rietvld issue was what I tested. That doesn't mean that *is* the correct code, but I didn't see anything newer at the time.

If you do end up pushing then we probably need a new tracker for the NR doc for this (and an entry in changes.tely as well). I've offered to do this but didn't want to mess this tracker up for Doc (that can come later).

James

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Enhancement: internal ledger lines

Issue Tracker for LilyPond

Searches

Help

#1193 Enhancement: internal ledger lines

Discussion