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

Issue 337870043: Charles Winston's GSoC code: Chord Semantics

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 1 month ago by chazwins6
Modified:
1 month ago
Reviewers:
pkx166h, pwm, lilypond-pkx
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Charles Winston's GSoC code: Chord Semantics

Patch Set 1 #

Patch Set 2 : Fixing errors with chord-semantics #

Patch Set 3 : Fixing errors in make doc. #

Total comments: 31
Unified diffs Side-by-side diffs Delta from patch set Stats (+658 lines, -114 lines) Patch
M Documentation/notation/chords.itely View 1 2 3 chunks +33 lines, -9 lines 1 comment Download
M input/regression/chord-name-exceptions.ly View 1 chunk +20 lines, -7 lines 2 comments Download
A input/regression/chord-semantics-additions.ly View 1 2 1 chunk +14 lines, -0 lines 0 comments Download
A input/regression/chord-semantics-alterations.ly View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
A input/regression/chord-semantics-basic.ly View 1 2 1 chunk +12 lines, -0 lines 1 comment Download
A input/regression/chord-semantics-bass.ly View 1 2 1 chunk +12 lines, -0 lines 1 comment Download
A input/regression/chord-semantics-extensions.ly View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
A input/regression/chord-semantics-inversions.ly View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
A input/regression/chord-semantics-lowercase-root.ly View 1 2 1 chunk +16 lines, -0 lines 1 comment Download
A input/regression/chord-semantics-name-exceptions.ly View 1 2 1 chunk +28 lines, -0 lines 1 comment Download
A input/regression/chord-semantics-power-chord.ly View 1 2 1 chunk +14 lines, -0 lines 1 comment Download
A input/regression/chord-semantics-removals.ly View 1 2 1 chunk +12 lines, -0 lines 0 comments Download
A input/regression/chord-semantics-sus.ly View 1 2 1 chunk +11 lines, -0 lines 0 comments Download
M lily/chord-name-engraver.cc View 6 chunks +31 lines, -1 line 0 comments Download
M ly/chord-modifiers-init.ly View 1 chunk +2 lines, -0 lines 0 comments Download
M ly/engraver-init.ly View 1 chunk +2 lines, -0 lines 0 comments Download
M scm/chord-entry.scm View 1 2 7 chunks +246 lines, -90 lines 11 comments Download
M scm/chord-ignatzek-names.scm View 4 chunks +118 lines, -4 lines 7 comments Download
M scm/chord-name.scm View 1 2 1 chunk +38 lines, -0 lines 5 comments Download
M scm/define-context-properties.scm View 4 chunks +7 lines, -2 lines 0 comments Download
M scm/define-event-classes.scm View 1 chunk +2 lines, -1 line 0 comments Download
M scm/define-music-properties.scm View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M scm/define-music-types.scm View 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 10
pkx166h
Hello, This fails a make check Processing `./fd/lily-2f531b63.ly' Parsing... Renaming input to: `/home/james/lilypond-git/input/regression/predefined-fretboards.ly' warning: cannot ...
1 year, 1 month ago (2017-11-04 12:29:51 UTC) #1
pkx166h
Charles, I just wanted to make sure you saw my last reply about your patch ...
1 year ago (2017-11-23 14:06:37 UTC) #2
chazwins6
Fixing errors with chord-semantics
11 months, 2 weeks ago (2017-12-29 22:38:54 UTC) #3
chazwins6
On 2017/12/29 22:38:54, chazwins6 wrote: > Fixing errors with chord-semantics This should be working now. ...
11 months, 2 weeks ago (2017-12-29 22:40:33 UTC) #4
pkx166h
Hello Charles, Thanks for your patch however, this fails make check. It fails on a ...
11 months, 2 weeks ago (2017-12-30 18:10:53 UTC) #5
chazwins6
Fixing errors in make doc.
4 months ago (2018-08-11 14:59:10 UTC) #6
lilypond-pkx
On 2018/08/11 14:59:10, chazwins6 wrote: > Fixing errors in make doc. Fails make check input/regression/chord-names-languages.ly' ...
4 months ago (2018-08-12 13:54:08 UTC) #7
chazwins6
On 2018/08/12 13:54:08, lilypond-pkx wrote: > On 2018/08/11 14:59:10, chazwins6 wrote: > > Fixing errors ...
4 months ago (2018-08-12 15:56:51 UTC) #8
pwm
Hi Charles, I was thinking about working on MusicXML export for chord symbols, and wondered ...
1 month ago (2018-11-10 05:44:23 UTC) #9
pwm
1 month ago (2018-11-10 19:44:47 UTC) #10
Hi Charles, Today I built and ran 'make check' with your patch applied to
current master.  I was able to get it to pass 'make check' by making the
following two changes.

1. In `chord-entry.scm` line 267, remove `(write-me "base3: " bass)`.

2. In that same file, line 100, remove the parens to change `(chord-semantics)`
to just `chord-semantics`.

The first change fixed this error (but note the type check warning):

input/regression/chord-names-languages.ly'

~~~
Parsing...
Renaming input to:
`/home/james/lilypond-git/input/regression/chord-names-languages.ly'
warning: type check for `bass' failed; value `(#t . #t)' must be of type
`boolean'
/home/james/lilypond-git/build/out/share/lilypond/current/scm/chord-entry.scm:267:59:
In procedure memoization in expression (if ba
ss (write-me "base3: " bass) ...):
/home/james/lilypond-git/build/out/share/lilypond/current/scm/chord-entry.scm:267:59:
In file "/home/james/lilypond-git/build/out/s
hare/lilypond/current/scm/chord-entry.scm", line 266: Missing or extra
expression in (if bass (write-me "base3: " bass) (list (make
-note-ev bass (quote bass) #t)) (quote ())).
~~~

And the second change fixed this error:

input/regression/chord-name-entry.ly

~~~
$ /home/dev/lilypond-git/build/out/bin/lilypond
input/regression/chord-name-entry.ly
GNU LilyPond 2.21.0
Processing `input/regression/chord-name-entry.ly'
Parsing.../home/dev/lilypond-git/build/out/share/lilypond/current/scm/chord-entry.scm:100:35:
In expression (chord-semantics):
/home/dev/lilypond-git/build/out/share/lilypond/current/scm/chord-entry.scm:100:35:
Wrong type to apply: ((modifier . #f) (root . #<Pitch c' >) (extension . 7)
(additions) (removals) (bass . #f))
~~~

So if you have a chance to upload a new patch set with those two changes, that
should get things moving forward with the code review process.

Cheers,
-Paul
Sign in to reply to this message.

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