[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Issue 4015: Add \magnifyStaff. (issue 117830043 by address@hidden)
|
From: |
david . nalesnik |
|
Subject: |
Re: Issue 4015: Add \magnifyStaff. (issue 117830043 by address@hidden) |
|
Date: |
Thu, 17 Jul 2014 12:44:20 +0000 |
Looks well coded and well commented. I have some observations/questions
about the regtests, but otherwise LGTM.
https://codereview.appspot.com/117830043/diff/60001/input/regression/magnifyStaff-bar-lines.ly
File input/regression/magnifyStaff-bar-lines.ly (right):
https://codereview.appspot.com/117830043/diff/60001/input/regression/magnifyStaff-bar-lines.ly#newcode30
input/regression/magnifyStaff-bar-lines.ly:30:
Are so many examples necessary?
https://codereview.appspot.com/117830043/diff/60001/input/regression/magnifyStaff-dots-beamlets.ly
File input/regression/magnifyStaff-dots-beamlets.ly (right):
https://codereview.appspot.com/117830043/diff/60001/input/regression/magnifyStaff-dots-beamlets.ly#newcode1
input/regression/magnifyStaff-dots-beamlets.ly:1: \version "2.19.11"
This looks to do what it says (and is very appealing visually), but I
notice that the note-spacing becomes more compressed as the
magnification increases. Is this OK?
https://codereview.appspot.com/117830043/diff/60001/input/regression/magnifyStaff-space-alist.ly
File input/regression/magnifyStaff-space-alist.ly (right):
https://codereview.appspot.com/117830043/diff/60001/input/regression/magnifyStaff-space-alist.ly#newcode2
input/regression/magnifyStaff-space-alist.ly:2:
Looks fine. I get a programming error: "No spacing entry from
KeyCancellation to `custos'" Not sure if this is something on my end.
https://codereview.appspot.com/117830043/diff/60001/input/regression/magnifyStaff-space-alist.ly#newcode35
input/regression/magnifyStaff-space-alist.ly:35:
Impressive display, but should there be so many examples?
https://codereview.appspot.com/117830043/