make calc-blot from bar-line.scm public

make calc-blot from bar-line.scm public

To facilitate defining custom-bar-lines using
ly:round-filled-box

[dak, 2016-06-05 15:51:25]

https://codereview.appspot.com/299250044/diff/1/scm/bar-line.scm
File scm/bar-line.scm (right):

https://codereview.appspot.com/299250044/diff/1/scm/bar-line.scm#newcode30
scm/bar-line.scm:30: (define-public (calc-blot thickness extent grob)
For a public function, the name is awful.  When made public, the naming should
be related to the functionality of the function from a general viewpoint rather
than its function within this file only.

Also I don't actually see how the "commit message" relates to making calc-blot
public:

"To facilitate defining custom-bar-lines using ly:round-filled-box"

Huh?  You mean, instead of having to use ly:round-filled-box directly?  Because
ly:round-filled-box should already be available anyway.  Or something else?

[thomasmorley651, 2016-06-05 17:27:15]

I expected this one to be a real nobrainer...

Anyway,

On 2016/06/05 15:51:25, dak wrote:
> https://codereview.appspot.com/299250044/diff/1/scm/bar-line.scm
> File scm/bar-line.scm (right):
> 
> https://codereview.appspot.com/299250044/diff/1/scm/bar-line.scm#newcode30
> scm/bar-line.scm:30: (define-public (calc-blot thickness extent grob)
> For a public function, the name is awful.  When made public, the naming should
> be related to the functionality of the function from a general viewpoint
rather
> than its function within this file only.

Understood. Right now I don't have a good idea for the naming, though.
At least one which would be short enough.

> Also I don't actually see how the "commit message" relates to making calc-blot
> public:
> 
> "To facilitate defining custom-bar-lines using ly:round-filled-box"
> 
> Huh?  You mean, instead of having to use ly:round-filled-box directly? 
Because
> ly:round-filled-box should already be available anyway.  Or something else?

Ok, the message _is_ awful.
I wanted to express: 
If a user copies p.e. `make-simple-bar-line' in order to do some minor
modifications to fit his personal needs, he will not also need to c/p
`calc-blot`, although it is used to determine the blot which is consumed by the
there used ly:round-filled-box-procedure.

I'll set it to need-work on the tracker for now, so that James doesn't need to
test.

[dak, 2016-06-05 17:39:35]

On 2016/06/05 17:27:15, thomasmorley651 wrote:
> I expected this one to be a real nobrainer...

Well, sorry for that.
 
> Anyway,
> 
> On 2016/06/05 15:51:25, dak wrote:
> > https://codereview.appspot.com/299250044/diff/1/scm/bar-line.scm
> > File scm/bar-line.scm (right):
> > 
> > https://codereview.appspot.com/299250044/diff/1/scm/bar-line.scm#newcode30
> > scm/bar-line.scm:30: (define-public (calc-blot thickness extent grob)
> > For a public function, the name is awful.  When made public, the naming
should
> > be related to the functionality of the function from a general viewpoint
> rather
> > than its function within this file only.
> 
> Understood. Right now I don't have a good idea for the naming, though.
> At least one which would be short enough.

bar-line::calc-blot would likely already fit the bill.

> > Also I don't actually see how the "commit message" relates to making
calc-blot
> > public:
> > 
> > "To facilitate defining custom-bar-lines using ly:round-filled-box"
> > 
> > Huh?  You mean, instead of having to use ly:round-filled-box directly? 
> Because
> > ly:round-filled-box should already be available anyway.  Or something else?
> 
> Ok, the message _is_ awful.
> I wanted to express: 
> If a user copies p.e. `make-simple-bar-line' in order to do some minor
> modifications to fit his personal needs, he will not also need to c/p
> `calc-blot`, although it is used to determine the blot which is consumed by
the
> there used ly:round-filled-box-procedure.

Is it used only there?  Should its define just be moved inside of the function? 
Then you formally would not "also need to c/p calc-blot" because it would
already be copied from inside.

> I'll set it to need-work on the tracker for now, so that James doesn't need to
> test.

At any rate: are the arguments for calc-blot such that it's only conceivably
useful inside of bar-lines?  If not, a better name would be desirable.  If so,
try fudging "bar-line" into its name.

[thomasmorley651, 2016-06-13 21:29:06]

bar-line::draw-filled-box

[thomasmorley651, 2016-06-13 21:40:56]

On 2016/06/05 17:39:35, dak wrote:
> On 2016/06/05 17:27:15, thomasmorley651 wrote:

> > On 2016/06/05 15:51:25, dak wrote:
> > > https://codereview.appspot.com/299250044/diff/1/scm/bar-line.scm
> > > File scm/bar-line.scm (right):
> > > 
> > > https://codereview.appspot.com/299250044/diff/1/scm/bar-line.scm#newcode30
> > > scm/bar-line.scm:30: (define-public (calc-blot thickness extent grob)
> > > For a public function, the name is awful.  

> bar-line::calc-blot would likely already fit the bill.

Done.

> > > Also I don't actually see how the "commit message" relates to making
> calc-blot
> > > public:
> > > 
> > > "To facilitate defining custom-bar-lines using ly:round-filled-box"
> > > 
> > > Huh?  You mean, instead of having to use ly:round-filled-box directly? 
> > Because
> > > ly:round-filled-box should already be available anyway.  Or something
else?
> > 
> > Ok, the message _is_ awful.
> > I wanted to express: 
> > If a user copies p.e. `make-simple-bar-line' in order to do some minor
> > modifications to fit his personal needs, he will not also need to c/p
> > `calc-blot`, although it is used to determine the blot which is consumed by
> the
> > there used ly:round-filled-box-procedure.
> 
> Is it used only there?  

calc-blot is used in
 make-simple-bar-line
 make-thick-bar-line
 make-tick-bar-line

> Should its define just be moved inside of the function? 
> Then you formally would not "also need to c/p calc-blot" because it would
> already be copied from inside.

Now the new bar-line::draw-filled-box is defined public calling renamed
bar-line::calc-blot

> At any rate: are the arguments for calc-blot such that it's only conceivably
> useful inside of bar-lines?  If not, a better name would be desirable.  If so,
> try fudging "bar-line" into its name.

Only useful inside of bar-lines as far as I can tell.

Commit message is now:

    Issue 4883 Implement public definition of bar-line::draw-filled-box
    
    bar-line::draw-filled-box calls the hereby renamed procedure
    bar-line::calc-blot to determine the blot diameter.
    It will be used in:
      make-simple-bar-line
      make-thick-bar-line
      make-tick-bar-line
    This simplifies user-customized bar-lines, which are build with
    ly:round-filled-box.
    No need to copy/paste the blot-calculating procedure anymore.