Menu
▾
▴
1 Attachments
#1367 Enhancement: NoteNames context should handle any note names language
Originally created by: anonymous
Originally created by: v.villenave
Originally owned by:* v.villenave
As Mats noticed a few years ago, default (Nederlands) note names are hardcoded in the Note_names_engraver: in the following example, the notes are entered in Spanish but printed in Dutch.
http://lists.gnu.org/archive/html/lilypond-user/2007-03/msg00021.html
\version "2.13.37"
\include "espanol.ly"
notes = \relative do' \{ do re mi do \}
\new Staff <<
\notes
\context NoteNames \{ \notes \}
<<
It would be more elegant to use the pitchnames variable (just like \displayLilyMusic does) so we could print note names in any language.
--
Valentin Villenave - 2019-02-02
OK, here’s a (I hope) much better patch that not only deals with the Note_name_engraver, but also aims to make all of the note-/pitch-naming functions more consistent and flexible (including in chord names). Included is a regtest showing off the new capabilities offered by this engraver, including quite a few user-settable properties and Scheme functions. (Documentation is not included yet though, as I’d like to first settle on an agreed feature set, property names and general mechanism.)
Originally posted by: v.villenave
Of course, there are workarounds: I posted a very dumb one on -user
http://lists.gnu.org/archive/html/lilypond-user/2010-10/msg00687.html
However, it would be interesting to improve the engraver itself (I suspect this task may be suitable for a Frog?).
Originally posted by: colingh...@gmail.com
User Jean-Charles Malahieude repeated this enhancement request here:
http://lists.gnu.org/archive/html/bug-lilypond/2012-06/msg00150.html
Originally posted by: fanzi...@gmail.com
I'm trying to work on this issue.
In order to add a \pitchnames command, the property of \pitchnames is need in note_name_engraver. Is it possible to read it with a get_property()?
Besides, where should I modify to add a new lilypond command? the lilypond parser?
Thanks!
p.s. I don't know if it's right to asking such questions here in the issue comment, if not, could you please tell me where to ask (or to read). Thanks very much:)
Originally posted by: fanzi...@gmail.com
note-name-engraver: add user-defined note names support
Property "noteNameList" added in order to display
note names in languages other than Deutsch.
Example:
\version "2.13.37"
\include "espanol.ly"
notes = \relative do' { do re mi do }
\new Staff <<
\notes
\context NoteNames {
\set noteNameList = #'("la" "si" "do" "re" "mi" "fa" "sol")
\notes }
>>
http://codereview.appspot.com/8593046
Originally posted by: janek.li...@gmail.com
Fan Ziye wrote:
> In order to add a \pitchnames command, the property of \pitchnames
> is need in note_name_engraver. Is it possible to read it with a get_property()?
It seems so.
> Besides, where should I modify to add a new lilypond command? the lilypond parser?
Usually not. More likely somewhere like ly/music-functions-init.ly (just try searching where other lily commands are defined).
But i don't think you need to define any new comment in this issue.
> p.s. I don't know if it's right to asking such questions here
> in the issue comment, if not, could you please tell me where
> to ask (or to read). Thanks very much:)
Rule of thumb:
- the tracker is a place to mostly discuss the design of the patch (what the results should be),
- rietveld tracker is for discussing actual code after it was written,
- general questions regarding Lily codebase are usually best discussed on -devel.
BTW, did you use git-cl to upload your patch or was the comment #4 written manually by you? Because one thing was missing - it should be set to Patch-new (which i'm doing now).
Labels: Patch-new
Originally posted by: pkx1...@gmail.com
Passes Make, make check and a full make doc.
Reg test diffs attached
https://www.yousendit.com/download/UVJneFlUQ0NuSlExWjhUQw
Labels: -Patch-new Patch-review
Owner: janek.li...@gmail.com
Originally posted by: janek.li...@gmail.com
PS I wrote "But i don't think you need to define any new comment in this issue." - i meant "any new command", of course.
Originally posted by: fanzi...@gmail.com
> It seems so.
>
> Usually not. More likely somewhere like ly/music-functions-init.ly (just try searching where other lily commands are defined).
> But i don't think you need to define any new comment in this issue.
>
> Rule of thumb:
> - the tracker is a place to mostly discuss the design of the patch (what the results should be),
> - rietveld tracker is for discussing actual code after it was written,
> - general questions regarding Lily codebase are usually best discussed on -devel.
OK, very useful to me.Thanks.
> BTW, did you use git-cl to upload your patch or was the comment #4 written manually by you? Because one thing was missing - it should be set to Patch-new (which i'm doing now).
Yes, I wrote comment #4 manually. It was the first time for me to use such tools (git and Rietveld) in real world, really exciting!
I'm trying to get pitch's name from variable 'pitchnames'. Is it necessary to display alternative labels? I think it should be display in '#' and 'b' format.
The example below will produce this result: do re mi re#
Example:
\version "2.13.37"
\include "espanol.ly"
notes = \relative do' { do re mi red }
\new Staff <<
\notes
\context NoteNames {
\notes }
>>
Originally posted by: pkx1...@gmail.com
Changing patch to New for now while Fanziye gets used to our 'work flow'
Labels: -Patch-review Patch-new
Originally posted by: pkx1...@gmail.com
I think this needs work
Passes make and make check and full make doc but reg test diffs are here:
https://www.yousendit.com/download/UVJnbGtNNDJUWUFzeHNUQw
Labels: -Patch-new Patch-needs_work
Originally posted by: janek.li...@gmail.com
2013/4/24 Fan Ziye wrote:
> Janek wrote:
>> BTW, did you use git-cl to upload your patch or was the comment #4 written
>> manually by you? Because one thing was missing - it should be set to
>> Patch-new (which i'm doing now).
>
> Yes, I wrote comment #4 manually. It was the first time for me to use such
> tools (git and Rietveld) in real world, really exciting!
:)
As James explained in the Rietveld issue, git-cl is the easiest way to upload a patch. I'll write a more detailed explanation in the Rietveld issue.
> I'm trying to get pitch's name from variable 'pitchnames'. Is it necessary
> to display alternative labels? I think it should be display in '#' and 'b'
> format.
> The example below will produce this result: do re mi re#
>
> Example:
> \version "2.13.37"
> \include "espanol.ly"
> notes = \relative do' { do re mi red }
>
> \new Staff <<
> \notes
> \context NoteNames {
> \notes }
Hmm. I'm not sure what exactly do you mean, but if i understand correctly, i don't think it should be done this way.
In other words, i think that if the user input is "do re mi red" then the NoteNames should be "do re mi red".
Originally posted by: janek.li...@gmail.com
PS there is one important thing that you may not know about: handling regression tests.
You may skim over the relevant section in the CG if you haven't already:
http://www.lilypond.org/doc/v2.17/Documentation/contributor/regression-tests
Some more info: when a google tracker issue has gets a label "Patch-new", Patchy (a script that helps us automate patch handling) goes to corresponding Rietveld issue, downloads the patch from there and runs regression tests on it. James Lowe (pkx166h@gmail.com) is the person who usually takes care of Patchy.
If the regression tests show differences, James uploads the results somewhere and posts a link in a comment (see comment #10). You are supposed to download these results and examine the differences (open file index.html that's inside the archive). If there are no bad changes, James will set the tracker issue to "Patch-review" and this will cause other developers to look at the corresponding Rietveld issue.
If there are problems (and from what i see there are some problems with current version of the patch), James will set it to "Patch-needs_work". In that case you should find what's wrong, fix it and post new patchset (other developers will usually don't look at issues that "need_work", assuming that the contributor is working on a fix. Of course, you can always ask questions if you're stuck).
As usual, ask if something's unclear!
Originally posted by: joao...@gmail.com
Hi Janek,
in order to review Fan's patch I'm trying to apply his patch to my git workspace. I downloaded the code from RietveID, read some instructions in CG 3.4.7. But I'm still clueless about how to apply the patch. Have you some hint in this case?
Originally posted by: PhilEHol...@googlemail.com
It's a straightforward git command, normally - the best way of learning this is generally to google what you're trying to do. However, to save you this time
git am patchname
should do what you want.
Originally posted by: janek.li...@gmail.com
Actually, what you download from Rietveld is not a patchfile, but just a diff (it doesn't contain commit information like author - just the changes). You apply it with
git apply FILE
(and you'll probably want to commit that state, as applying a diff doesn't create any commits onn its own).
Originally posted by: dak@gnu.org
Correction: you apply it with
git apply --index
since otherwise added (and under some conditions deleted) files will escape git's notice.
Originally posted by: v.villenave
note-name-engraver: add user-defined note names support
Rebase onto master & add regtest
This is merely an adaptation from patch #8593046 posted
in April 2013 by Fan Ziye.
See https://codereview.appspot.com/8593046
and https://code.google.com/p/lilypond/issues/detail?id=1367
http://codereview.appspot.com/221710044
Labels: -Patch-needs_work Patch-new
Owner: v.villenave
Status: Started
Originally posted by: pkx1...@gmail.com
http://codereview.appspot.com/221710044
Originally posted by: pkx1...@gmail.com
Fails make check.
% ****************************************************************
% ly snippet:
% ****************************************************************
\sourcefilename "/tmp/lilypond-autobuild/input/regression/note-names-custom.ly"
\sourcefileline 0
\version "2.19.18"
\header {
texidoc = "The NoteNames context may be used
to print arbitrary user-defined names.
"
}
notes = \relative c' {
c d e c
}
<<
\new Staff \notes
\context NoteNames {
\set noteNameList = #'("la" "si" "do" "re" "mi" "fa" "sol")
\notes
}
>>
% ****************************************************************
% end ly snippet
---
Processing `./bc/lily-00c33939.ly'
Parsing...
Renaming input to: `/tmp/lilypond-autobuild/input/regression/note-names-custom.ly'
Interpreting music...
warning: cannot find property type-check for `noteNameList' (translation-type?). perhaps a typing error?
Writing timing to bc/lily-00c33939.profile...
programming error: Parsed object should be dead: #<Prob: Stream_event C++: Stream_event((value la si do re mi fa sol)
(symbol . noteNameList) (origin . #<location /tmp/lilypond-autobuild/input/regression/note-names-custom.ly:15:5>))((
class SetProperty StreamEvent)) >
continuing, cross fingers
Labels: -Patch-new Patch-needs_work
Originally posted by: v.villenave
Replace NoteNames with a Scheme engraver
http://codereview.appspot.com/221710044
Labels: -Patch-needs_work Patch-new
Originally posted by: pkx1...@gmail.com
Now it fails make:
--snip--
[/tmp/build-lilypond-autobuild/out/share/lilypond/current/ly/engraver-init.ly/tmp/build-lilypond-autobuild/out/sha
re/lilypond/current/scm/lily.scmBacktrace:
In unknown file:
?: 0* [lilypond-main ("/tmp/lilypond-autobuild/ly/generate-documentation")]
In /tmp/build-lilypond-autobuild/out/share/lilypond/current/scm/lily.scm:
967: 1* (let* ((failed #)) (if (ly:get-option #) (begin #)) ...)
967: 2* [lilypond-all ("/tmp/lilypond-autobuild/ly/generate-documentation")]
980: 3 (let* ((failed #) (separate-logs #) (ping-log #) ...) (gc) ...)
992: 4* [for-each #<procedure #f #> #]
In unknown file:
?: 5* [#<procedure #f (x)> "/tmp/lilypond-autobuild/ly/generate-documentation"]
In /tmp/build-lilypond-autobuild/out/share/lilypond/current/scm/lily.scm:
994: 6* (let* (# # #) (if separate-logs #) (if ping-log #) ...)
1005: 7* [lilypond-file #<procedure #f #> ...]
1040: 8 [catch ly-file-failed #<procedure #f ()> #<procedure #f (x . args)>]
In unknown file:
?: 9* [#<procedure #f ()>]
In /tmp/build-lilypond-autobuild/out/share/lilypond/current/scm/lily.scm:
1041: 10* [ly:parse-file "/tmp/lilypond-autobuild/ly/generate-documentation"]
In /tmp/lilypond-autobuild/ly/init.ly:
12: 11* [session-initialize #<procedure #f ()>]
In /tmp/build-lilypond-autobuild/out/share/lilypond/current/scm/lily.scm:
131: 12 (if (ly:undead? lilypond-declarations) (begin # #) (begin # # #))
142: 13 (begin (thunk) (set! lilypond-interfaces (filter # #)) ...)
143: 14* [#<procedure #f ()>]
In /tmp/lilypond-autobuild/ly/init.ly:
20: 15 [ly:parser-parse-string # "\\include \"declarations-init.ly\""]
/tmp/lilypond-autobuild/ly/init.ly:20:4: In procedure ly:parser-parse-string in expression (ly:parser-parse-string (
ly:parser-clone parser) "\\include \"declarations-init.ly\""):
/tmp/lilypond-autobuild/ly/init.ly:20:4: Unbound variable: Note_name_engraver
--sinp--
Labels: -Patch-new Patch-needs_work
Originally posted by: v.villenave
Add printOctaveNames property
http://codereview.appspot.com/221710044
Labels: -Patch-needs_work Patch-new
Originally posted by: v.villenave
Well yes, it’s gonna fail make for now since Scheme engravers don’t have translator-descriptions and the like, and therefore can’t be documented in the IR.
In other words, it’s blocked by issue 1375.
Originally posted by: pkx1...@gmail.com
Then this patch is not Patch new as I cannot test this if it is going to fail until 1375 has been applied.
Labels: -Patch-new Patch-needs_work
Diff: