Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-10 Thread Carl Sorensen
On 7/10/11 3:09 PM, Neil Puttock n.putt...@gmail.com wrote: On 7 July 2011 23:22, carl.d.soren...@gmail.com wrote: On 2011/07/07 16:12:08, Neil Puttock wrote: LGTM, though the regtest is still a bit messy. What do you consider messy about the regtest? Just a few nitpicks: +\score{

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-10 Thread Neil Puttock
On 10 July 2011 23:34, Carl Sorensen c_soren...@byu.edu wrote: Ahh, thanks.  Fixed now: {  \new Voice \with  {    \consists Ambitus_engraver    \consists Mensural_ligature_engraver Heh, you didn't have to do this; I meant the missing quotation marks around the engraver names. :) (and now

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-10 Thread Carl Sorensen
On 7/10/11 4:41 PM, Neil Puttock n.putt...@gmail.com wrote: On 10 July 2011 23:34, Carl Sorensen c_soren...@byu.edu wrote: Ahh, thanks.  Fixed now: {  \new Voice \with  {    \consists Ambitus_engraver    \consists Mensural_ligature_engraver Heh, you didn't have to do this; I meant

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-09 Thread ColinPKCampbell
This has had a 48-hour countdown, and can be pushed and closed, please. Colin http://codereview.appspot.com/4667055/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-09 Thread Carl Sorensen
On 7/9/11 7:55 PM, colinpkcampb...@gmail.com colinpkcampb...@gmail.com wrote: This has had a 48-hour countdown, and can be pushed and closed, please. Issue 1715 issue marked fixed; Rietveld issue closed Commit: 4fd0de625eba203265241fca20c504724013f534 Thanks, Carl

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-07 Thread n . puttock
LGTM, though the regtest is still a bit messy. http://codereview.appspot.com/4667055/diff/14001/scm/define-grob-interfaces.scm File scm/define-grob-interfaces.scm (right): http://codereview.appspot.com/4667055/diff/14001/scm/define-grob-interfaces.scm#newcode128

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-07 Thread Carl . D . Sorensen
On 2011/07/07 16:12:08, Neil Puttock wrote: LGTM, though the regtest is still a bit messy. What do you consider messy about the regtest? scm/define-grob-interfaces.scm:128: A note-head that can become part of a ligature. note head Done Thanks, Carl

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-05 Thread n . puttock
On 2011/07/04 22:31:18, Carl wrote: OK, so I've added ligature-interface to NoteHead, and everything seems to work now. Erm, can I take back what I said yesterday? :) This looks really weird now, since it appears we're acknowledging ligature grobs. I think ligature-head-interface would be

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-04 Thread n . puttock
On 2011/07/04 01:54:40, Carl wrote: Instead of filtering out bad events, I chose to filter in only events with ligature interfaces. That's a lot of internal_has_interface () calls. :) You might as well create a new interface just for NoteHead (e.g., ligature-head-interface). Cheers, Neil

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-04 Thread Carl . D . Sorensen
On 2011/07/04 20:30:10, Neil Puttock wrote: On 2011/07/04 01:54:40, Carl wrote: Instead of filtering out bad events, I chose to filter in only events with ligature interfaces. That's a lot of internal_has_interface () calls. :) I wondered about that. But I think the first one that is

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-04 Thread Neil Puttock
On 4 July 2011 21:57, carl.d.soren...@gmail.com wrote: The original way you suggested was to have two internal_has_interface () calls; this one only adds one. But for the invalid heads, all three calls would be made (and of course, for a NoteHead itself only the first interface check will

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-04 Thread Carl . D . Sorensen
On 2011/07/04 21:22:58, Neil Puttock wrote: On 4 July 2011 21:57, mailto:carl.d.soren...@gmail.com wrote: We already have the unused ligature-interface. nbsp;What if I just added ligature-interface to NoteHead? Sounds fine to me. OK, so I've added ligature-interface to NoteHead,

Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-03 Thread Carl . D . Sorensen
Reviewers: , Message: Here is a proposed patch for fixing issue 1715. It works by checking for event-cause before acknowledging a notehead, thus ignoring AmbitusNoteHeads Description: Fix segfault with ambitus and ligature (Issue 1715) Check for an event-cause before acknowledging note_head.

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-03 Thread n . puttock
Hi Carl, This LGTM, though the regtest could do with a bit of tidying. A more correct approach would be to filter interfaces since the Ligature_engraver also potentially acknowledges PitchTrillGroup. It's probably not worth the bother though: it does have an event-cause and is ignored by the

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-03 Thread Carl . D . Sorensen
On 2011/07/03 21:25:58, Neil Puttock wrote: Hi Carl, This LGTM, though the regtest could do with a bit of tidying. Yeah, I thought of that, but I was too lazy. I just took what was in the tracker. OK, I'll trim it up some more. A more correct approach would be to filter interfaces

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-03 Thread n . puttock
On 2011/07/03 21:32:45, Carl wrote: On 2011/07/03 21:25:58, Neil Puttock wrote: Hi Carl, This LGTM, though the regtest could do with a bit of tidying. Yeah, I thought of that, but I was too lazy. I just took what was in the tracker. OK, I'll trim it up some more. A more correct

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-03 Thread n . puttock
On 2011/07/03 21:39:16, Neil Puttock wrote: On 2011/07/03 21:32:45, Carl wrote: On 2011/07/03 21:25:58, Neil Puttock wrote: Hi Carl, This LGTM, though the regtest could do with a bit of tidying. Yeah, I thought of that, but I was too lazy. I just took what was in the tracker. OK,

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-03 Thread pkx166h
Passes make and reg test James http://codereview.appspot.com/4667055/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fix segfault with ambitus and ligature (Issue 1715) (issue4667055)

2011-07-03 Thread Carl . D . Sorensen
Instead of filtering out bad events, I chose to filter in only events with ligature interfaces. I'm not sure my line-breaking is correct on the internal_has_interface () calls, but we'll soon have fixcc.py working, right? :) If it isn't right, please help me know how to fix it. I also trimmed