Allows lyrics to slide under TimeSignature when OctaveEight present.

Allows lyrics to slide under TimeSignature when OctaveEight present.

[janek, 2012-05-10 21:40:48]

http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver.cc
File lily/pure-from-neighbor-engraver.cc (right):

http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver...
lily/pure-from-neighbor-engraver.cc:56: in_same_column (Grob *g1, Grob *g2)
That's problably the most stupid question ever, but why this doesn't it begin
with Pure_from_neighbor_engraver:: ?  I don't see it being special.

http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver...
lily/pure-from-neighbor-engraver.cc:119: Pointer_group_interface::add_grob
(need_pure_heights_from_neighbors[pos[j]][k], ly_symbol2scm ("neighbors"),
pure_relevants_[i]);
could you reduce line width, please?  This one is way over regular 80 chars
limit.

[Carl, 2012-05-10 23:48:29]

LGTM, but I concur with the line-wrap comment of Janek.

[MikeSol, 2012-05-11 05:23:27]

http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver.cc
File lily/pure-from-neighbor-engraver.cc (right):

http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver...
lily/pure-from-neighbor-engraver.cc:56: in_same_column (Grob *g1, Grob *g2)
On 2012/05/10 21:40:48, janek wrote:
> That's problably the most stupid question ever, but why this doesn't it begin
> with Pure_from_neighbor_engraver:: ?  I don't see it being special.

It doesn't need to be a class method - it'll never be called outside of this
file.  It is just a small helper function.

http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver...
lily/pure-from-neighbor-engraver.cc:119: Pointer_group_interface::add_grob
(need_pure_heights_from_neighbors[pos[j]][k], ly_symbol2scm ("neighbors"),
pure_relevants_[i]);
On 2012/05/10 21:40:48, janek wrote:
> could you reduce line width, please?  This one is way over regular 80 chars
> limit.

Done.

[dak, 2012-05-11 07:09:04]

http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver.cc
File lily/pure-from-neighbor-engraver.cc (right):

http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver...
lily/pure-from-neighbor-engraver.cc:56: in_same_column (Grob *g1, Grob *g2)
On 2012/05/11 05:23:27, MikeSol wrote:
> On 2012/05/10 21:40:48, janek wrote:
> > That's problably the most stupid question ever, but why this doesn't it
begin
> > with Pure_from_neighbor_engraver:: ?  I don't see it being special.
> 
> It doesn't need to be a class method - it'll never be called outside of this
> file.  It is just a small helper function.

Then it should be declared static, right?

[mike_apollinemike.com, 2012-05-11 08:02:16]

On 11 mai 2012, at 09:09, dak@gnu.org wrote:

> 
>
http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver.cc
> File lily/pure-from-neighbor-engraver.cc (right):
> 
>
http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver...
> lily/pure-from-neighbor-engraver.cc:56: in_same_column (Grob *g1, Grob
> *g2)
> On 2012/05/11 05:23:27, MikeSol wrote:
>> On 2012/05/10 21:40:48, janek wrote:
>>> That's problably the most stupid question ever, but why this doesn't
> it begin
>>> with Pure_from_neighbor_engraver:: ?  I don't see it being special.
> 
>> It doesn't need to be a class method - it'll never be called outside
> of this
>> file.  It is just a small helper function.
> 
> Then it should be declared static, right?
> 

Why would it be part of the class?  It's just a small helper function -
architecturally, I don't see it as being part of a class.  There are small
helper functions like this all over the code base.

Cheers,
MS

[dak, 2012-05-11 08:11:18]

"mike@apollinemike.com" <mike@apollinemike.com> writes:

> On 11 mai 2012, at 09:09, dak@gnu.org wrote:
>
>> 
>>
http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver.cc
>> File lily/pure-from-neighbor-engraver.cc (right):
>> 
>>
http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver...
>> lily/pure-from-neighbor-engraver.cc:56: in_same_column (Grob *g1, Grob
>> *g2)
>> On 2012/05/11 05:23:27, MikeSol wrote:
>>> On 2012/05/10 21:40:48, janek wrote:
>>>> That's problably the most stupid question ever, but why this doesn't
>> it begin
>>>> with Pure_from_neighbor_engraver:: ?  I don't see it being special.
>> 
>>> It doesn't need to be a class method - it'll never be called outside
>> of this
>>> file.  It is just a small helper function.
>> 
>> Then it should be declared static, right?
>> 
>
> Why would it be part of the class?  It's just a small helper function
> - architecturally, I don't see it as being part of a class.  There are
> small helper functions like this all over the code base.

I did not say it should be declared a class function.  I said that it
should be declared static since it is not referenced outside of the
source file.

-- 
David Kastrup

[MikeSol, 2012-05-17 06:05:39]

On 2012/05/11 08:11:18, dak wrote:
> "mike@apollinemike.com" <mailto:mike@apollinemike.com> writes:
> 
> > On 11 mai 2012, at 09:09, mailto:dak@gnu.org wrote:
> >
> >> 
> >>
>
http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver.cc
> >> File lily/pure-from-neighbor-engraver.cc (right):
> >> 
> >>
>
http://codereview.appspot.com/6201068/diff/1/lily/pure-from-neighbor-engraver...
> >> lily/pure-from-neighbor-engraver.cc:56: in_same_column (Grob *g1, Grob
> >> *g2)
> >> On 2012/05/11 05:23:27, MikeSol wrote:
> >>> On 2012/05/10 21:40:48, janek wrote:
> >>>> That's problably the most stupid question ever, but why this doesn't
> >> it begin
> >>>> with Pure_from_neighbor_engraver:: ?  I don't see it being special.
> >> 
> >>> It doesn't need to be a class method - it'll never be called outside
> >> of this
> >>> file.  It is just a small helper function.
> >> 
> >> Then it should be declared static, right?
> >> 
> >
> > Why would it be part of the class?  It's just a small helper function
> > - architecturally, I don't see it as being part of a class.  There are
> > small helper functions like this all over the code base.
> 
> I did not say it should be declared a class function.  I said that it
> should be declared static since it is not referenced outside of the
> source file.
> 
> -- 
> David Kastrup

Hey David,

I don't completely understand the hows and whys of it, and there are many
functions in the source code that are only used in one file but are not declared
static.  If all of these should be declared static, I'd recommend opening a
tracker issue describing the general problem.

Cheers,
MS