Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-17 Thread hanwenn
On 2020/04/13 22:10:25, dan_faithful.be wrote: > On Apr 13, 2020, at 16:31, mailto:hanw...@gmail.com wrote: > > > >> Does const serve a purpose here? The iterator doesn't carry through > > with any > >> kind of enforcement. The same question applies to try_retrieve() and > >> to_alist(). > > >

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread Dan Eble
On Apr 13, 2020, at 16:31, hanw...@gmail.com wrote: > >> Does const serve a purpose here? The iterator doesn't carry through > with any >> kind of enforcement. The same question applies to try_retrieve() and >> to_alist(). > > It allows one to iterate over properties in a const method. > > Wha

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread dak
On 2020/04/13 20:27:34, hanwenn wrote: > On 2020/04/13 20:10:35, dak wrote: > > > https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh > > File lily/include/mutable-properties.hh (right): > > > > > https://codereview.appspot.com/561640043/diff/565900043/lily/i

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread hanwenn
https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh File lily/include/mutable-properties.hh (right): https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh#newcode54 lily/include/mutable-properties.hh:54: Iterator iter()

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread hanwenn
On 2020/04/13 20:10:35, dak wrote: > https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh > File lily/include/mutable-properties.hh (right): > > https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh#newcode31 > lily/include

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread dak
https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh File lily/include/mutable-properties.hh (right): https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh#newcode31 lily/include/mutable-properties.hh:31: class Iterator {

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread jonas . hahnfeld
On 2020/04/13 17:28:16, Dan Eble wrote: > On 2020/04/13 17:22:52, hanwenn wrote: > > > > If you're not interested in doing this, I might try it myself. > > > > By structuring it like this, you enforce the implementation to store the > > key/value as SCM cells, which is exactly what we want to get

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread nine . fierce . ballads
On 2020/04/13 17:22:52, hanwenn wrote: > > If you're not interested in doing this, I might try it myself. > > By structuring it like this, you enforce the implementation to store the > key/value as SCM cells, which is exactly what we want to get away from. OK, I didn't understand that from the d

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread hanwenn
https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh File lily/include/mutable-properties.hh (right): https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh#newcode31 lily/include/mutable-properties.hh:31: class Iterator {

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread nine . fierce . ballads
https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh File lily/include/mutable-properties.hh (right): https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh#newcode31 lily/include/mutable-properties.hh:31: class Iterator {

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread hanwenn
On 2020/04/13 16:03:00, hahnjo wrote: > On 2020/04/13 15:06:16, hanwenn wrote: > > https://codereview.appspot.com/561640043/diff/565900043/lily/grob-scheme.cc > > File lily/grob-scheme.cc (left): > > > > > https://codereview.appspot.com/561640043/diff/565900043/lily/grob-scheme.cc#oldcode326 > > l

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread jonas . hahnfeld
On 2020/04/13 15:06:16, hanwenn wrote: > https://codereview.appspot.com/561640043/diff/565900043/lily/grob-scheme.cc > File lily/grob-scheme.cc (left): > > https://codereview.appspot.com/561640043/diff/565900043/lily/grob-scheme.cc#oldcode326 > lily/grob-scheme.cc:326: > On 2020/04/13 14:56:45, h

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread hanwenn
https://codereview.appspot.com/561640043/diff/565900043/lily/grob-scheme.cc File lily/grob-scheme.cc (left): https://codereview.appspot.com/561640043/diff/565900043/lily/grob-scheme.cc#oldcode326 lily/grob-scheme.cc:326: On 2020/04/13 14:56:45, hahnjo wrote: > This should very probably not be p

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread hanwenn
Reviewers: lemzwerg, Message: On 2020/04/13 14:58:01, lemzwerg wrote: > From visual inspection, LGTM. > > I only wonder whether we should use > > if > { > foo(...); > } > > or > > if > foo(...); > > for single statements ­– right now, the source code contains both variants...

Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread jonas . hahnfeld
https://codereview.appspot.com/561640043/diff/565900043/lily/grob-scheme.cc File lily/grob-scheme.cc (left): https://codereview.appspot.com/561640043/diff/565900043/lily/grob-scheme.cc#oldcode326 lily/grob-scheme.cc:326: This should very probably not be part of this patch? https://codereview.a

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread lemzwerg--- via Discussions on LilyPond development
>From visual inspection, LGTM. I only wonder whether we should use if { foo(...); } or if foo(...); for single statements ­– right now, the source code contains both variants... https://codereview.appspot.com/561640043/