LilyIssues / Issues / #2024 Patch: Let #{ ... #} pass its $ handling to environment cloning

Google Importer - 2011-11-06

Originally posted by: dak@gnu.org

Ok, this is the first sketch how I want to let $ take over the job of ly:export and have it handled automagically in #{ ... #} by environment juggling.

Should be fully functional, but missing docs and python/convertrules.py, so several regtests will fail in predictable, easy to fix ways, with reasonably useful error messages.

You'll notice that the patch removes quite more lines than it adds.

Labels: -Patch-new Patch-needs_work
Owner: dak@gnu.org

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-07

Originally posted by: dak@gnu.org

Issue 2024: Let #{ ... #} pass its $ handling to environment cloning

Consists of the patches:

Adapt docs to new $ and #{ ... #} behavior

Run scripts/auxiliar/update-with-convert-ly.sh

Let #{ ... #} pass its $ handling to environment cloning

Includes convertrules.py rules

Permit ly:parser-clone to receive an environment

lexer.ll: add $ for immediate export.

Purpose is to make $ generally available rather than to have it just
in #{ ... #}, let it completely replace the need for ly:export, and to
let #{ ... #} import its lexical environment into its embedded Scheme
expressions via # or $.

Unfortunately, you will likely have to bump PATCHLEVEL in VERSION
manually for testing, as dev/staging is still locked at 2.15.17 which
is too small after running convert-ly.

http://codereview.appspot.com/5341049

Labels: Patch-new

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-08

Originally posted by: dak@gnu.org

Issue 2024: Let #{ ... #} pass its $ handling to environment cloning

Consists of the patches:

Adapt docs to new $ and #{ ... #} behavior

Run scripts/auxiliar/update-with-convert-ly.sh

Let #{ ... #} pass its $ handling to environment cloning

Includes convertrules.py rules

Permit ly:parser-clone to receive an environment

lexer.ll: add $ for immediate export.

Purpose is to make $ generally available rather than to have it just
in #{ ... #}, let it completely replace the need for ly:export, and to
let #{ ... #} import its lexical environment into its embedded Scheme
expressions via # or $.

Unfortunately, you will likely have to bump PATCHLEVEL in VERSION
manually for testing, as dev/staging is still locked at 2.15.17 which
is too small after running convert-ly.

http://codereview.appspot.com/5341049

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-08

Originally posted by: dak@gnu.org

I am rather serious about this. The last upload removes all uses of ly:export _via_ convertrules. Automatically, even though this involves non-trivial editing of Scheme code and embedded Lilypond. In the process, it slightly changes the call syntax of, I think, 5 Scheme functions which, for no imaginable reason whatsoever, are called in Scheme rather than as music functions, but return a packed identifier that is only ever useful used in Lilypond syntax.

This is not a question of GLISS but rather of sanity. This patch does not address the sanity angle; it merely requires the user to use $ when using those functions rather than #. As a side benefit, their return values are now useful when called from Scheme, but I doubt anybody actually wants to do that. They should really be turned into music functions. All manual uses of ly:export are also patched away, and definition and use of a similarly weird function in the baerenreiter regtest are automagically detected and changed.

After this gets accepted, I will remove ly:export altogether, and split the phases of reading and evaluation # expressions, getting rid of their synchronization problems.

You better review this before it passes a countdown. As I said, I am dead serious about it. It straightens out a lot of wrinkles and inconsistencies in the interaction of Lilypond, Scheme, and #{ ... #} code blocks, with an interplay that is logical and understandable rather than a series of unrelated hacks.

You'll need to handbump VERSION for reviewing at the moment, since convert-ly has to pick a not-yet existing release number. Let's hope that we get the official version bump soon.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-08

Originally posted by: gra...@percival-music.ca

sigh. ok, I'll reformat my computer and stick 10.04 back on it so we can have a release. I'll try to do my benchmarking for the phd on my laptop, but if that fails I'll install 11.10 again. or maybe I can set up a triple-boot system.

2.15.17 hopefully tomorrow.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-08

Originally posted by: pkx1...@gmail.com

This didn't pass 'make'. Not sure if it is JUST the version number (not sure how to test this patch otherwise) or if there is an additional problem. I can also do a full make doc when it passes make.

--snip--

error: program too old: 2.15.17 (file requires: 2.15.18)
/home/jlowe/lilypond-git/ly/engraver-init.ly:19:9: error: syntax error, unexpected \version-error
\version
"2.15.18"
]]
[/home/jlowe/lilypond-git/ly/generate-documentation.ly
[/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/documentation-lib.scm]
[/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/lily-sort.scm]
[/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-functions.scm]
[/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-translation.scm]
[/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-music.scm]
[/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-type-predicates.scm]
[/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-identifiers.scm]
[/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-context-mods.scm]
[/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-backend.scm]
[/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-markup.scm]
Writing "internals.texi"...
]]
programming error: Parsed object should be dead: static scm_unused_struct* Prob::mark_smob(scm_unused_struct*)
continuing, cross fingers
programming error: Parsed object should be dead: static scm_unused_struct* Context_mod::mark_smob(scm_unused_struct*)
continuing, cross fingers
programming error: Parsed object should be dead: static scm_unused_struct* Context_def::mark_smob(scm_unused_struct*)
continuing, cross fingers
fatal error: failed files: "/home/jlowe/lilypond-git/ly/generate-documentation"
make[1]: *** [out/internals.texi] Error 1
make[1]: Leaving directory `/home/jlowe/lilypond-git/build/Documentation'
make: *** [all] Error 2
jlowe@jlowe-lilybuntu2:~/lilypond-git/build$

--snip--

James

Labels: -Patch-new Patch-needs_work
Status: Started

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-08

Originally posted by: gra...@percival-music.ca

as the message says, it's the version number.

Hold on a bit, I'll download 10.04, install it, and release 2.15.17 tomorrow. Let's leave this as new.

Labels: -Patch-needs_work Patch-new

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-08

Originally posted by: dak@gnu.org

For testing, James, edit VERSION and set PATCHLEVEL to 18. I was half of a mind to do this in the Rietveld issue myself, but decided that it was impolite to have a munged VERSION flying around in the open. I definitely won't commit any such thing.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-08

Originally posted by: dak@gnu.org

Issue 2024: Let #{ ... #} pass its $ handling to environment cloning

Consists of the patches:

Adapt docs to new $ and #{ ... #} behavior

Run scripts/auxiliar/update-with-convert-ly.sh

Let #{ ... #} pass its $ handling to environment cloning

Includes convertrules.py rules

Permit ly:parser-clone to receive an environment

lexer.ll: add $ for immediate export.

Purpose is to make $ generally available rather than to have it just
in #{ ... #}, let it completely replace the need for ly:export, and to
let #{ ... #} import its lexical environment into its embedded Scheme
expressions via # or $.

Unfortunately, you will likely have to bump PATCHLEVEL in VERSION
manually for testing, as dev/staging is still locked at 2.15.17 which
is too small after running convert-ly.

http://codereview.appspot.com/5341049

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-08

Originally posted by: pkx1...@gmail.com

Thanks everyone, I changed VERSION and it passed make, there no pictoral reg test diffs, but as this seems to be quite a fundamental change to the code, I have attached the output of the test results anyway as there is a lot of text that might be important.

@graham, hopefully this saves you the bother now and you can do something more constructive.

James

Labels: -Patch-new Patch-review

test-results.zip

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-08

Originally posted by: reinhold...@gmail.com

Hmm, most of the diffs is just due to different snippet hashes, which should actually not count as a diff any more (apparently my fix doesn't work in all cases...).

However, there is one relevant issue:
input/regression/display-lily-tests.log:
@@ -7,6 +7,9 @@
out = { \grace { { s1*0( \override Flag #'stroke-style = #"grace"
} c8 { \revert Flag #'stroke-style
s1*0) } } d2 }
+Test 99 unequal: .
+in = $(set-time-signature 5 8 '(3 2))
+out = #(set-time-signature 5 8 '(3 2))
Interpreting music... Test 105 unequal: NOT A BUG.
in = \relative c' { c b }
out = { c'4 b }

This shows that display-lily-music does NOT properly create scheme code yet with the $ syntax, but instead still creates expressions with the old # syntax...

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-08

Originally posted by: dak @gnu.org

Uh, the # syntax is not old, it is different in meaning (namely
auto-exporting). I should definitely hope that display-lily-music would
not create $ gratuitiously.

Since set-time-signature does no longer export itself, display-lily-music indeed produces wrong syntax (I think it is hardwired what it produces for a given output, and not related to what input will in reality produce the output).

I don't think I'll bother fixing this; instead I'll try tomorrow to create music function versions of the five ugly-syntax functions, and then change display-lily-music accordingly. It's probably saner to do this in one iteration of convert-ly instead of generating an intermediate state. This will still be done for automated conversion (like in Baerenreiter regtest) since conversion into music functions can't be automated: you need to figure out proper argument list types.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-08

Originally posted by: ColinPKC...@gmail.com

(No comment was entered for this change.)

Labels: -Patch-review Patch-countdown

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-08

Originally posted by: dak @gnu.org

@Reinhold: had a change of mind: I don't want to disturb this countdown, and the syntax when changing those 5 commands (actually 4, since empty-music is unused and rather unnecessary) is a separate discussion. There is also no saved effort in convert-ly rules since the semiautomatic removal of ly:export is necessary anyway. So I just ran convert-ly on */*.scm with options -e -n -f 2.15.17, and there was a one-character change in the formatter for set-time-signature, and nothing else. Uploaded the corrected patch. A single-character change in a string should not disturb the countdown, and it is obvious (TM) that it only affects the reported problem.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-09

Originally posted by: dak @gnu.org

(No comment was entered for this change.)

Blocking: 2032

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-09

Originally posted by: dak @gnu.org

(No comment was entered for this change.)

Blocking: 2033

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-10

Originally posted by: dak@gnu.org

I've pushed to dev/staging as [rcd229915fc873fdb6fd0125827452cb0ba0067a7]. Since it contains a merge commit (in order to have the convert-ly run separate from the actual change without hurting bisection), getting it into master needs some attention. Attention we need anyway for the translation merges, so let's see how we fare.

Once it has hit master, we'll need another version bump.

Labels: -Patch-countdown

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-10

Originally posted by: dak @gnu.org

(No comment was entered for this change.)

Status: Fixed

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-10

Originally posted by: dak @gnu.org

(No comment was entered for this change.)

Blocking: -2033

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-10

Originally posted by: dak @gnu.org

(No comment was entered for this change.)

Blocking: -2032

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-13

Originally posted by: PhilEHol...@googlemail.com

David - could you confirm that this commitish is just intended to be for the documentation change?

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-13

Originally posted by: dak@gnu.org

Negative. The commit changes the code behavior, adds convertrules, and executes them in order to keep regtests going.

Maybe you overlooked the code behavior change in the extensive Rietveld review, but the substructure of the commit (done as merge commit) makes the actual code change separate available as [rfecc5999e224304e9d54e48bc7a92cdbb123cd35]

git log --graph displays this as

* commit [rcd229915fc873fdb6fd0125827452cb0ba0067a7]
| Author: David Kastrup <dak@gnu.org>
| Date:   Mon Nov 7 15:22:24 2011 +0100
|
|     Adapt docs to new $ and #{ ... #} behavior
|
*   commit [r2a07e6101abd393b88f22309dcda0c7ed032d85c]
|\  Merge: ccebc52 ccc4855
| | Author: David Kastrup <dak@gnu.org>
| | Date:   Thu Nov 10 15:41:41 2011 +0100
| |
| |     Let #{ ... #} pass its $ handling to environment cloning and run convert
| |
| * commit [rccc485525e63f68623bce1c3ca7a2d4fb6e94499]
| | Author: David Kastrup <dak@gnu.org>
| | Date:   Tue Nov 8 15:21:49 2011 +0100
| |
| |     Run update-with-convert-ly, and run convert-ly manually on scm/*.scm
| |
| * commit [rfecc5999e224304e9d54e48bc7a92cdbb123cd35]
|/ Author: David Kastrup <dak@gnu.org>
|   Date:   Sun Nov 6 19:15:27 2011 +0100
|
|       Let #{ ... #} pass its $ handling to environment cloning
|
|       Includes convertrules.py rules for dealing with #{ ... #} and for
|       removing uses of ly:export
|

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Google Importer - 2011-11-18

Originally posted by: PhilEHol...@googlemail.com

I just did what we're supposed to do - check the commit in Git - and all it showed was doc changes, which seemed strange. I've now verified against the other commitish you've supplied.

Status: Verified

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Patch: Let #{ ... #} pass its $ handling to environment cloning

Issue Tracker for LilyPond

Searches

Help

#2024 Patch: Let #{ ... #} pass its $ handling to environment cloning

Discussion