Remove ly:grob-properties

Remove ly:grob-properties

This function is hard to use correctly:

* one cannot set the head of the alist, so it cannot be used for
  modifications.

* it will not return the properties that were \overridden

Instead, ly:grob-property should be used. 

This paves the way to further work on (grob) property storage
mechanisms.

Fixes
Documentation/snippets/overriding-articulations-of-destinct-type,
which should be mirrored back into LSR.

NOSUBMIT: this is an incompatible change; where are the release notes?

[dak, 2020-04-11 15:59:09]

https://codereview.appspot.com/549840044/diff/567430043/lily/grob-scheme.cc
File lily/grob-scheme.cc (left):

https://codereview.appspot.com/549840044/diff/567430043/lily/grob-scheme.cc#o...
lily/grob-scheme.cc:327: LY_DEFINE (ly_grob_basic_properties,
"ly:grob-basic-properties",
There is really no point in removing ly:grob-properties while retaining
ly:grob-basic-properties as they are a pair of accessors.

[dak, 2020-04-11 16:02:31]

The description says "[ly:grob-properties] will not return the properties that
were \overridden."

Aren't you confusing this with ly:grob-basic-properties ?  I think
ly:grob-properties will actually _only_ return the properties that were
overriden.

[thomasmorley651, 2020-04-11 16:31:48]

The change in the snippet is ofcourse nice.

Though, please don't delete ly:grob-properties or ly:grob-basic-properties.
I use them every day.
They are tools to get insights at the grob at hand.
Don't judge from our code base and the few occurencies there.
Btw, it's used in
input/regression/multi-measure-rest-reminder.ly
input/regression/scheme-text-spanner.ly
as well.

Again, it's in every days use!!!

https://codereview.appspot.com/549840044/diff/567430043/Documentation/snippet...
File Documentation/snippets/overriding-articulations-of-destinct-type.ly
(right):

https://codereview.appspot.com/549840044/diff/567430043/Documentation/snippet...
Documentation/snippets/overriding-articulations-of-destinct-type.ly:31:
(ly:grob-property grob 'cause)
Yep, that's better here.

[thomasmorley651, 2020-04-11 16:32:58]

On 2020/04/11 16:31:48, thomasmorley651 wrote:

> Btw, it's used in
> input/regression/multi-measure-rest-reminder.ly
> input/regression/scheme-text-spanner.ly
> as well.

No, that's ly:grob-properties?
Bad grepping ...

[dak, 2020-04-11 16:36:44]

On 2020/04/11 16:32:58, thomasmorley651 wrote:
> On 2020/04/11 16:31:48, thomasmorley651 wrote:
> 
> > Btw, it's used in
> > input/regression/multi-measure-rest-reminder.ly
> > input/regression/scheme-text-spanner.ly
> > as well.
> 
> No, that's ly:grob-properties?
> Bad grepping ...

Also icky name overlaps.  ly:grob-properties? was last.  Maybe it should be
ly:context-grob-properties? instead?  It's the thing contexts use for tracking
grob properties.

[hanwenn, 2020-04-11 16:38:28]

On 2020/04/11 16:02:31, dak wrote:
> The description says "[ly:grob-properties] will not return the properties that
> were \overridden."
> 
> Aren't you confusing this with ly:grob-basic-properties ?  I think
> ly:grob-properties will actually _only_ return the properties that were
> overriden.

\override pushes data onto the immutable list (or at least, it used to be like
that). So if you do \override Stem.length = #5 , you won't see 'length in the
return value of ly:grob-properties

I thought of removing the other function too, and I agree in principle, but it
can only break more user files. As long as we're not considering reorganizing
the immutable lists, I think it can stay.

[thomasmorley651, 2020-04-11 16:38:38]

> Fixes
> Documentation/snippets/overriding-articulations-of-destinct-type,
> which should be mirrored back into LSR.

Fixed in LSR

[dak, 2020-04-11 17:19:19]

On 2020/04/11 16:38:28, hanwenn wrote:
> On 2020/04/11 16:02:31, dak wrote:
> > The description says "[ly:grob-properties] will not return the properties
that
> > were \overridden."
> > 
> > Aren't you confusing this with ly:grob-basic-properties ?  I think
> > ly:grob-properties will actually _only_ return the properties that were
> > overriden.
> 
> \override pushes data onto the immutable list (or at least, it used to be like
> that).

In effect you are right (and I misremembered), but override pushes onto
context-maintained lists which are ultimately used as template when make_grob
gets called and _then_ the overriden properties end up on the immutable list.

> I thought of removing the other function too, and I agree in principle, but it
> can only break more user files. As long as we're not considering reorganizing
> the immutable lists, I think it can stay.

Note that there is no necessity of returning a bonafide part of the grob
structure: one can construct a list of properties on the fly.  Particularly
because this is used read-only and only occasionally.

[hanwenn, 2020-04-11 17:44:11]

On 2020/04/11 17:19:19, dak wrote:
> > I thought of removing the other function too, and I agree in principle, but
it
> > can only break more user files. As long as we're not considering
reorganizing
> > the immutable lists, I think it can stay.
> 
> Note that there is no necessity of returning a bonafide part of the grob
> structure: one can construct a list of properties on the fly.  Particularly
> because this is used read-only and only occasionally.

I know. But if we make a copy, that will be expensive too.

Any comment on 

NOSUBMIT: this is an incompatible change; where are the release notes?

[dak, 2020-04-11 18:37:50]

On 2020/04/11 17:44:11, hanwenn wrote:
> On 2020/04/11 17:19:19, dak wrote:
> > > I thought of removing the other function too, and I agree in principle,
but
> it
> > > can only break more user files. As long as we're not considering
> reorganizing
> > > the immutable lists, I think it can stay.
> > 
> > Note that there is no necessity of returning a bonafide part of the grob
> > structure: one can construct a list of properties on the fly.  Particularly
> > because this is used read-only and only occasionally.
> 
> I know. But if we make a copy, that will be expensive too.

It's either imprudent user code or code that actually potentially uses all the
properties.  Either way I'd not be worried about the cost.  That we don't want
to use it in the internals for regular use then is obvious.

[hanwenn, 2020-05-01 19:56:59]

commit 7688936a4e598bc9919e8a9adceb3ba1c9b2c23c
Author: Han-Wen Nienhuys <hanwen@lilypond.org>
Date:   Sat Apr 11 17:49:38 2020 +0200

    Remove ly:grob-properties