Re: ambitus: special handling of small ambits' lines (issue4609041)

[address@hidden]

On Jun 21, 2011, at 3:04 PM, address@hidden 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