https://codereview.appspot.com/225040043/diff/40001/lily/include/sources.hh File lily/include/sources.hh (right):
https://codereview.appspot.com/225040043/diff/40001/lily/include/sources.hh#newcode27 lily/include/sources.hh:27: static SCM source_file_list; static? That means that every compilation unit including this header file gets a private copy of source_file_list. Hardly a good idea. https://codereview.appspot.com/225040043/diff/40001/lily/sources.cc File lily/sources.cc (right): https://codereview.appspot.com/225040043/diff/40001/lily/sources.cc#newcode78 lily/sources.cc:78: source_file_list = scm_cons (ly_string2scm (file_string), source_file_list); source_file_list is a global variable here (and I might add that you forget to protect it from garbage collection, likely inducing crashes, but that's a different problem). So if I do lilypond -fembed-source-files a.ly b.ly the PDF for b.ly will include the source(s) for a.ly. That's undesirable. Is there any reason you don't rely on the per-parser array source_files_ for your list of source files? https://codereview.appspot.com/225040043/diff/40001/scm/backend-library.scm File scm/backend-library.scm (right): https://codereview.appspot.com/225040043/diff/40001/scm/backend-library.scm#newcode85 scm/backend-library.scm:85: "-P" I read in "man gs" -P Makes Ghostscript to look first in the current directory for library files. By default, Ghostscript no longer looks in the current directory, unless, of course, the first explicitly sup‐ plied directory is "." in -I. See also the INITIALIZATION FILES section below, and bundled Use.htm for detailed discussion on search paths and how Ghostcript finds files. This seems quite unrelated to the issue of this patch and the rationale for specifying this option is totally undocumented. What is this for, and should this not rather be a separate patch if necessary for some reason for newer versions of GhostScript? https://codereview.appspot.com/225040043/diff/40001/scm/framework-ps.scm File scm/framework-ps.scm (right): https://codereview.appspot.com/225040043/diff/40001/scm/framework-ps.scm#newcode421 scm/framework-ps.scm:421: (map (lambda (fname) Use of `map' instead of `for-each'. https://codereview.appspot.com/225040043/diff/40001/scm/framework-ps.scm#newcode422 scm/framework-ps.scm:422: (set! idx (1+ idx)) quite unschemish. It seems cleaner to just give two list arguments to for-each, the second being (iota (length source-list)). Either that, or use a named let for your loop. https://codereview.appspot.com/225040043/diff/40001/scm/framework-ps.scm#newcode425 scm/framework-ps.scm:425: [ /_objdef {ly~a_stream} /type /stream /OBJ pdfmark [ ... pdfmark is a really ugly pairing. I'd rather use mark ... pdfmark https://codereview.appspot.com/225040043/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel