WIP! Attempt at changing plugin loading structure #19
Conversation
This doesn't work yet, but I want to submit it for review. What I think is the right thing is how package.ily goes along with choosing the "backend"/target. I also think that FileExport in api.scm does the right thing. What does *not* work is making the export-lilypond function from within Humdrum.scm (so far this only touches the Humdrum file) available to api.scm. I really don't understand it because in my MWE it *did* work. (see #17 (comment))
|
I have to admit that I got no clue about modules, packages, and the best way to call things, either in scheme in general or in LP in particular. I'm very new to all of this, and I'm sure you guys will know how to expose this the best. Sorry, but I just don't have enough expertise and intuition to offer here :-) |
|
I don't have a good grasp on the OLL package mechanisms but I'll try to take a look (but likely not before this weekend). |
|
@PaulMorris my problem is very little about how OLL's package mechanism, but mainly about how Scheme modules are handled in the different constellations between LilyPond code/files and Scheme code/files. |
|
Hi @uliska , I had a chance to take a look. I'm seeing this: Is the problem the error "no code for module (oll-core internal music-tools)"? (Where this module is loaded at the top of lilypond-export/api.scm.) When I look in I tried checking out the previous commit in the lilypond-export repo b69b579 (Move I see the commit where |
No, the problem comes later. Sorry for not being explicit enough, but the music-tools module is still on the unmerged pull request openlilylib/oll-core#41 and its BTW Scheme modules starting with With that |
There was a problem hiding this comment.
Hi @uliska , Thanks for the details on what oll-core branch to use. Once I un-commented the two lines indicated below, I got no more errors and a success message in the console when compiling the export-example.ly. See also Guile 1.8 docs on creating modules and using modules.
| @@ -33,7 +33,7 @@ | |||
|
|
|||
| ;% TODO ties, slurs, grace notes | |||
|
|
|||
| (define-module (lilypond-export Humdrum)) | |||
| ;(define-module (lilypond-export Humdrum)) | |||
There was a problem hiding this comment.
You still need this define-module here.
| @@ -41,12 +41,28 @@ | |||
| (oll-core internal music-tools) | |||
| (lilypond-export lily) | |||
| (lilypond-export MusicXML) | |||
| (lilypond-export Humdrum) | |||
| ;(lilypond-export Humdrum) | |||
There was a problem hiding this comment.
And you still need to use the Humdrum module here, so the functions that are define-public in it can be used in this api module.
There was a problem hiding this comment.
@PaulMorris thank you for looking into this. Now that I see it I can't believe that I seem to have managed to miss that combination. But yet, it seems it works properly now, and I'll be able to remove the "WIP" from this PR's title ASAP (was completely away over the weekend and will need some time to catch up, though).
There was a problem hiding this comment.
Glad to help @uliska . I had another thought about this. So far only the humdrum module is exporting an 'export-lilypond' function, but when musicxml does as well (and eventually MEI, etc.), will we end up with naming collisions where the api module doesn't know which 'export-lilypond' function to call? I may be missing something, but it would be worth testing this out before merging. (Which may well have been your next step!)
There was a problem hiding this comment.
No, this is by design. Until now only the Humdrum module has been modified, and once that works I'll make all modules behave that way. Actually that's the requirement for plugging in a new exporter: providing an export-lilypond function. The actual idea is that load-exporter is configured to load the appropriate module and then takes the export-lilypond procedure from that module.
There was a problem hiding this comment.
See also Guile 1.8 docs on creating modules and using modules.
I had actually read this and still was confused, and I'd say the problem at hand isn't explained in a way that I would be able to really understand it.
The Guile 1.8 docs on Loading from file state that load-from-file does "Load filename and evaluate its contents in the top-level environment".
What is the "top-level environment" here? Wouldn't that imply the result of a define or define-public should be visible in the file that loads the module? =>
If api.scm' "loads" humdrum.scm and export-lilypond is defined in humdrum.scm I would expect the code in api.scm (after the loading) to see export-lilypond. Is there a way to make that work without having to explicitly define a module in humdrum.scm and use it in api.scm?
Is primitive-load anything different than load and load-from-file ir is the absence of the optional reader argument the only difference?
My goal would be to make it possible that a new exporter is added by only adding a new exporter file, without the need to update api.scm.
There was a problem hiding this comment.
Here is some code:
\version "2.19.82"
\include "oll-core/package.ily"
% I don't load the lilypond-export package here to show the import from a list
% force loading of lilypond-export_api_module - this is not needed, if the base package is already loaded
#(display (@ (lilypond-export api) moment->duration))
% this list shall be generated with a file walk through the export-module folder
exporters.MusicXML = #'exportMusicXML
exporters.Humdrum = #'exportHumdrum
% now fetch the
mods = #'()
#(for-each
(lambda (p)
; the (cdr p) may be replaced by mandatory const like 'lilypond-export
(let ((exporter (module-ref (resolve-module `(lilypond-export ,(car p))) (cdr p)) ))
; add this to the list of exporters
(set! mods `(,@mods ,exporter)) ; use (cons key exporter)
)) exporters)
% log found exporters with its file-suffix
#(for-each
(lambda (exporter)
; when the exporter is defined with a mandatory name the name is stored in an object-property
(ly:message "~A ~A" (procedure-name exporter) (object-property exporter 'file-suffix))
) mods)
So you can collect all files in folder, import a function lilypond-export and add it to an alist (or some other kind of store). That way you can add exporters by dropping them in a directory - as long as the module follows the mandatory rules.
When my schedule allows I will prepare some code fitting into this PR.
There was a problem hiding this comment.
I only have a vague idea after a first look, but there's one thing I'd definitely not want to do: produce a list of all available exporter functions. These functions are already large and will become huge when they will become more comprehensive, so we should really only load one such function.
Therefore we may produce a list of available module names but the actual module loading and function parsing should occur only once.
I think what I'm missing is somehow hidden in the (let ((exporter (module-ref (resolve-module line?
There was a problem hiding this comment.
If I'm not mistaken this approach doesn't help either with my problem. resolve-module allows loading a module by its name, but this seems to require the module being "used" too - so there's no real advantage over using load-from-path.
|
Am 05.11.18 um 17:49 schrieb Urs Liska:
I think what I'm missing is somehow hidden in the |(let ((exporter
(module-ref (resolve-module | line?
yes, the module is implicitly loaded there - it is like somewhere
use-modules is called and then the desired object/function/whatever is
imported. It is loaded once like any scheme-module. If we want to
prevent the loading at all we can only rely on the filename of the
module. I'd like to be able to retrieve a list of exporter objects to be
able to get informations within some object-properties.
And the exporter is just one lambda so I don't think having them loaded
all - will there ever be more than 5 exporters? - will not lead to any
RAM-problems.
Though complex exporters might need some time to load? I doubt it
significant, but if we want to load the exporter when it is actually
used we can just use the module-ref thing in a loader-function and
provide the available exporters in a symbol-list.
understandable?
|
|
o yes, there is one thing: load-from-path loads the file into the
current context. resolve-module loads the module into the guile-VM,
retrieves the one object from the module, but leaves the current context
untouched. If you retrieve the lilypond-export object from an exporter
module you can store it in a list for later usage. If the object in an
exporter is called lilypond-export it does not conflict with the other
plugins because the lambda-object is not called by this name.
Of course this resolve-module thing can happen within a loader-function
to only load the needed exporters.
Am 05.11.18 um 18:01 schrieb Urs Liska:
… ***@***.**** commented on this pull request.
------------------------------------------------------------------------
In api.scm
<#19 (comment)>:
> @@ -41,12 +41,28 @@
(oll-core internal music-tools)
(lilypond-export lily)
(lilypond-export MusicXML)
- (lilypond-export Humdrum)
+ ;(lilypond-export Humdrum)
If I'm not mistaken this approach doesn't help either with my problem.
|resolve-module| allows loading a module by its name, but this seems to
require the module being "used" too - so there's no /real/ advantage
over using |load-from-path|.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#19 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADFWTjfpGyGQKraDTOECb71lCITOoKEcks5usG8AgaJpZM4X_92s>.
|
This doesn't work yet, but I want to submit it for review.
The idea is to create better modularity for the various exporter modules (see discussion at #17 (comment)).
What I think is the right thing is how package.ily goes along with choosing the "backend"/target.
I also think that FileExport in api.scm does the right thing.
What does not work is making the export-lilypond function from within Humdrum.scm (so far this only touches the Humdrum file) available to api.scm. I really don't understand it because in my MWE it did work. (see #17 (comment))