Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(318)

Issue 3743043: [Patch] make post-event music functions direction-aware (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 6 months ago by Valentin Villenave
Modified:
8 years, 6 months ago
Reviewers:
carl.d.sorensen, Neil Puttock, Carl
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Add direction-awareness ability to post-event music functions.

Patch Set 1 #

Total comments: 2

Patch Set 2 : Update version number #

Patch Set 3 : set direction directly at parsing level #

Patch Set 4 : Add check. #

Patch Set 5 : More compact code #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -1 line) Patch
A input/regression/music-function-post-event.ly View 1 1 chunk +22 lines, -0 lines 0 comments Download
M lily/parser.yy View 1 2 3 4 1 chunk +5 lines, -1 line 0 comments Download

Messages

Total messages: 19
Valentin Villenave
Greetings everybody, here's a patch that Neil suggested to me (if you remember my earlier ...
8 years, 6 months ago (2010-12-20 15:08:32 UTC) #1
Carl
This seems like a cool idea! Could it be made more simple by just letting ...
8 years, 6 months ago (2010-12-20 15:31:43 UTC) #2
Valentin Villenave
On 2010/12/20 15:31:43, Carl wrote: > This seems like a cool idea! Thanks! As said ...
8 years, 6 months ago (2010-12-20 16:01:46 UTC) #3
Carl
On 2010/12/20 16:01:46, Valentin Villenave wrote: > On 2010/12/20 15:31:43, Carl wrote: > > This ...
8 years, 6 months ago (2010-12-20 16:22:30 UTC) #4
Valentin Villenave
On 2010/12/20 16:22:30, Carl wrote: > IMO, go ahead and set the neutral direction. If ...
8 years, 6 months ago (2010-12-20 21:47:45 UTC) #5
Neil Puttock
Hi Valentin, It's great to see you tackle this. The code looks pretty good, but ...
8 years, 6 months ago (2010-12-20 23:18:37 UTC) #6
Valentin Villenave
On 2010/12/20 23:18:37, Neil Puttock wrote: > The code looks pretty good, but I agree ...
8 years, 6 months ago (2010-12-20 23:22:22 UTC) #7
Neil Puttock
On 2010/12/20 23:22:22, Valentin Villenave wrote: > I've been looking for a way of doing ...
8 years, 6 months ago (2010-12-20 23:27:58 UTC) #8
Valentin Villenave
On 2010/12/20 23:18:37, Neil Puttock wrote: > Simply set 'direction inside the parser > rule ...
8 years, 6 months ago (2010-12-21 14:19:56 UTC) #9
Valentin Villenave
On 2010/12/20 16:22:30, Carl wrote: > IMO, go ahead and set the neutral direction. If ...
8 years, 6 months ago (2010-12-21 20:45:42 UTC) #10
Carl
On 2010/12/21 20:45:42, Valentin Villenave wrote: > On 2010/12/20 16:22:30, Carl wrote: > > actually, ...
8 years, 6 months ago (2010-12-21 21:24:52 UTC) #11
Valentin Villenave
On 2010/12/21 21:24:52, Carl wrote: > I can't understand why you don't want to push ...
8 years, 6 months ago (2010-12-21 22:12:46 UTC) #12
Neil Puttock
On 2010/12/21 14:19:56, Valentin Villenave wrote: > something like that? (see new patch set) Almost ...
8 years, 6 months ago (2010-12-21 22:21:38 UTC) #13
Neil Puttock
On 2010/12/21 22:12:46, Valentin Villenave wrote: > It does, since the 'direction property is never ...
8 years, 6 months ago (2010-12-21 22:27:43 UTC) #14
Valentin Villenave
On 2010/12/21 22:21:38, Neil Puttock wrote: > Almost there, but you're still setting 'direction unconditionally ...
8 years, 6 months ago (2010-12-21 22:34:23 UTC) #15
Valentin Villenave
On 2010/12/21 22:27:43, Neil Puttock wrote: > Use ly:dir? instead of (not (null? ...). Duh. ...
8 years, 6 months ago (2010-12-21 22:39:54 UTC) #16
Neil Puttock
On 2010/12/21 22:34:23, Valentin Villenave wrote: > Done (new patch). However, I can't really understand ...
8 years, 6 months ago (2010-12-21 22:45:53 UTC) #17
Valentin Villenave
On 2010/12/21 22:45:53, Neil Puttock wrote: > CENTER = 0, so is always false. Oh, ...
8 years, 6 months ago (2010-12-21 22:56:05 UTC) #18
Neil Puttock
8 years, 6 months ago (2010-12-21 23:15:35 UTC) #19
On 2010/12/21 22:56:05, Valentin Villenave wrote:

> Oh, I didn't know 0 was false, in Guile you can have something like:
> 
> guile> (define a 0)
> guile> (if a (display "true\n"))
> true
> guile>

Yep, Scheme is strict about #f: it's not equal to any other object.
 
> Do you think I can push this patch now? (Not sure where to document it,
though.
> Probably somewhere in the Extending manual.)

I don't think you need to copy the function evaluation result into a new SCM
object, but apart from that it LGTM.

Cheers,
Neil
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b