Issue 4849054: T1349 - Fix load order for running with Guile V2 (Closed)
|
Created:
13 years, 7 months ago by Ian Hulin (gmail) Modified:
13 years, 7 months ago CC:
lilypond-devel_gnu.org Visibility:
Public. |
DescriptionT1349 - Fix load order for running with Guile V2
1. Split load list into components (init-scheme-files-lib, *-used, *-body
and *-tail, and append them together before doing load.
2. Split markup macros from markup.scm to new file markup-macros.scm
=========================================
Closing Rietveld issue. Patches have been pushed and verified in Tracker system (1349).
Patch Set 1 #
Total comments: 77
Patch Set 2 : Implement Carl's observations and fix whitespace issues #Patch Set 3 : As patch-set 2, but including new version of markup.scm #Patch Set 4 : scm/lily.scm now rebased and merged with origin/master #
Total comments: 36
Patch Set 5 : Fix more whitespace errors in scm/lily.scm after Neil's comments #
Total comments: 2
Patch Set 6 : Fix two more whitespace errors in scm/lily.scm #Patch Set 7 : Patch based on rebased and merged version of scm/lily.scm #
MessagesTotal messages: 18
This patch adjusts the load order to one which works for both Guile V1.8.7 and Guile V2.0. The markup syntax macros have been moved from markup.scm to a new file markup-macros.scm, so these can be loaded before markup.scm. Attempting to move markup.scm without doing this causes interactions with other dependent .scm files in the list. Make and regtests run with Guile V1.8.7, V2.0 system completes load successfully but fails later in initialization owing to deprecation warnings (see Tracker 1780). Patch available for review on http:://codereview/appspot.com/4849054 Ian Hulin Ian Hulin
Sign in to reply to this message.
Thanks for tackling this. Your tab to space conversion on lily.scm has lots of errors. Please either revert those changes or get them right. markup.scm should continue to use interval-length, rather than car. Thanks, Carl http://codereview.appspot.com/4849054/diff/1/scm/lily.scm File scm/lily.scm (right): http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode229 scm/lily.scm:229: (ly:message Indent is wrong here -- should align with test, not with if. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode272 scm/lily.scm:272: (ly:get-option 'trace-memory-frequency) Indentation needs to move in one parenthesis level on this line and next. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode294 scm/lily.scm:294: (ly:progress "[~A" file-name)) indent one more level http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode296 scm/lily.scm:296: (ly:error (_ "cannot find: ~A") x)) indent one more level http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode299 scm/lily.scm:299: (ly:progress "]\n")))) indent one more level http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode303 scm/lily.scm:303: (vector-ref (uname) 0) char-set:letter+digit))) needs to be indented beyond string-tokenize http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode305 scm/lily.scm:305: (member (string-downcase (cadr platform)) '("95" "98" "me"))))) Indent one more level http://codereview.appspot.com/4849054/diff/1/scm/markup.scm File scm/markup.scm (right): http://codereview.appspot.com/4849054/diff/1/scm/markup.scm#newcode74 scm/markup.scm:74: (xoff (+ space (cdr (ly:stencil-extent head X))))) Please keep this interval-length, because it isolates the function from the data representation. If we need to split lily-library.scm so we can load it earlier, we should do so.
Sign in to reply to this message.
Response to Carl's comments, plus notes to self to sort out white-space issues. New Patchset will follow Ian http://codereview.appspot.com/4849054/diff/1/scm/lily.scm File scm/lily.scm (right): http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode116 scm/lily.scm:116: "midi") Sort out (increase) indentation for lines 115-116 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode229 scm/lily.scm:229: (ly:message On 2011/08/11 00:01:29, Carl wrote: > Indent is wrong here -- should align with test, not with if. Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode272 scm/lily.scm:272: (ly:get-option 'trace-memory-frequency) On 2011/08/11 00:01:29, Carl wrote: > Indentation needs to move in one parenthesis level on this line and next. Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode296 scm/lily.scm:296: (ly:error (_ "cannot find: ~A") x)) On 2011/08/11 00:01:29, Carl wrote: > indent one more level Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode299 scm/lily.scm:299: (ly:progress "]\n")))) On 2011/08/11 00:01:29, Carl wrote: > indent one more level Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode303 scm/lily.scm:303: (vector-ref (uname) 0) char-set:letter+digit))) On 2011/08/11 00:01:29, Carl wrote: > needs to be indented beyond string-tokenize Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode305 scm/lily.scm:305: (member (string-downcase (cadr platform)) '("95" "98" "me"))))) On 2011/08/11 00:01:29, Carl wrote: > Indent one more level Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode312 scm/lily.scm:312: (string-regexp-substitute "\\\\" "/" x)))) Sort out (increase) indentation for lines 311-312 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode327 scm/lily.scm:327: (eq? (string-ref file-name 2) #\/)))))) Sort out (increase) indentation for lines 322-327 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode342 scm/lily.scm:342: (fresh-interface!)))) Sort out (increase) indentation for lines 336-340/342 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode359 scm/lily.scm:359: #t)) Sort out (increase) indentation for lines 352-359 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode364 scm/lily.scm:364: (recursion-helper (cdr signature) (cdr arguments) (1+ count))))) Sort out (increase) indentation for lines 362-364 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode395 scm/lily.scm:395: (ly:version)) Sort out (increase) indentation for lines 393-395 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode603 scm/lily.scm:603: (stats (gc-stats))) Increase indentation. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode607 scm/lily.scm:607: (assoc-get 'total-cells-allocated stats 0)))) Increase indentation for 605-607 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode611 scm/lily.scm:611: (diff (map (lambda (y) (apply - y)) (zip this last)))) Increase indentation http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode618 scm/lily.scm:618: (cadr diff)))) Sort out (increase) indentation for lines 614-618 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode634 scm/lily.scm:634: (string<? (car x) (car y)))))) Sort out (increase) indentation for lines 631 634 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode645 scm/lily.scm:645: (outfile (open-file out-file-name "w"))) Sort out (increase) indentation for lines 639 645 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode655 scm/lily.scm:655: protects)) Sort out (increase) indentation for lines 649 655 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode661 scm/lily.scm:661: protects))) Sort out (increase) indentation for lines 657 661 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode704 scm/lily.scm:704: (mem (string->number (match:substring (car interesting) 1)))) Sort out (increase) indentation for lines 698 704 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode722 scm/lily.scm:722: acc)) Sort out (increase) indentation for lines 718 722 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode760 scm/lily.scm:760: files))))) Sort out (increase) indentation for lines 755 760 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode810 scm/lily.scm:810: (ly:exit 1 #f)))))) Sort out (increase) indentation for lines 764 810 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode823 scm/lily.scm:823: (ly:exit 0 #f))))) Sort out (increase) indentation for lines 816-818, 820-823 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode836 scm/lily.scm:836: (set! failed (append (list failed-file) failed))))) Sort out (increase) indentation for lines 828-836 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode864 scm/lily.scm:864: (dump-gc-protects) Sort out (increase) indentation for lines 841-864 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode872 scm/lily.scm:872: (dump-profile "lily-run-total" '(0 0) (profile-measurements))) Sort out (increase) indentation for lines 870/872 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode878 scm/lily.scm:878: (lambda (x . args) (handler x file-name)))) Sort out (increase) indentation for lines 887-888 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode891 scm/lily.scm:891: (ly:message "# -*-compilation-*-")) Sort out (increase) indentation for lines 887-891 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode901 scm/lily.scm:901: (ly:exit 0 #f))))) Sort out (increase) indentation for lines 893-901 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode907 scm/lily.scm:907: (cmd (get-editor-command welcome-ly 0 0 0))) Sort out (increase) indentation for lines 905-907 http://codereview.appspot.com/4849054/diff/1/scm/markup.scm File scm/markup.scm (right): http://codereview.appspot.com/4849054/diff/1/scm/markup.scm#newcode74 scm/markup.scm:74: (xoff (+ space (cdr (ly:stencil-extent head X))))) On 2011/08/11 00:01:29, Carl wrote: > Please keep this interval-length, because it isolates the function from the data > representation. > > If we need to split lily-library.scm so we can load it earlier, we should do so. Done.
Sign in to reply to this message.
http://codereview.appspot.com/4849054/diff/1/scm/lily.scm File scm/lily.scm (right): http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode116 scm/lily.scm:116: "midi") On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 115-116 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode312 scm/lily.scm:312: (string-regexp-substitute "\\\\" "/" x)))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 311-312 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode327 scm/lily.scm:327: (eq? (string-ref file-name 2) #\/)))))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 322-327 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode342 scm/lily.scm:342: (fresh-interface!)))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 336-340/342 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode359 scm/lily.scm:359: #t)) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 352-359 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode364 scm/lily.scm:364: (recursion-helper (cdr signature) (cdr arguments) (1+ count))))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 362-364 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode395 scm/lily.scm:395: (ly:version)) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 393-395 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode603 scm/lily.scm:603: (stats (gc-stats))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Increase indentation. Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode611 scm/lily.scm:611: (diff (map (lambda (y) (apply - y)) (zip this last)))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Increase indentation Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode618 scm/lily.scm:618: (cadr diff)))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 614-618 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode634 scm/lily.scm:634: (string<? (car x) (car y)))))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 631 634 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode645 scm/lily.scm:645: (outfile (open-file out-file-name "w"))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 639 645 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode655 scm/lily.scm:655: protects)) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 649 655 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode661 scm/lily.scm:661: protects))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 657 661 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode677 scm/lily.scm:677: (dump-live-object-stats outfile))) Indentation sorted, lines 666-677 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode685 scm/lily.scm:685: '(protected-objects bytes-malloced cell-heap-size))) Indentation sorted, lines 681-685 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode693 scm/lily.scm:693: (text (read-delimited "" file))) Indentation increased http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode704 scm/lily.scm:704: (mem (string->number (match:substring (car interesting) 1)))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 698 704 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode709 scm/lily.scm:709: (raise 1))))) Indentation sorted, line 708,709 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode722 scm/lily.scm:722: acc)) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 718 722 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode734 scm/lily.scm:734: (else (ly:message "")))) Indentation sorted, lines 732-734 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode744 scm/lily.scm:744: (ly:exit 0 #t))) Indentation sorted, line 744 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode747 scm/lily.scm:747: (ly:exit 0 #t))) Indentation sorted, line 747 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode752 scm/lily.scm:752: (ly:exit 2 #t))) Indentation sorted, line 752 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode760 scm/lily.scm:760: files))))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 755 760 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode762 scm/lily.scm:762: (>= (length files) (ly:get-option 'job-count))) Indentations sorted, line 762 http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode810 scm/lily.scm:810: (ly:exit 1 #f)))))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 764 810 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode823 scm/lily.scm:823: (ly:exit 0 #f))))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 816-818, 820-823 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode836 scm/lily.scm:836: (set! failed (append (list failed-file) failed))))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 828-836 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode864 scm/lily.scm:864: (dump-gc-protects) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 841-864 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode872 scm/lily.scm:872: (dump-profile "lily-run-total" '(0 0) (profile-measurements))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 870/872 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode878 scm/lily.scm:878: (lambda (x . args) (handler x file-name)))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 887-888 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode891 scm/lily.scm:891: (ly:message "# -*-compilation-*-")) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 887-891 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode901 scm/lily.scm:901: (ly:exit 0 #f))))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 893-901 Done. http://codereview.appspot.com/4849054/diff/1/scm/lily.scm#newcode907 scm/lily.scm:907: (cmd (get-editor-command welcome-ly 0 0 0))) On 2011/08/11 13:32:43, Ian Hulin (gmail) wrote: > Sort out (increase) indentation for lines 905-907 Done.
Sign in to reply to this message.
New patch-set available for review. Ian
Sign in to reply to this message.
On 2011/08/11 14:36:24, Ian Hulin (gmail) wrote: > New patch-set available for review. Please rebase against master. Cheers, Neil
Sign in to reply to this message.
On 2011/08/11 17:35:22, Neil Puttock wrote: > On 2011/08/11 14:36:24, Ian Hulin (gmail) wrote: > > New patch-set available for review. > > Please rebase against master. > > Cheers, > Neil Thanks for the catch. Rebased, scm/lily.scm merged and new patch-set uploaded. Cheers, Ian
Sign in to reply to this message.
http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm File scm/lily.scm (right): http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode324 scm/lily.scm:324: (> file-name-length 2) tab->space conversion has broken indentation here (and lines below) http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode408 scm/lily.scm:408: ;; Library definitions, need to be at the head of the list this is a top-level comment, so you can't indent it two spaces (alternatively, put it inside the define) http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode412 scm/lily.scm:412: ;; Files containing definitions used later by other files later in load top-level comment; fix indent http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode415 scm/lily.scm:415: ;; Main body of files to be loaded top-level comment; fix indent http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode467 scm/lily.scm:467: ;; Files to be loaded last top-level comment; fix indent http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode476 scm/lily.scm:476: init-scheme-files-used indent: (append init-scheme-files-lib init-scheme-files-used ...) http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode638 scm/lily.scm:638: (lambda (a b) tab->space conversion has broken indentation here (and lines below) http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode770 scm/lily.scm:770: 'log-file (format #f "~a-~a" restore indentation http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode779 scm/lily.scm:779: (acons (list-element-index joblist pid) restore indentation http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode795 scm/lily.scm:795: job (status:term-sig state))) restore indentation http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode802 scm/lily.scm:802: (map car errors))) restore indentation http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode806 scm/lily.scm:806: '(0 0) (profile-measurements))) restore indentation http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode817 scm/lily.scm:817: (string-contains f "lilypond"))))) restore indentation http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode831 scm/lily.scm:831: (format #f "~a.log" (ly:get-option 'log-file)) restore indentation http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode840 scm/lily.scm:840: (profile-measurements) restore indentation http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode855 scm/lily.scm:855: (mtrace:dump-results base))) restore indentation http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode857 scm/lily.scm:857: (ly:set-option (car s) (cdr s))) restore indentation http://codereview.appspot.com/4849054/diff/17001/scm/markup.scm File scm/markup.scm (left): http://codereview.appspot.com/4849054/diff/17001/scm/markup.scm#oldcode239 scm/markup.scm:239: (defmacro*-public markup (#:rest body) move this for consistency?
Sign in to reply to this message.
http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm File scm/lily.scm (right): http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode324 scm/lily.scm:324: (> file-name-length 2) On 2011/08/11 20:40:46, Neil Puttock wrote: > tab->space conversion has broken indentation here (and lines below) Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode408 scm/lily.scm:408: ;; Library definitions, need to be at the head of the list On 2011/08/11 20:40:46, Neil Puttock wrote: > this is a top-level comment, so you can't indent it two spaces > > (alternatively, put it inside the define) Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode412 scm/lily.scm:412: ;; Files containing definitions used later by other files later in load On 2011/08/11 20:40:46, Neil Puttock wrote: > top-level comment; fix indent Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode415 scm/lily.scm:415: ;; Main body of files to be loaded On 2011/08/11 20:40:46, Neil Puttock wrote: > top-level comment; fix indent Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode467 scm/lily.scm:467: ;; Files to be loaded last On 2011/08/11 20:40:46, Neil Puttock wrote: > top-level comment; fix indent Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode476 scm/lily.scm:476: init-scheme-files-used On 2011/08/11 20:40:46, Neil Puttock wrote: > indent: > > (append init-scheme-files-lib > init-scheme-files-used > ...) Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode638 scm/lily.scm:638: (lambda (a b) On 2011/08/11 20:40:46, Neil Puttock wrote: > tab->space conversion has broken indentation here (and lines below) Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode770 scm/lily.scm:770: 'log-file (format #f "~a-~a" On 2011/08/11 20:40:46, Neil Puttock wrote: > restore indentation Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode779 scm/lily.scm:779: (acons (list-element-index joblist pid) On 2011/08/11 20:40:46, Neil Puttock wrote: > restore indentation Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode795 scm/lily.scm:795: job (status:term-sig state))) On 2011/08/11 20:40:46, Neil Puttock wrote: > restore indentation Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode802 scm/lily.scm:802: (map car errors))) On 2011/08/11 20:40:46, Neil Puttock wrote: > restore indentation Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode806 scm/lily.scm:806: '(0 0) (profile-measurements))) On 2011/08/11 20:40:46, Neil Puttock wrote: > restore indentation Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode817 scm/lily.scm:817: (string-contains f "lilypond"))))) On 2011/08/11 20:40:46, Neil Puttock wrote: > restore indentation Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode831 scm/lily.scm:831: (format #f "~a.log" (ly:get-option 'log-file)) On 2011/08/11 20:40:46, Neil Puttock wrote: > restore indentation Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode840 scm/lily.scm:840: (profile-measurements) On 2011/08/11 20:40:46, Neil Puttock wrote: > restore indentation Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode855 scm/lily.scm:855: (mtrace:dump-results base))) On 2011/08/11 20:40:46, Neil Puttock wrote: > restore indentation Done. http://codereview.appspot.com/4849054/diff/17001/scm/lily.scm#newcode857 scm/lily.scm:857: (ly:set-option (car s) (cdr s))) On 2011/08/11 20:40:46, Neil Puttock wrote: > restore indentation Done. http://codereview.appspot.com/4849054/diff/17001/scm/markup.scm File scm/markup.scm (left): http://codereview.appspot.com/4849054/diff/17001/scm/markup.scm#oldcode239 scm/markup.scm:239: (defmacro*-public markup (#:rest body) On 2011/08/11 20:40:46, Neil Puttock wrote: > move this for consistency? I had some problems with other files in the load list on V2 if I moved it, it works here for running with V1.8.7 now, so I'd rather not touch it yet.
Sign in to reply to this message.
Two more indentation problems in scm/lily.scm new patchset follows. Ian http://codereview.appspot.com/4849054/diff/8004/scm/lily.scm File scm/lily.scm (right): http://codereview.appspot.com/4849054/diff/8004/scm/lily.scm#newcode394 scm/lily.scm:394: (ly:version)) Needs indenting to line up with (lambda in 391 http://codereview.appspot.com/4849054/diff/8004/scm/lily.scm#newcode900 scm/lily.scm:900: (ly:exit 0 #f))))) Indent block ll 892-900 by two spaces (one indent)
Sign in to reply to this message.
On 2011/08/11 23:16:59, Ian Hulin (gmail) wrote: > Two more indentation problems in scm/lily.scm new patchset follows. > Ian > > http://codereview.appspot.com/4849054/diff/8004/scm/lily.scm > File scm/lily.scm (right): > > http://codereview.appspot.com/4849054/diff/8004/scm/lily.scm#newcode394 > scm/lily.scm:394: (ly:version)) > Needs indenting to line up with (lambda in 391 > > http://codereview.appspot.com/4849054/diff/8004/scm/lily.scm#newcode900 > scm/lily.scm:900: (ly:exit 0 #f))))) > Indent block ll 892-900 by two spaces (one indent) New patch-set 6 available for review, Ian
Sign in to reply to this message.
Reinhold's recent commit for log-levels will require a git re-base and probably a merge for scm/lily.scm. Will do this, re-test and upload new patch-set. Ian
Sign in to reply to this message.
Lgtm. Some scheme indentation errors as artifacts from the old file, bit these will be fixed automatically when fix-scheme.sh is run after GOP-Prop 10 is accepted.
Sign in to reply to this message.
LGTM.
Sign in to reply to this message.
The load-order issue appears to be fixed, testing with git and guile 1.8 and 2.0.2. Ignoring whitespace changes, this patch LGTM. Some more shuffling is needed to make sure we have markup commands defined where they need to be, but that's beyond the scope of this patch. Thanks, Patrick
Sign in to reply to this message.
Hi Patrick, On Thu 18 Aug 2011 07:50:28 BST, pnorcks@gmail.com wrote: > > The load-order issue appears to be fixed, testing with git and guile 1.8 > and 2.0.2. Ignoring whitespace changes, this patch LGTM. > > Some more shuffling is needed to make sure we have markup commands > defined where they need to be, but that's beyond the scope of this > patch. > Should we have a Lilypond Markup Syntax/Guile V2 Tracker, then? If there are any dependencies in the load list that you know about that can still bite us could we record these in the tracker, for instance the markup command itself works with both Guile 1.8 and 2.0 if it is loaded later in the list in a module apart from markup-macros.scm, but fails with 2.0 if you move the definition from markup.scm to the bottom of markup-macros.scm. There are also some design issues to consider like: * Would the markup.scm and markup-macros.scm definitions work better as a SCM module? * Should we/can we move over to making the markup subsystem hygienic by using define-syntax? o (This is quite a big piece of work, and last time I tried looking at it, it seemed like it should be simple but made my head hurt). o It's a translation exercise very like doing the document translation, and currently *definitely* needs to be done by a human being rather than software. Current plans once this is pushed is to tackle Tracker 1780 (Guile V2 squawking with deprecation errors because of (format) calls without a destination parameter) and then get back to Tracker 1686 (tackling the issue of scheme compilation). > > Thanks, > Patrick > > https://codereview.appspot.com/4849054/ Cheers, Ian
Sign in to reply to this message.
On Thu, Aug 18, 2011 at 3:30 AM, Ian Hulin <ianhulin44@gmail.com> wrote: > On Thu 18 Aug 2011 07:50:28 BST, pnorcks@gmail.com wrote: > > The load-order issue appears to be fixed, testing with git and guile 1.8 > and 2.0.2. Ignoring whitespace changes, this patch LGTM. > > Some more shuffling is needed to make sure we have markup commands > defined where they need to be, but that's beyond the scope of this > patch. > > Should we have a Lilypond Markup Syntax/Guile V2 Tracker, then? I think we should have one tracker per issue that we encounter. It becomes difficult to handle what has been fixed (and where) when we have umbrella issues. > If there are any dependencies in the load list that you know about that can > still bite us could we record these in the tracker, for instance the markup > command itself works with both Guile 1.8 and 2.0 if it is loaded later in > the list in a module apart from markup-macros.scm, but fails with 2.0 if you > move the definition from markup.scm to the bottom of markup-macros.scm. Yes, any issues we find like this should have separate tracker issues. I've just opened one: https://code.google.com/p/lilypond/issues/detail?id=1826 I'll open more later today if I find time. > There are also some design issues to consider like: > > Would the markup.scm and markup-macros.scm definitions work better as a SCM > module? Possibly. > Should we/can we move over to making the markup subsystem hygienic by using > define-syntax? > > (This is quite a big piece of work, and last time I tried looking at it, it > seemed like it should be simple but made my head hurt). > It's a translation exercise very like doing the document translation, and > currently definitely needs to be done by a human being rather than software. If we want backwards compatibility with Guile 1.8, we shouldn't move to define-syntax, as it's not implemented in Guile 1.8. :P > Current plans once this is pushed is to tackle Tracker 1780 (Guile V2 > squawking with deprecation errors because of (format) calls without a > destination parameter) and then get back to Tracker 1686 (tackling the issue > of scheme compilation). Sounds good, thanks Ian!
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
