Re: Add Changes entries for \temporary, \omit, \hide, \single, multiple tags (issue 8187044)
On 2013/03/30 19:38:09, dak wrote:
Address Thomas' comments
The description doesn't explain why
\relative c'' { 1 }
returns a programming error.
Though, this is definitely not a topic of \single, \omit or "Changes" in
general.
LGTM
https://codereview.appspot.com/8187044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add Changes entries for \temporary, \omit, \hide, \single, multiple tags (issue 8187044)
Reviewers: janek, thomasmorley65,
Message:
I'll address the other points presently, or on Tuesday, depending on my
connectivity.
https://codereview.appspot.com/8187044/diff/1/Documentation/changes.tely
File Documentation/changes.tely (right):
https://codereview.appspot.com/8187044/diff/1/Documentation/changes.tely#newcode78
Documentation/changes.tely:78: respectively. They can be given a music
expression to tweak, or
On 2013/03/30 19:25:04, thomasmorley65 wrote:
From description and example a user might expect that the following
snippets are
working:
\relative c'' { 1 }
\relative c'' { 1 }
\relative c'' { 1 }
Perhaps inserting a link to LM or NR or wherever they are explained,
would be
sufficient.
I think changes entries should be self-explanatory.
Description:
Add Changes entries for \temporary, \omit, \hide, \single, multiple tags
Please review this at https://codereview.appspot.com/8187044/
Affected files:
M Documentation/changes.tely
Index: Documentation/changes.tely
diff --git a/Documentation/changes.tely b/Documentation/changes.tely
index
2b1a060f41c1ca95794c62ef58bccd5212ae8825..95f73723c91d1a1fff6fbeeac22a45245a03c8e8
100644
--- a/Documentation/changes.tely
+++ b/Documentation/changes.tely
@@ -62,6 +62,56 @@ which scares away people.
@end ignore
@item
+A new command @code{\single} can be used for converting a property
+override into a tweak to be applied on a single music expression:
+
+@lilypond[quote,verbatim,relative=2]
+1
+@end lilypond
+
+@item
+Two ways of letting graphical objects not appear in the PDF are
+overriding its @code{transparent} property with @code{#t}
+(retaining the original spacing) or overriding its @code{stencil}
+property with @code{#f} (not using any space at all). Those two
+operations now have the shorthands @code{\hide} and @code{\omit},
+respectively. They can be given a music expression to tweak, or
+the name of a graphical object for which an override should be
+created:
+
+@lilypond[quote,verbatim]
+\new Staff \with { \omit Clef }
+\relative c'' 1
+@end lilypond
+
+@item
+A new command @code{\temporary} can be applied to overrides in
+order to not have them replace previous property settings. If a
+@code{\revert} is applied to the same property subsequently, the
+previous setting reappears:
+
+@lilypond[quote,verbatim,relative=2]
+\override NoteHead.color #red c4
+\override NoteHead.color #green d
+\revert NoteHead.color e2
+\override NoteHead.color #red c4
+\temporary\override NoteHead.color #green d
+\revert NoteHead.color e
+\revert NoteHead.color c
+@end lilypond
+
+This is mainly useful for writing music functions that need to
+have some property changed just for the duration of the function.
+
+@item
+@code{\tag}, @code{\removeWithTag}, and @code{\keepWithTag} can
+now accept a list of symbols rather than just a single symbol for
+marking, removing, and keeping music with any of multiple tags.
+This is particularly important for @code{\keepWithTag} since one
+cannot achieve the same effect by using multiple consecutive
+@code{\keepWithTag} commands.
+
+@item
The @samp{-d old-relative} option has been removed. Not actually
accessible from the command line any more, its remaining use was
for interpretating @code{\relative} in LilyPond files converted
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add Changes entries for \temporary, \omit, \hide, \single, multiple tags (issue 8187044)
Some nitpicks.
Otherwise LGTM
https://codereview.appspot.com/8187044/diff/1/Documentation/changes.tely
File Documentation/changes.tely (right):
https://codereview.appspot.com/8187044/diff/1/Documentation/changes.tely#newcode73
Documentation/changes.tely:73: Two ways of letting graphical objects not
appear in the PDF are
What about the others: png, eps, svg?
https://codereview.appspot.com/8187044/diff/1/Documentation/changes.tely#newcode78
Documentation/changes.tely:78: respectively. They can be given a music
expression to tweak, or
From description and example a user might expect that the following
snippets are working:
\relative c'' { 1 }
\relative c'' { 1 }
\relative c'' { 1 }
Perhaps inserting a link to LM or NR or wherever they are explained,
would be sufficient.
https://codereview.appspot.com/8187044/diff/1/Documentation/changes.tely#newcode94
Documentation/changes.tely:94: \override NoteHead.color #red c4
Seems you missed some "="
https://codereview.appspot.com/8187044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: rewrite Self_alignment_interface (issue 7768043)
On 2013/03/30 18:15:11, MikeSol wrote: In general, I see a lot of reassigning of parents. What is the goal with this (sorry if you've explained this already)? What do you mean by "reassigning of parents"? I fetch grobs' parents often, but i don't give grobs new parents. https://codereview.appspot.com/7768043/diff/38001/lily/fingering-engraver.cc#newcode142 lily/fingering-engraver.cc:142: Self_alignment_interface::aligned_on_x_parent (fingerings_[i]->self_scm ())); This needs to be a chained offset procedure. See chain_offset_callback elsewhere in the code. ok, will investigate. https://codereview.appspot.com/7768043/diff/38001/lily/paper-column.cc#newcode222 lily/paper-column.cc:222: /* Good work! Thanks :) Janek https://codereview.appspot.com/7768043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Add Changes entries for \temporary, \omit, \hide, \single, multiple tags (issue 8187044)
LGTM https://codereview.appspot.com/8187044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: reorganize self_alignment_interface (issue 7768043)
In general, I see a lot of reassigning of parents. What is the goal with this (sorry if you've explained this already)? https://codereview.appspot.com/7768043/diff/38001/lily/fingering-engraver.cc File lily/fingering-engraver.cc (right): https://codereview.appspot.com/7768043/diff/38001/lily/fingering-engraver.cc#newcode142 lily/fingering-engraver.cc:142: Self_alignment_interface::aligned_on_x_parent (fingerings_[i]->self_scm ())); This needs to be a chained offset procedure. See chain_offset_callback elsewhere in the code. https://codereview.appspot.com/7768043/diff/38001/lily/paper-column.cc File lily/paper-column.cc (right): https://codereview.appspot.com/7768043/diff/38001/lily/paper-column.cc#newcode222 lily/paper-column.cc:222: /* Good work! https://codereview.appspot.com/7768043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
substantial changes in alignment interface - please review
Hi folks, after some work on self-alignment-interface i have reached the point when a big picture starts to emerge. I'd really appreciate some feedback, also from people who are not really into Lily internals (i'd like to know whether user interfaces produced by me are clear). Rietveld issue is here: https://codereview.appspot.com/7768043, but i *strongly* recommend reviewing these changes in a branch, so that you can see each of 11 commits separately. You can see them in dev/janek-alignment (commit c204c4a5bc865). I've also put these changes up on Github (https://github.com/janek-warchol/lilypond-git/commits/dev/janek-alignment), because Github allows to post inline comments while still viewing commits separately - maybe we should review all our code there? If you need some explanation to get an idea of what this code is doing at all, look here: http://lists.gnu.org/archive/html/lilypond-user/2013-03/msg00956.html What i'm going to do next? Add support fot alining on different extents; you can get an idea here: http://lists.gnu.org/archive/html/lilypond-devel/2012-06/msg00230.html i'm looking forward to reading your opinions, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Offline through Easter
Hi, we had a rain day today, so I had a lot of net time. This evening, the one-week internet pass from the hotel runs out (probably somewhere about 20:00 UTC or later). With some luck, I'll be able to put through the results of the last Patchy run (under way), but at any rate, I'll be offline soonish until Tuesday, and anyway, we'll be climbing tomorrow and driving home on Monday. So have a merry Easter, and I'll be back on track afterwards. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: instrument name patch reverted
"m...@mikesolomon.org" writes: > Hey all, > > I reverted the instrument name patch in staging. It might be worth trying to figure out the relation between the problems that this patch was addressing and the breakage of functionality that occured as a consequence of it. Maybe the patch goals can be accomplished in a manner not affecting existing code in such a way. Or one can show that the desired behaviors are fundamentally incompatible in which case we have to make a choice ultimately and figure out how we can minimize the impact on users. At any rate, I think that even outside of the stable release, an approach of "break and patchup everywhere" is not doing our users and code base favors. Anything requiring a "patchup everywhere" needs to be closely examined. Sometimes it might be the only reasonable way forward, but then we need a migration strategy for both ourselves as well as our users. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
instrument name patch reverted
Hey all, I reverted the instrument name patch in staging. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [Lilypond-auto] Issue 3256 in lilypond: Patch: Eliminates side poisition calculations for system-start-text grobs.
Apparently not posted on the issue itself, so replying to the list. "m...@mikesolomon.org" writes: > On 30 mars 2013, at 08:27, lilyp...@googlecode.com wrote: >> commit 6e4e69f2735a764eab2e6f70f83546461da0203b >> Author: Mike Solomon >> Date: Fri Mar 29 05:55:13 2013 +0100 >> >>Uses special X alignment for instrument names. >> >> I propose reverting it since using the instrumentName for prefatory >> material is documented and previously working behavior >> anadam.spi...@gmail.com d it is not >> clear what this will break in addition to incipits. So it is not >> really fit to be included in a stable release at this point of time. > > It is not clear what this patch does and doesn't break until people > use it. Unfortunately, the regtest checker does not pick up on > certain differences, so it is important for users testing the software > to signal these. I fix them right away. You can't fix what has not been reported, and we are talking about functionality that is used by users, based on recommendations in our code base and snippets. > You are absolutely right that it is not clear what this will break in > addition to incipits, but I'd rather deal with that as it crops up The time frame for it to crop up is not available if we are planning to make a stable release. > as it is impossible to know given our current regtest checking > suite. With Phil's pixel comparator this sort of problem will go away, > but that'd need to get integrated into the build system. > > Fixes are quick, so I just need to know the problem. I can then fix > it ASAP. You can't fix the code from users. We have had several followup issues from this patch already which makes very clear that code in our own code base relied on previous behavior. It is far too optimistic to assume that user-level code would not rely on previous behavior. Fine-tuning the balance between improvement and damage is a process taking months, based on feedback from users as well as discoveries in our own code base from developers. The fallout we have seen so far makes _very_ clear that this patch is unsuitable for inclusion in a stable release. Mixing it with several spontaneous followup issues will cause things to go uncontrollably unstable. Either this patch can be done well-confined _without_ numerous repercussions (and that does not mean rolling lots of loosely related followup fixes elsewhere into the same patch, but an actually well-confined patch with well-confined and expected changes), or it is off the plate for a timely stable release. The desire to push in patches like this was the reason I called off the stable release in the first place. It was you who said we should try for it if it is feasible. This patch obviously makes it unfeasible, and if you try wedging in a flurry of followup patches to make up for it, there will be no stable release. It was fine to try and see whether a long-standing problem can be fixed with a reasonable amount of effort and impact. But when one sees that the impact is quite larger than expected, one needs to adapt one's plans. You can't have both a stable release and patches pushed in at last minute that are shown to have a large destabilizing impact. It is not possible to have both, and I am tired of being fought as the bad guy for having to point that out time and again. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allows outside staff positioning for vertical axis groups with no staff (issue 8188044)
On 30 mars 2013, at 10:29, k-ohara5...@oco.net wrote: > Simpler to just reverse the "special X-alignment" commit. > > The old code, that followed the comment about a 'kldge' whatever that > means, was rather elegant. If we are placing something alongside an > empty set, place it against the reference point for that set. Having a > special case for StanzaNumbers and another special case for empty > VerticalAxisGroups gets messy. > > https://codereview.appspot.com/8188044/ Fair enough, revert and I'll add a comment. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allows outside staff positioning for vertical axis groups with no staff (issue 8188044)
On 30 mars 2013, at 08:33, k-ohara5...@oco.net wrote:
> Why not use the flat-line for empty VerticalAxisGroups specifically in
> side-position-interface.cc when placing something against the staff (in
> the clause if (include_staff) {} ) ?
include_staff is false for VerticalAxisGroups without staves because there is
no staff grob.
>
> If you make each empty VerticalAxisGroup into a line, wouldn't that
> prevent interleaving beteen left- and right-hand piano staves, if there
> is a Dynamics in between that happens to be empty ?
This wouldn't happen (at least I don't think it would after a quick test) -
vertical axis group spacing is controlled in page-layout-problem.cc and doesn't
use the same mechanisms.
Cheers,
MS
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [Lilypond-auto] Issue 3256 in lilypond: Patch: Eliminates side poisition calculations for system-start-text grobs.
On 30 mars 2013, at 08:27, lilyp...@googlecode.com wrote: > Updates: > Labels: -Patch-push > > Comment #8 on issue 3256 by d...@gnu.org: Patch: Eliminates side poisition > calculations for system-start-text grobs. > http://code.google.com/p/lilypond/issues/detail?id=3256 > > This patch has broken incipits, like in input/regression/incipit.ly. Its > commit message is _totally_ different from the issue description or the > Rietveld issue description. It does not contain the issue number either. > You have not marked the issue as fixed, and the commit id is not to be found > anywhere. > > In consequence, it is not possible to track the commit to this particular > issue and review. It is not the first time, either. Please keep the commit > messages and the issue and review descriptions the same and mark the issues > as fixed _including_ the commit id _right_ _after_ _pushing_. > > The commit in question is > commit 6e4e69f2735a764eab2e6f70f83546461da0203b > Author: Mike Solomon > Date: Fri Mar 29 05:55:13 2013 +0100 > >Uses special X alignment for instrument names. > >Previously, instrument names used the side-position-interface for >calculations in its alignment function. This made no sense, as it never >had any side-support elements, requiring a kludge to get them to >align properly. > >Here, we remove that kludge and align them by aligning them to their >right end and adding padding. We also change the name of the grob array >in which vertical axis groups are stored to >system-start-text-align-elements so that the grob is not accidentally >treated as an axis-group implementing an elements array. > > I propose reverting it since using the instrumentName for prefatory material > is documented and previously working behavior and it is not clear what this > will break in addition to incipits. So it is not really fit to be included > in a stable release at this point of time. > It is not clear what this patch does and doesn't break until people use it. Unfortunately, the regtest checker does not pick up on certain differences, so it is important for users testing the software to signal these. I fix them right away. You are absolutely right that it is not clear what this will break in addition to incipits, but I'd rather deal with that as it crops up as it is impossible to know given our current regtest checking suite. With Phil's pixel comparator this sort of problem will go away, but that'd need to get integrated into the build system. Fixes are quick, so I just need to know the problem. I can then fix it ASAP. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allows outside staff positioning for vertical axis groups with no staff (issue 8188044)
Simpler to just reverse the "special X-alignment" commit. The old code, that followed the comment about a 'kldge' whatever that means, was rather elegant. If we are placing something alongside an empty set, place it against the reference point for that set. Having a special case for StanzaNumbers and another special case for empty VerticalAxisGroups gets messy. https://codereview.appspot.com/8188044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
