parenthesizeStencil and bracketifyStencil

parenthesizeStencil and bracketifyStencil

Issue 2646

These function can parenthesize/bracketify nearly all kind of stencils.

[thomasmorley65, 2012-07-13 21:10:16]

please review

[thomasmorley65, 2012-07-13 21:17:43]

2012/7/13  <thomasmorley65@googlemail.com>:
> Reviewers: ,
>
> Message:
> please review
>
> Description:
> parenthesizeStencil and bracketifyStencil
>
> Issue 2646
>
> These function can parenthesize/bracketify nearly all kind of stencils.
>
> Please review this at http://codereview.appspot.com/6397043/
>
> Affected files:
>   M Documentation/ly-examples/aucun-snippet.ly
>   A input/regression/parenthesizeStencil-bracketifyStencil.ly
>   M ly/music-functions-init.ly
>   M scm/define-markup-commands.scm
>   M scm/stencil.scm
>
>

This patch will break two other files:
/Documentation/snippets/time-signature-in-parentheses---method-3.ly
/Documentation/snippets/time-signature-in-parentheses.ly

Fixing these files is very easy.
But after reading the in-file-comment:

%% DO NOT EDIT this file manually; it is automatically
%% generated from LSR http://lsr.dsi.unimi.it
%% Make any changes in LSR itself, or in Documentation/snippets/new/ ,
%% and then run scripts/auxiliar/makelsr.py

I wasn't sure what to do.

-Harm

[mail_philholmes.net, 2012-07-13 21:46:10]

----- Original Message ----- 
From: "Thomas Morley" <thomasmorley65@googlemail.com>
To: <thomasmorley65@googlemail.com>
Cc: <reply@codereview-hr.appspotmail.com>; <lilypond-devel@gnu.org>
Sent: Friday, July 13, 2012 10:17 PM
Subject: Re: parenthesizeStencil and bracketifyStencil (issue 6397043)


> 2012/7/13  <thomasmorley65@googlemail.com>:
>> Reviewers: ,
>>
>> Message:
>> please review
>>
>> Description:
>> parenthesizeStencil and bracketifyStencil
>>
>> Issue 2646
>>
>> These function can parenthesize/bracketify nearly all kind of stencils.
>>
>> Please review this at http://codereview.appspot.com/6397043/
>>
>> Affected files:
>>   M Documentation/ly-examples/aucun-snippet.ly
>>   A input/regression/parenthesizeStencil-bracketifyStencil.ly
>>   M ly/music-functions-init.ly
>>   M scm/define-markup-commands.scm
>>   M scm/stencil.scm
>>
>>
>
> This patch will break two other files:
> /Documentation/snippets/time-signature-in-parentheses---method-3.ly
> /Documentation/snippets/time-signature-in-parentheses.ly
>
> Fixing these files is very easy.
> But after reading the in-file-comment:
>
> %% DO NOT EDIT this file manually; it is automatically
> %% generated from LSR http://lsr.dsi.unimi.it
> %% Make any changes in LSR itself, or in Documentation/snippets/new/ ,
> %% and then run scripts/auxiliar/makelsr.py
>
> I wasn't sure what to do.


Kind of what it says.  Files in /Documentation/snippets/ are not a permanent 
feature of the source - they get updated from the LSR whenever the 
LRS-meister grabs a tarball from the LSR and uses it to update the snippets. 
So - if your changes will break the snippet, but will only work in in future 
versions, copy the snippet to snippets/new and update it there.  Otherwise, 
update the snippet by editing the LSR.

--
Phil Holmes 

[thomasmorley65, 2012-07-13 22:13:30]

2012/7/13 Phil Holmes <mail@philholmes.net>:
(...)
> So - if your changes will break the snippet, but will only work in in future
> versions, copy the snippet to snippets/new and update it there.

Copy or cut and paste?
What about
%% and then run scripts/auxiliar/makelsr.py
?

-Harm

[mail_philholmes.net, 2012-07-14 10:42:43]

----- Original Message ----- 
From: "Thomas Morley" <thomasmorley65@googlemail.com>
To: "Phil Holmes" <mail@philholmes.net>
Cc: <reply@codereview-hr.appspotmail.com>; <lilypond-devel@gnu.org>
Sent: Friday, July 13, 2012 11:13 PM
Subject: Re: parenthesizeStencil and bracketifyStencil (issue 6397043)


> 2012/7/13 Phil Holmes <mail@philholmes.net>:
> (...)
>> So - if your changes will break the snippet, but will only work in in 
>> future
>> versions, copy the snippet to snippets/new and update it there.
>
> Copy or cut and paste?
> What about
> %% and then run scripts/auxiliar/makelsr.py
> ?
>
> -Harm


Copy will be fine.  makelsr uses the version in /new to overwrite the 
tarball version.  It's not always _necessary_ to run makelsr, since if you 
create a patch with a new snippet in /new, the LSR-meister (i.e. me) will at 
some point run makelsr and therefore update snippets.  However, if the 
changes break the snippet completely, and this breaks make doc, then you 
should run makelsr and then also include the new version of the snippet in 
/snippets in the patch.

Hope this makes some sense.  It's difficult to explain.

--
Phil Holmes 

[janek, 2012-07-14 12:11:09]

Hi, 

this is really cool!

A few general remarks:
- is there a command to turn it off?
- there already is a command \parenthesize.  Your function seems to be smarter
(at least in some cases); would it be to combine them?  Having two similar
functions is confusing.
- i get a segmentation fault when i tried to parenthesize a NoteColumn.
- it produces bad results when used on stemmed NoteHead's. [1]
- there are unsymmetric results for dynamics:

\relative c' {
  \bracketifyStencil DynamicText
  c1\p c\mp c\mf c\f
}
\relative c' {
  \parenthesizeStencil DynamicText
  c1\p c\mp c\mf c\f
}

I'm quite sure they are caused by glyphs bounding boxes which are inappropriate
for this task.  Have you seen Mike's work on stencil skylines (tracker 2148,
Rietveld 5626052)?  I think that it would be useful here.  See
http://lilypond-stuff.1065243.n5.nabble.com/skyline-integrals-test-results-21...
("skylines demonstration (patchset 40)" to see what is it all about.

[1] introducing inner- and outer-extents should help here. I'm working on
introducing them, together with general_alignment method. See this issue and
linked message http://code.google.com/p/lilypond/issues/detail?id=2613 (it will
take some time to implement this fully, though).

thanks!
Janek

[Valentin Villenave, 2015-04-12 23:52:49]

On 2012/07/14 12:11:09, janek wrote:
> this is really cool!

Hi everybody,
since this patch seems to have gone cold, I thought I’d update it a bit and
re-upload it:
https://codereview.appspot.com/226110044/