On Jun 21, 2011, at 3:04 PM, lemniskata.bernoull...@gmail.com wrote: > > http://codereview.appspot.com/4609041/diff/12001/scm/define-grobs.scm > File scm/define-grobs.scm (right): > > http://codereview.appspot.com/4609041/diff/12001/scm/define-grobs.scm#newcode141 > scm/define-grobs.scm:141: (woot . 1) > On 2011/06/17 07:18:49, MikeSol wrote: >> This seems like 1337 $p34k - >> I have never heard woot in any other context. >> Perhaps change to something more descriptive? > > Of course! > I had no idea for the name and decided to use a placeholder and ask you > instead of wasting 15 minutes on something so simple (i was quite tired > when i wrote this code). > > http://codereview.appspot.com/4609041/diff/12001/scm/output-lib.scm > File scm/output-lib.scm (right): > > http://codereview.appspot.com/4609041/diff/12001/scm/output-lib.scm#newcode944 > scm/output-lib.scm:944: (linear-gap (+ (max gap-property 0.3) -0.45 > On 2011/06/17 07:18:49, MikeSol wrote: >> Indentation: the -0.45 should be on the next line & lined up with the >> left-parenthesis of (max. > > Done. > > http://codereview.appspot.com/4609041/diff/12001/scm/output-lib.scm#newcode950 > scm/output-lib.scm:950: (unwooted (max (min calculated-gap gap-property) > (/ gap-property 4.5))) > On 2011/06/17 07:18:49, MikeSol wrote: >> 80 column max > > Done. > > http://codereview.appspot.com/4609041/diff/12001/scm/output-lib.scm#newcode951 > scm/output-lib.scm:951: (gap (+ (* unwooted woot) (* gap-property (- 1 > woot)))) > On 2011/06/17 07:18:49, MikeSol wrote: >> This codes a lot of properties. I'm fine with the code (though I'd > need to see >> a regtest). Can you try using a "details" property (like for the Beam > grob) >> that stores all of these values? > > Umm, I don't want to define properties like linear-gap, calculated-gap > etc. They are just temporary variables so that the code calculating > final gap is easier to read. Had i not used them, i would have to write > everything explicitely like this (with better indentation perhaps): > > (gap > (+ > (* > (max > (min > (if > (< > (+ > (max (ly:grob-property grob 'gap 0.35) 0.3) > -0.45 > (* > 0.2 > (- > (interval-start (ly:grob-extent head-up common Y)) > (interval-end (ly:grob-extent head-down common > Y))))) > 0.2) > (+ > (max (ly:grob-property grob 'gap 0.35) 0.3) > -0.45 > (* > 0.2 > (- > (interval-start (ly:grob-extent head-up common Y)) > (interval-end (ly:grob-extent head-down common Y))))) > (+ > (* > (floor > (/ > (- > (+ > (max (ly:grob-property grob 'gap 0.35) 0.3) > -0.45 > (* > 0.2 > (- > (interval-start (ly:grob-extent head-up > common Y)) > (interval-end (ly:grob-extent head-down > common Y))))) > 0.2) > 0.25)) > 0.25) > 0.2)) > (ly:grob-property grob 'gap 0.35)) > (/ (ly:grob-property grob 'gap 0.35) 4.5)) > (ly:grob-property grob 'woot 1)) > (* (ly:grob-property grob 'gap 0.35) > (- 1 (ly:grob-property grob 'woot 1))))) > > looks suicidal... > When i noticed that point-max and point-min don't seem to be any > properties but only a sort of temporary variables, i used this idea for > my piece of code. Maybe i didn't understand how this works...
What I meant is that every time you use a magic number (i.e. 0.35), consider making it user-tweakable unless you are absolutely sure that there is no utility in changing that number. Cheers, MS _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
