Re: make calc-blot from bar-line.scm public (issue 299250044 by address@hidden)

[dak]

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.

https://codereview.appspot.com/299250044/