Skip to content

WIP! Attempt at changing plugin loading structure #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

WIP! Attempt at changing plugin loading structure #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Humdrum.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@

;% TODO ties, slurs, grace notes

(define-module (lilypond-export Humdrum))
;(define-module (lilypond-export Humdrum))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still need this define-module here.


(use-modules
(oll-core tree)
Expand All @@ -44,7 +44,7 @@
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; humdrum export

(define-public (exportHumdrum musicexport filename . options)
(define-public (export-lilypond musicexport filename . options)
;(display musicexport)
(let ((grid (tree-create 'grid))
(bar-list (sort (filter integer? (tree-get-keys musicexport '())) (lambda (a b) (< a b))) )
Expand Down Expand Up @@ -269,5 +269,5 @@
))
))

(set-object-property! exportHumdrum 'file-suffix "krn")
(set-object-property! export-lilypond 'file-suffix "krn")

23 changes: 20 additions & 3 deletions api.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,28 @@
(oll-core internal music-tools)
(lilypond-export lily)
(lilypond-export MusicXML)
(lilypond-export Humdrum)
;(lilypond-export Humdrum)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

(lily))

(re-export exportLilyPond)
(re-export exportMusicXML)
(re-export exportHumdrum)
;(re-export exportHumdrum)

(define-public (load-exporter target)
(let*
; We don't have access to os-path from a Scheme module
((oll-core-root #{ \getOption oll-core.root #})
(oll-root (list-head oll-core-root (- (length oll-core-root) 1)))
(here (string-join (append oll-root '("lilypond-export")) "/"))
(module-file (format "~a/~a.scm" here target)))
(ly:message "Going to load ~a\n" module-file)
(load-from-path module-file)
; FIXME
; I don't know why export-lilypond is not visible here.
; In my MWE this approach worked perfectly.
; It's clear that load-from-path does succeed
; (because it fails if you change module-file by a character).
export-lilypond))

; create duration from moment
(define-public (moment->duration mom)
Expand Down Expand Up @@ -467,7 +483,8 @@
(define-public scoreExporter
; engraver to export tree in foreign format
(define-scheme-function (options)(list?)
(let* ((exporter (ly:assoc-get 'exporter options exportHumdrum #f))
(let* ((target (ly:assoc-get 'target options 'Humdrum #f))
(exporter (load-exporter target))
(suffix (ly:assoc-get 'filesuffix options (object-property exporter 'file-suffix) #f))
(filename (ly:assoc-get 'filename options
(format "~A.~A"
Expand Down
18 changes: 9 additions & 9 deletions export-example.ly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ music = \new PianoStaff <<
>>

% exporter can run without actually typesetting
\exportMusic \default hum \music
\exportMusic #'Humdrum \music

opts.exporter = #exportMusicXML
%opts.exporter = #exportMusicXML
% or as a layout extension that is added to the layout
\score {
\music
\layout {
\FileExport #opts
}
\midi {}
}
%\score {
% \music
% \layout {
% \FileExport #opts
% }
% \midi {}
%}
28 changes: 14 additions & 14 deletions package.ily
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,21 @@

%%%% export music
% filebase: file basename - suffix (.krn/.xml) is taken from the exporter
% exporter: symbol or function: hum -> humdrum, xml -> musicXML,
% not implemented yet: [l]mei -> [L-]MEI, lily -> LilyPond
% target: export module name (symbol?):
% - Humdrum,
% - MusicXML,
% to be implemented:
% - MEI
% - LilyPond
% or an exporter function #(lambda (export-tree filename . options) ...)
% music: the music to export
#(define (symbol-or-procedure? v) (or (symbol? v)(procedure? v)))

exportMusic =
#(let
((exporters
`((xml . ,exportMusicXML)
(hum . ,exportHumdrum)
(lily . ,exportLilyPond))))
(define-void-function (filebase exporter music)
((string? (ly:parser-output-name)) symbol-or-procedure? ly:music?)
(if (symbol? exporter)
(set! exporter (ly:assoc-get exporter exporters exportMusicXML #t)))
(ly:run-translator
(ly:score-music (scorify-music music))
(FileExport `((filebase . ,filebase)(exporter . ,exporter))))))
#(define-void-function (filebase target music)
((string? (ly:parser-output-name)) symbol-or-procedure? ly:music?)
(ly:run-translator
(ly:score-music (scorify-music music))
(FileExport
`((filebase . ,filebase)
(target . ,target)))))