Re: Deletes logfiles in build dir with make clean (issue 6300079)
I'm not certain cleaning the build tree should include log files cleaning, when a simple find -name '*.log' |xargs rm -f does the job. However, if this is a very popular request, then you may want to read a few comments. http://codereview.appspot.com/6300079/diff/1/GNUmakefile.in File GNUmakefile.in (right): http://codereview.appspot.com/6300079/diff/1/GNUmakefile.in#newcode340 GNUmakefile.in:340: find . -name *.log -delete I think something like rm -f *test*.log is enough here, we don't ask for removing all *.log files of build tree by making test-clean, do we? http://codereview.appspot.com/6300079/diff/1/stepmake/stepmake/generic-targets.make File stepmake/stepmake/generic-targets.make (right): http://codereview.appspot.com/6300079/diff/1/stepmake/stepmake/generic-targets.make#newcode16 stepmake/stepmake/generic-targets.make:16: find . -name *.log -delete make already recurses into subdirectories, so using find is superfluous, rm -f *.log is enough. Moreover, this would better go above $(LOOP) to preserve the logic of cleaning a directory *then* its subdirectories. http://codereview.appspot.com/6300079/diff/1/stepmake/stepmake/generic-targets.make#newcode209 stepmake/stepmake/generic-targets.make:209: find . -name *.log -delete This line is superfluous, as $(MAKE) out=www clean already executes actions specified in clean target. http://codereview.appspot.com/6300079/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Sets TabVoice Stem height to ##f (issue 6303065)
http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc File lily/grob.cc (right): http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc#newcode472 lily/grob.cc:472: real_ext[d] += offset; I don't understand this. The only way to get a nan from adding an offset to infinity is by adding another nan or an infinite offset with different sign. What case is this supposed to catch? http://codereview.appspot.com/6303065/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Sets TabVoice Stem height to ##f (issue 6303065)
On 2012/06/12 13:22:10, dak wrote: On 2012/06/12 12:54:40, MikeSol wrote: On 2012/06/12 12:49:45, dak wrote: http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc File lily/grob.cc (right): http://codereview.appspot.com/6303065/diff/10003/lily/grob.cc#newcode472 lily/grob.cc:472: real_ext[d] += offset; On 2012/06/12 12:32:37, dak wrote: I don't understand this. The only way to get a nan from adding an offset to infinity is by adding another nan or an infinite offset with different sign. What case is this supposed to catch? So what you actually meant to say was if (!real_ext.is_empty ()) real_ext.translate (offset); If that's what you mean, why don't you write it instead of some puzzle? This is not what I mean. An empty interval is, in LilyPond, an interval whose left is greater than it's right. I disagree. An empty interval is an interval not containing any point. The details are not important. So (3 . 2) is empty, but (3 . 2) can be translated by an infinite value in either direction and not result in nan. What meaning is there in translating an empty interval and afterwards getting an interval no longer being empty since its lower bound no longer is smaller than its upper bound? Are you, by chance, confusing an _interval_ with a tuple of points? You can _implement_ a closed interval as a tuple of points, but that does not lend meaning to shifting an empty interval. So just checking for emptiness isn't enough. For mimicking all side effects of the current implementation, it isn't. But if some code relies on those side effects, it is broken in my opinion. If you want to manipulate a two-dimensional vector without inherent relation between its two elements, use a Drul, not an Interval. And frankly, shouldn't we rather put this into flower/include/interval.hh instead of trying to catch this in arbitrary places in our code? I had toyed with this idea - this is a bit out of my league, as I don't know if LilyPond will take a performance hit if we add extra operations to something as elemental as translate or is_empty. Every piece of inherent safety takes a performance hit. Personally I think that the only case where we have is_empty true is for (+inf, -inf). And I don't mean that we should change the test: the test is fine. There just isn't a combination of interval bounds that makes more sense. We could equally well define the empty interval as (0, -1) and get consistent behavior from that. But while the interval is empty, its bounds should not be interpreted as conveying meaning. If all empty intervals are the same then, by design, we need to make sure that all of them equal (+inf.0 . -inf.0). This means that: --) Interval should have a method set_to_empty () that sets it to (+inf.0 . -inf.0) (maybe it already does). --) Anytime an interval is set to its left being greater than its right such that it is not infinitely empty, a programming error should be issued. --) Perhaps all math done on empty intervals should raise programming errors. I'm all for making the code more coherent. It sounds like we're talking about a much deeper problem than this patch, and perhaps it's wiser to come up w/ a solution to that first before pushing this. Cheers, MS http://codereview.appspot.com/6303065/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Sets TabVoice Stem height to ##f (issue 6303065)
On 2012/06/12 13:47:34, MikeSol wrote: On 2012/06/12 13:43:09, dak wrote: If a type is named Interval, it needs to be employed as an Interval, not as something totally different that relies on implementation details. Otherwise type abstraction makes code _less_ maintainable rather than more, since you always need to take all side-effects into consideration. Dunno - it sounds like time for clean up. Maybe. It does not make sense in my opinion to pepper the whole code with fixups intended to cater for cases where intervals are supposed to be used as non-intervals, or with fixups in order to cater for cases where intervals are supposed to be used as intervals, and we can't put the respective fixes into the interval implementation itself since it would violate the assumptions of those uses where intervals are used as non-intervals. Y-position of beams were stored as intervals for years (they may still be - I forget). Storing Y-positions of beams in intervals is fine when we are talking about a possible set of definitions for Y-positions at one point. When we are talking about lower interval bound is left Y-position, higher interval bound is right Y-position, and left Y-position may be higher than right Y-position, we are venturing into the domain of nonsense. This sounds like a pretty major task, so as always, I'd touch base w/ Han-Wen to see what Intervals were supposed to be at their inception and then evaluate what they've grown into. We can then firm up an Interval API and write a strongly-worded comment in interval.hh and interval.tcc NOT to touch either of these files. Sounds like it would make sense. And _if_ we have use cases for ordered point/value pairs (like using Linear_combination or whatever), then we should make sure that those are available on _appropriate_ data types as well rather than misusing Intervals for them because their internals happen to be convenient for that. http://codereview.appspot.com/6303065/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
giving out git push ability
Who thinks they should have git push ability? I usually tell people not to ask unless prompted, so now I'm prompting. General rule of thumb is that you should have a bunch of patches accepted, and generally be a trustworthy character. Or something like that. - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
PATCH: Countdown to 20120614
For 20:00 MDT Thursday June 14 Critical: Issue 2587 http://code.google.com/p/lilypond/issues/detail?id=2587: Ugly slur in TabStaff if font is changed - R 6303065 http://codereview.appspot.com/6303065/ Defect: Issue 582 http://code.google.com/p/lilypond/issues/detail?id=582: slur and tie start in (almost) same spot - R 6301067 http://codereview.appspot.com/6301067/ Issue 2574 http://code.google.com/p/lilypond/issues/detail?id=2574: Footnote on first time signature gets mark but no footnote text - R 6306064 http://codereview.appspot.com/6306064/ Documentation: Issue 2522 http://code.google.com/p/lilypond/issues/detail?id=2522: duration*0 considered harmful - R 6197068 http://codereview.appspot.com/6197068/ Enhancement: Issue 2598 http://code.google.com/p/lilypond/issues/detail?id=2598: Patch: Allow brackets and parens to start lyrics, disallow accents - R 6297074 http://codereview.appspot.com/6297074/ Issue 2596 http://code.google.com/p/lilypond/issues/detail?id=2596: Patch: Protect a few events from gc - R 6303066 http://codereview.appspot.com/6303066/ Cheers, Colin -- I've learned that you shouldn't go through life with a catcher's mitt on both hands. You need to be able to throw something back. -Maya Angelou, poet (1928- ) ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
