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

Issue 7231072: \note-by-number supports flag-styles (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 3 months ago by thomasmorley65
Modified:
11 years, 2 months ago
Reviewers:
dak
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

\note-by-number supports flag-styles Issue 3104 For mensural and neomensural the flag is changed due to the note-head-style. Straight flags are possible using the new introduced flag-style-property

Patch Set 1 #

Total comments: 2

Patch Set 2 : Including David's suggestions #

Unified diffs Side-by-side diffs Delta from patch set Stats (+236 lines, -114 lines) Patch
M input/regression/markup-note.ly View 1 chunk +50 lines, -44 lines 0 comments Download
M input/regression/markup-note-styles.ly View 2 chunks +38 lines, -13 lines 0 comments Download
M scm/define-markup-commands.scm View 1 5 chunks +148 lines, -57 lines 0 comments Download

Messages

Total messages: 5
thomasmorley65
Please review. The regtests markup-note.ly and markup-note-styles.ly are modified. Changes are expected.
11 years, 3 months ago (2013-01-31 22:29:53 UTC) #1
dak
https://codereview.appspot.com/7231072/diff/1/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): https://codereview.appspot.com/7231072/diff/1/scm/define-markup-commands.scm#newcode3366 scm/define-markup-commands.scm:3366: (string-append "flags." I'd rather use (format #f (if ancient-flags? ...
11 years, 3 months ago (2013-02-02 16:11:19 UTC) #2
thomasmorley65
Including David's suggestions
11 years, 3 months ago (2013-02-03 14:02:05 UTC) #3
thomasmorley65
Including David's suggestions
11 years, 3 months ago (2013-02-03 14:05:13 UTC) #4
thomasmorley65
11 years, 3 months ago (2013-02-03 14:09:17 UTC) #5
Don't know why two identical patch-sets were uploaded

On 2013/02/02 16:11:19, dak wrote:
> https://codereview.appspot.com/7231072/diff/1/scm/define-markup-commands.scm
> File scm/define-markup-commands.scm (right):
> 
>
https://codereview.appspot.com/7231072/diff/1/scm/define-markup-commands.scm#...
> scm/define-markup-commands.scm:3366: (string-append "flags."
> I'd rather use
>   (format #f (if ancient-flags? "flags.mensural~a2~a"
>                                 "flags.~a~a")
>              (if (> dir 0) "u" "d") log)
> here.  It is somewhat more readable.

Changed.

> 
>
https://codereview.appspot.com/7231072/diff/1/scm/define-markup-commands.scm#...
> scm/define-markup-commands.scm:3387: (or (eq? flag-style 'default) (list?
> flag-style))
> Why (list? flag-style)?  Is that a tricky obfuscation of (null? flag-style)?

Rather bad thinking.
Changed.
Sign in to reply to this message.

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