Re: Axis group interface ignores column rank for pure-from-neighbor-interface (issue 5843063)
Reviewers: Keith, mike_apollinemike.com,
Message:
Running regtests. Will post new patch once it gets a clean bill of
health.
http://codereview.appspot.com/5843063/diff/7/input/regression/pure-from-neighbor-interface-pure-height.ly
File input/regression/pure-from-neighbor-interface-pure-height.ly
(right):
http://codereview.appspot.com/5843063/diff/7/input/regression/pure-from-neighbor-interface-pure-height.ly#newcode1
input/regression/pure-from-neighbor-interface-pure-height.ly:1: \version
2.15.34
On 2012/03/21 05:58:42, Keith wrote:
.35
The title makes less sense now. lyrics-spanbarstub.ly ?
Done.
http://codereview.appspot.com/5843063/diff/7/input/regression/pure-from-neighbor-interface-pure-height.ly#newcode4
input/regression/pure-from-neighbor-interface-pure-height.ly:4: texidoc
= @code{axis-group-interface::pure-height} does not take spanned
On 2012/03/21 05:58:42, Keith wrote:
The description no longer fits. Maybe
empty measures do not confuse SpanBarStub. These lyrics should
remain clear of
the span bars.
Done.
http://codereview.appspot.com/5843063/diff/7/lily/item.cc
File lily/item.cc (right):
http://codereview.appspot.com/5843063/diff/7/lily/item.cc#newcode245
lily/item.cc:245: cache_pure_height (Grob::pure_height (this, 0,
INT_MAX));
On 2012/03/21 05:58:42, Keith wrote:
This commit :
http://git.savannah.gnu.org/gitweb/?p=lilypond.git;a=commitdiff;h=9fe7859a8592a080413b744e3768db41059dbfe3
depended on having 'start' and 'end' passed through, so we should put
this back.
Caching in this way assumes that the pure_height of an Item is
independent of
where the line-breaks are.
Anything that gets pure_height from its neighbors would break that
assumption,
but now it seems there are no such Items, as they all use
pure-from-neighbors-interface to set their extra-spacing-height.
Tied accidentals break that assumption as well, but that's not our
fault. I'll
write a cautionary comment in a separate commit.
Done.
http://codereview.appspot.com/5843063/diff/7/scm/define-grobs.scm
File scm/define-grobs.scm (right):
http://codereview.appspot.com/5843063/diff/7/scm/define-grobs.scm#newcode1883
scm/define-grobs.scm:1883: (Y-extent . (1 . -1))
On 2012/03/21 05:58:42, Keith wrote:
I suggest (Y-extent . #f)
because (1 . -1) looks so much like (-1 . 1), and it also makes
suspicious
people like me think it is an uncommented magic number.
Done.
http://codereview.appspot.com/5843063/diff/7/scm/output-lib.scm
File scm/output-lib.scm (right):
http://codereview.appspot.com/5843063/diff/7/scm/output-lib.scm#newcode437
scm/output-lib.scm:437: (let* ((height (ly:grob-pure-height grob grob 0
1000))
On 2012/03/21 05:58:42, Keith wrote:
The C code uses INT_MAX to represent the end of the whole score, so it
would be
nice to use the same here, if possible.
There'd have to be a constant defined for INT_MAX in guile. I believe
that INT_MAX can change between computers (not sure if it's the compiler
or system that does this). Probably worth doing in a separate commit
where INT-MAX is defined in the same place as all of the PI constants
and then plugged anywhere where there's a large integer.
Description:
Axis group interface ignores column rank for
pure-from-neighbor-interface
Please review this at http://codereview.appspot.com/5843063/
Affected files:
A input/regression/pure-from-neighbor-interface-pure-height.ly
M lily/item.cc
M lily/pure-from-neighbor-engraver.cc
M scm/define-grobs.scm
M scm/output-lib.scm
Index: input/regression/pure-from-neighbor-interface-pure-height.ly
diff --git a/input/regression/pure-from-neighbor-interface-pure-height.ly
b/input/regression/pure-from-neighbor-interface-pure-height.ly
new file mode 100644
index
..3e7515dfea1cc991fef736e76c9566ffca728c64
--- /dev/null
+++ b/input/regression/pure-from-neighbor-interface-pure-height.ly
@@ -0,0 +1,18 @@
+\version 2.15.34
+
+\header {
+ texidoc = @code{axis-group-interface::pure-height} does not take spanned
+rank interval into account for @code{pure-from-neighbor-interface} grobs.
+
+}
+
+\new StaffGroup
+ \new Staff { \repeat unfold 8 { R1 e'1 } }
+ \addlyrics {
+Worked twice...
+and then
+I continued...
+working... correctly.
+ }
+ \new Staff { R1*16 }
+
Index: lily/item.cc
diff --git a/lily/item.cc b/lily/item.cc
index
6ea5643a5445e7d791a119479e73d54a5fe7aead..19c9926c3b755ae660e3a698da441498d755d5e1
100644
--- a/lily/item.cc
+++ b/lily/item.cc
@@ -242,7 +242,7 @@ Item::pure_height (Grob *g, int start, int end)
if (cached_pure_height_valid_)
return cached_pure_height_ + pure_relative_y_coordinate (g, start,
end);
- cache_pure_height (Grob::pure_height (this, start, end));
+ cache_pure_height (Grob::pure_height (this, 0, INT_MAX));
return cached_pure_height_ + pure_relative_y_coordinate (g, start, end);
}
Index: lily/pure-from-neighbor-engraver.cc
diff --git a/lily/pure-from-neighbor-engraver.cc
Re: FW: [LilyPond] Your organization application has been rejected.
On Wed, Mar 21, 2012 at 3:49 AM, Han-Wen Nienhuys hanw...@gmail.com wrote: 2012/3/20 Łukasz Czerwiński milimet...@gmail.com: Can you share your thoughts? Lilypond would never be useful for Google products. There is absolutely no point for Google to pay for code that would never be useful for them. The point of GSOC is not to improve google products. See also: http://www.google-melange.com/document/show/gsoc_program/google/gsoc2011/faqs#goals I suppose that Łukasz (along with some other people that i've talked with) finds it hard to believe that Google does something that doesn't have to bring measurable profit :) Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fingering avoidance of accidentals depends on Fingering #'script-priority
Nick
On 19 March 2012 05:19, Nick Payne nick.pa...@internode.on.net wrote:
In a chord with two accidentals, if I set the fingering script-priority to
-99, the fingerings still avoid both accidentals, but if I set it to -100,
each fingering indication is positioned without regard to the accidental on
the other note. Is this related to
https://code.google.com/p/lilypond/issues/detail?id=2182, or soemthing else?
\relative c' {
\set fingeringOrientations = #'(left)
fis-4 dis-21
\override Fingering #'script-priority = #-99
fis-4 dis-2
\override Fingering #'script-priority = #-100
fis-4 dis-2
}
I've updated https://code.google.com/p/lilypond/issues/detail?id=2182
with this new example.
--
--
James
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: FW: [LilyPond] Your organization application has been rejected.
Janek Warchoł janek.lilyp...@gmail.com writes: On Wed, Mar 21, 2012 at 3:49 AM, Han-Wen Nienhuys hanw...@gmail.com wrote: 2012/3/20 Łukasz Czerwiński milimet...@gmail.com: Can you share your thoughts? Lilypond would never be useful for Google products. There is absolutely no point for Google to pay for code that would never be useful for them. The point of GSOC is not to improve google products. See also: http://www.google-melange.com/document/show/gsoc_program/google/gsoc2011/faqs#goals I suppose that Łukasz (along with some other people that i've talked with) finds it hard to believe that Google does something that doesn't have to bring measurable profit :) It certainly does bring measurable profit. It makes Google a cool company, and that means you get highly qualified and motivated people. Google is a company built around advertising, image and perception. As is its stock value. I think it is also encouraged of employees to work on one project of one's liking per week rather than on company business. That seems like it would carry a higher burden than the whole GSoC enterprise. Open Source proponents try to liken successful distributors of free software to the bottled water business. Google, in contrast, excels at bottling hot air and making that fly. Which is actually still more substantial than controlling the dispersion of bottled promises (bank notes), nowadays done mostly without the bottles (namely as electronic bank transfers, making it possible for, say, Icelandic banks to deal with many more promises than they could actually bottle). -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Axis group interface ignores column rank for pure-from-neighbor-interface (issue 5843063)
http://codereview.appspot.com/5843063/diff/7/scm/output-lib.scm File scm/output-lib.scm (right): http://codereview.appspot.com/5843063/diff/7/scm/output-lib.scm#newcode437 scm/output-lib.scm:437: (let* ((height (ly:grob-pure-height grob grob 0 1000)) On 2012/03/21 07:05:50, MikeSol wrote: On 2012/03/21 05:58:42, Keith wrote: The C code uses INT_MAX to represent the end of the whole score, so it would be nice to use the same here, if possible. There'd have to be a constant defined for INT_MAX in guile. I believe that INT_MAX can change between computers (not sure if it's the compiler or system that does this). Probably worth doing in a separate commit where INT-MAX is defined in the same place as all of the PI constants and then plugged anywhere where there's a large integer. INT_MAX would be a stupid constant to use in Scheme since it rather certainly does not fit in a single cell and thus does not have an efficient representation. If we need such a thing, we should pick an arbitrary large constant that is large enough to do the job and small enough not to trigger multi-cell representations in any Guile implementation. http://codereview.appspot.com/5843063/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 1320: Rewriting bar-line::print
Am 21.03.2012 03:20, schrieb David Kastrup: Marc Hohlm...@hohlart.de writes: Is this a feasible approach? What am I currently doing wrong? #(define bar-line-stencil-alist '((| . thin-stil) (. . thick-stil) ( . empty-stil) )) (thin-stil (bar-line::simple-bar-line grob hair extent rounded)) (thick-stil (bar-line::simple-bar-line grob fatline extent rounded)) (empty-stil (make-filled-box-stencil (cons 0 0) (cons 0 extent))) (stencil (assoc-get glyph bar-line-stencil-alist empty-stil)) ) stencil )) That does not work. stencil is set to a symbol, and that symbol is never converted to a value. When the function is being executed, the _symbols_ thin-stil, stick-stil and empty-stil have long lost any connection with the expressions that they were associated with while the let-statement was being compiled. You could write (local-eval stencil (the-environment)) to let the lexical environment of the function compilation linger over long enough to make this work, but you would earn a reward for the unnecessarily most ugly and unstable code imaginable. Instead, try using a case statement. There is no need to calculate all stencils and throwing most of them away. That's what I originally did, but my idea was to provide an easy way to add new stencils/bar lines without fiddling with bar-line::print. But if this way is a no-go, I'll use case. Thanks for your explanations! Marc ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 1320: Rewriting bar-line::print
Marc Hohl m...@hohlart.de writes: Am 21.03.2012 03:20, schrieb David Kastrup: Marc Hohlm...@hohlart.de writes: Is this a feasible approach? What am I currently doing wrong? #(define bar-line-stencil-alist '((| . thin-stil) (. . thick-stil) ( . empty-stil) )) (thin-stil (bar-line::simple-bar-line grob hair extent rounded)) (thick-stil (bar-line::simple-bar-line grob fatline extent rounded)) (empty-stil (make-filled-box-stencil (cons 0 0) (cons 0 extent))) (stencil (assoc-get glyph bar-line-stencil-alist empty-stil)) ) stencil )) That does not work. stencil is set to a symbol, and that symbol is never converted to a value. When the function is being executed, the _symbols_ thin-stil, stick-stil and empty-stil have long lost any connection with the expressions that they were associated with while the let-statement was being compiled. You could write (local-eval stencil (the-environment)) to let the lexical environment of the function compilation linger over long enough to make this work, but you would earn a reward for the unnecessarily most ugly and unstable code imaginable. Instead, try using a case statement. There is no need to calculate all stencils and throwing most of them away. That's what I originally did, but my idea was to provide an easy way to add new stencils/bar lines without fiddling with bar-line::print. But if this way is a no-go, I'll use case. thin-stil and thick-stil are internals of the function. The way to make this work is as an association list of functions (I don't know the problem well enough to know what parameter list they should be getting, but they would likely return a stencil, and so it would probably make sense to have them get the same parameter list that a stencil callback would) and/or finished stencils. If the variability is likely to be restricted to the thickness, you can provide something like (define ((bar-line::simple-bar-line-maker thickness) grob) [...] (bar-line::simple-bar-line grob thickness extent rounded)) And then put (bar-line::simple-bar-line-maker 1.5) as the function into the association list. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 1320: Rewriting bar-line::print
Am 21.03.2012 11:03, schrieb David Kastrup: Marc Hohlm...@hohlart.de writes: Am 21.03.2012 03:20, schrieb David Kastrup: Marc Hohlm...@hohlart.de writes: Is this a feasible approach? What am I currently doing wrong? #(define bar-line-stencil-alist '((| . thin-stil) (. . thick-stil) ( . empty-stil) )) (thin-stil (bar-line::simple-bar-line grob hair extent rounded)) (thick-stil (bar-line::simple-bar-line grob fatline extent rounded)) (empty-stil (make-filled-box-stencil (cons 0 0) (cons 0 extent))) (stencil (assoc-get glyph bar-line-stencil-alist empty-stil)) ) stencil )) That does not work. stencil is set to a symbol, and that symbol is never converted to a value. When the function is being executed, the _symbols_ thin-stil, stick-stil and empty-stil have long lost any connection with the expressions that they were associated with while the let-statement was being compiled. You could write (local-eval stencil (the-environment)) to let the lexical environment of the function compilation linger over long enough to make this work, but you would earn a reward for the unnecessarily most ugly and unstable code imaginable. Instead, try using a case statement. There is no need to calculate all stencils and throwing most of them away. That's what I originally did, but my idea was to provide an easy way to add new stencils/bar lines without fiddling with bar-line::print. But if this way is a no-go, I'll use case. thin-stil and thick-stil are internals of the function. The way to make this work is as an association list of functions (I don't know the problem well enough to know what parameter list they should be getting, but they would likely return a stencil, and so it would probably make sense to have them get the same parameter list that a stencil callback would) and/or finished stencils. If the variability is likely to be restricted to the thickness, you can provide something like (define ((bar-line::simple-bar-line-maker thickness) grob) [...] (bar-line::simple-bar-line grob thickness extent rounded)) And then put (bar-line::simple-bar-line-maker 1.5) as the function into the association list. Ah, ok, but there are more bar line styles, so this won't solve the problem. Anyway, I don't get the case statement right - am I too stupid? Please have a look at the attached file; I don't get a stencil back, the case statement always uses the else clause. A (string? glyph) returns #t, so glyph is obviously a string ... ?? Regards, Marc \version 2.15.34 #(define (bar-line::calc-bar-extent grob) (let ((staff-symbol (ly:grob-object grob 'staff-symbol)) (staff-extent (cons 0 0))) (if (ly:grob? staff-symbol) (let* ((bar-line-color (ly:grob-property grob 'color)) (staff-color (ly:grob-property staff-symbol 'color)) (radius (ly:staff-symbol-staff-radius grob)) (line-thickness (ly:staff-symbol-line-thickness grob))) (set! staff-extent (ly:staff-symbol::height staff-symbol)) (if (and (eq? bar-line-color staff-color) radius) (interval-widen staff-extent (- 1 (* 1/2 (/ line-thickness radius))) staff-extent)) #(define-public (bar-line::print grob) (let* ((glyph (ly:grob-property grob 'glyph-name)) (bar-extent (ly:grob-property grob 'bar-extent '(0 . 0))) (extent (interval-length bar-extent)) (stencil (if (and (not (eq? glyph '())) ( extent 0)) (bar-line::compound-bar-line grob glyph extent #f stencil)) #(define (bar-line::simple-bar-line grob width extent rounded) (let ((blot (if rounded (ly:output-def-lookup layout 'blot-diameter) 0))) (ly:round-filled-box (cons 0 width) (cons (* -0.5 extent) (* 0.5 extent)) blot))) #(define (bar-line::tick-bar-line grob height rounded) (let ((half-staff (* 1/2 (ly:staff-symbol-staff-space grob))) (stafflinethick (ly:staff-symbol-line-thickness grob)) (blot (if rounded (ly:output-def-lookup layout 'blot-diameter) 0))) (ly:round-filled-box (cons 0 stafflinethick) (cons (- height half-staff) (+ height half-staff)) blot))) #(define (bar-line::compound-bar-line grob glyph extent rounded) (let* ((staff-symbol (ly:grob-object grob 'staff-symbol)) (line-count (if (ly:grob? staff-symbol) (ly:grob-property staff-symbol 'line-count) 0)) (staff-space (ly:staff-symbol-staff-space grob)) (stafflinethick (ly:staff-symbol-line-thickness grob)) (dist
Re: Issue 1320: Rewriting bar-line::print
Am 21.03.2012 02:11, schrieb Thomas Morley: Hi Marc, Some observations: the main problem seems to be the storing of the new stencils in an alist and how to call them. Directly in bar-line::compound-bar-line works. Ok, this is what David explained to me, too. Also, I created bar-line-stencil-alist-demo containing new stencils, which are defined before. Calling them in bar-line::compound-bar-line works too. But I didn't manage to make it work with the original alist. The alist approach doesn't work, so I will use alternatives. In addition I changed a line in bar-line::simple-bar-line otherwise the barlines are printed misplaced in Y-direction. Thanks! Regards, Marc HTH, Harm ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Autobeaming works properly with tuplet recheck (Issue 2408) (issue 5844052)
http://codereview.appspot.com/5844052/diff/3001/lily/beaming-pattern.cc File lily/beaming-pattern.cc (right): http://codereview.appspot.com/5844052/diff/3001/lily/beaming-pattern.cc#newcode361 lily/beaming-pattern.cc:361: } My comment has nothing to do with your patch but with this function - it seems like there's a memory leak with *dur (I know very little about memory management, so sorry in advance if I'm totally off...). http://codereview.appspot.com/5844052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Autobeaming works properly with tuplet recheck (Issue 2408) (issue 5844052)
http://codereview.appspot.com/5844052/diff/3001/lily/beaming-pattern.cc File lily/beaming-pattern.cc (right): http://codereview.appspot.com/5844052/diff/3001/lily/beaming-pattern.cc#newcode361 lily/beaming-pattern.cc:361: } On 2012/03/21 10:51:29, MikeSol wrote: My comment has nothing to do with your patch but with this function - it seems like there's a memory leak with *dur (I know very little about memory management, so sorry in advance if I'm totally off...). No, you are quite correct. There is no sense in allocating a duration on the heap here. It would be simpler to just write Duration dur (2 + ...); [...] * dur.get_length (); http://codereview.appspot.com/5844052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 1320: Rewriting bar-line::print
Marc Hohl m...@hohlart.de writes: Anyway, I don't get the case statement right - am I too stupid? Stupid enough to follow my advice without checking the manual first (or afterwards). Looking in the Guile manual, it turns out that case uses eqv? for checking equality. guile (eqv? x x) #f guile So you need to revert to cond here and use string=. Or employ assoc in some form. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: lilypond-book: Set include path for --output option (issue 2423). (issue 5846075)
On Wed, Mar 21, 2012 at 1:39 AM, joenee...@gmail.com wrote: http://codereview.appspot.com/5846075/diff/1/scripts/lilypond-book.py File scripts/lilypond-book.py (right): http://codereview.appspot.com/5846075/diff/1/scripts/lilypond-book.py#newcode639 scripts/lilypond-book.py:639: global_options.include_path.insert (0, inverse_relpath (original_dir, global_options.output_dir)) Wouldn't it be easier if you just added the absolute path to original_dir? It would be easier, but I think the proposed patch is a better implementation. Using absolute paths makes the document less portable and prone to errors caused by any weird characters that a user might have in the names of parent directories. Absolute paths also didn't play well with lilypond on Windows (see http://code.google.com/p/lilypond/issues/detail?id=2209 ). While I didn't test the suggested patch on Windows yet, I will do so very soon. Regards, Julien http://codereview.appspot.com/5846075/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
A new home for browser-language (issue 2412). (issue 5870043)
http://codereview.appspot.com/5870043/diff/1/python/auxiliar/postprocess_html.py File python/auxiliar/postprocess_html.py (right): http://codereview.appspot.com/5870043/diff/1/python/auxiliar/postprocess_html.py#newcode71 python/auxiliar/postprocess_html.py:71: browser_language_url = /misc/browser-language please change to /website/misc/browser-language. I'm not 100% certain it's safe to have a /misc/ so let's avoid that potential problem for now. http://codereview.appspot.com/5870043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: A new home for browser-language (issue 2412). (issue 5870043)
On Wed, Mar 21, 2012 at 4:33 PM, gra...@percival-music.ca wrote: http://codereview.appspot.com/5870043/diff/1/python/auxiliar/postprocess_html.py File python/auxiliar/postprocess_html.py (right): http://codereview.appspot.com/5870043/diff/1/python/auxiliar/postprocess_html.py#newcode71 python/auxiliar/postprocess_html.py:71: browser_language_url = /misc/browser-language please change to /website/misc/browser-language. I'm not 100% certain it's safe to have a /misc/ so let's avoid that potential problem for now. http://codereview.appspot.com/5870043/ Sure I'll change it. Does this also apply to the other patch on countdown i.e. http://codereview.appspot.com/5843069/diff/17/Documentation/common-macros.itexi ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Tracks old announcements, news and changelogs. (issue 5843069)
http://codereview.appspot.com/5843069/diff/17/Documentation/common-macros.itexi
File Documentation/common-macros.itexi (right):
http://codereview.appspot.com/5843069/diff/17/Documentation/common-macros.itexi#newcode185
Documentation/common-macros.itexi:185:
@uref{http://www.lilypond.org/misc/\MISC-FILE\,\MISC-TEXT\}
sorry, please change this to http://www.lilypond.org/website/misc/
http://codereview.appspot.com/5843069/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Tracks old announcements, news and changelogs. (issue 5843069)
On Wed, Mar 21, 2012 at 5:16 PM, gra...@percival-music.ca wrote:
http://codereview.appspot.com/5843069/diff/17/Documentation/common-macros.itexi
File Documentation/common-macros.itexi (right):
http://codereview.appspot.com/5843069/diff/17/Documentation/common-macros.itexi#newcode185
Documentation/common-macros.itexi:185:
@uref{http://www.lilypond.org/misc/\MISC-FILE\,\MISC-TEXT\}
sorry, please change this to http://www.lilypond.org/website/misc/
http://codereview.appspot.com/5843069/
Done. I'm running patch-new patchy on this.
Julien
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: A new home for browser-language (issue 2412). (issue 5870043)
On Wed, Mar 21, 2012 at 4:42 PM, Julien Rioux julien.ri...@gmail.com wrote: On Wed, Mar 21, 2012 at 4:33 PM, gra...@percival-music.ca wrote: http://codereview.appspot.com/5870043/diff/1/python/auxiliar/postprocess_html.py File python/auxiliar/postprocess_html.py (right): http://codereview.appspot.com/5870043/diff/1/python/auxiliar/postprocess_html.py#newcode71 python/auxiliar/postprocess_html.py:71: browser_language_url = /misc/browser-language please change to /website/misc/browser-language. I'm not 100% certain it's safe to have a /misc/ so let's avoid that potential problem for now. http://codereview.appspot.com/5870043/ Sure I'll change it. Does this also apply to the other patch on countdown i.e. http://codereview.appspot.com/5843069/diff/17/Documentation/common-macros.itexi Done. I'm running patch-new patchy on this. Julien ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
FW: Issue 2389 in lilypond: Patch: Fix make error in regression tests coming from midi2ly
On 3/20/12 9:08 PM, lilyp...@googlecode.com lilyp...@googlecode.com wrote: Updates: Labels: -Patch-countdown Patch-push Comment #11 on issue 2389 by ColinPKCampbell: Patch: Fix make error in regression tests coming from midi2ly http://code.google.com/p/lilypond/issues/detail?id=2389 Counted down to 20120320, please push. This patch has been tested on my system, but not by patchy-merge. It passed the tests before, but broke staging. In order to avoid messing up staging, I think it would be best to have this patch tested by patchy-merge as the only patch on staging. Could we set up a separate branch (e.g. staging-test) on which to test this patch? Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 2389 in lilypond: Patch: Fix make error in regressiontests coming from midi2ly
- Original Message - From: Carl Sorensen c_soren...@byu.edu To: Lily-Devel List lilypond-devel@gnu.org Sent: Wednesday, March 21, 2012 5:50 PM Subject: FW: Issue 2389 in lilypond: Patch: Fix make error in regressiontests coming from midi2ly On 3/20/12 9:08 PM, lilyp...@googlecode.com lilyp...@googlecode.com wrote: Updates: Labels: -Patch-countdown Patch-push Comment #11 on issue 2389 by ColinPKCampbell: Patch: Fix make error in regression tests coming from midi2ly http://code.google.com/p/lilypond/issues/detail?id=2389 Counted down to 20120320, please push. This patch has been tested on my system, but not by patchy-merge. It passed the tests before, but broke staging. In order to avoid messing up staging, I think it would be best to have this patch tested by patchy-merge as the only patch on staging. Could we set up a separate branch (e.g. staging-test) on which to test this patch? Thanks, Carl Simpler to run on a clean system other than patchy. I'll apply the patch and run make, make doc and make test and let you know. -- Phil Holmes ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Corrected style of comments (issue 5862052)
Reviewers: , Message: This is a continuation of Issue: http://codereview.appspot.com/5651069/ Description: Corrected style of comments Please review this at http://codereview.appspot.com/5862052/ Affected files: M .gitignore M flower/include/direction.hh M lily/accidental-placement.cc M lily/grob.cc M lily/include/grob-interface.hh M lily/include/note-collision.hh M lily/note-collision.cc M lily/note-column.cc M lily/staff-symbol-referencer.cc ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: some comments and complaints on the code (issue 5651069)
The next patch set was by mistake placed by me in a new issue: http://codereview.appspot.com/5862052. http://codereview.appspot.com/5651069/diff/1/lily/accidental-placement.cc File lily/accidental-placement.cc (right): http://codereview.appspot.com/5651069/diff/1/lily/accidental-placement.cc#newcode211 lily/accidental-placement.cc:211: * @return A vector of Accidental_placement_entrys On 2012/02/13 03:36:27, joeneeman wrote: On 2012/02/11 12:27:31, Milimetr88 wrote: Do you mean @param and @return or the comment to the function? What comment would you propose? I'm complaining about the @return comment, since it's redundant (ie. no need to change it, just remove that line). I'm ok with the @param comment, because you can't tell from the function signature that accs is supposed to be a list. Done. http://codereview.appspot.com/5651069/diff/1/lily/note-collision.cc File lily/note-collision.cc (right): http://codereview.appspot.com/5651069/diff/1/lily/note-collision.cc#newcode536 lily/note-collision.cc:536: s[LEFT]--; // why LEFT and RIGHT instead of UP and DOWN? On 2012/02/13 11:33:48, hanwenn wrote: sometimes i liked to think about intervals as lying on the horizontal line. Feel free to change to up/down. Done. http://codereview.appspot.com/5651069/diff/5002/flower/include/direction.hh File flower/include/direction.hh (right): http://codereview.appspot.com/5651069/diff/5002/flower/include/direction.hh#newcode67 flower/include/direction.hh:67: */ On 2012/02/13 11:33:48, hanwenn wrote: If you think there should be doxygen style commenting, rather than doing these one off, build consensus on the list, and fix all comments in the codebase in one go. I personally think they look terribly verbose, and are unpleasant to read in the sourcecode. Concise english sentences work better than @awkward comments to document things, IMO. Done. http://codereview.appspot.com/5651069/diff/5002/flower/include/direction.hh#newcode90 flower/include/direction.hh:90: */ On 2012/02/13 11:33:48, hanwenn wrote: if we decide to go ahead with this, there is no need to refer to the old style syntax, nor is it necessary to call the old one ugly. Done. http://codereview.appspot.com/5651069/diff/5002/lily/accidental-placement.cc File lily/accidental-placement.cc (right): http://codereview.appspot.com/5651069/diff/5002/lily/accidental-placement.cc#newcode116 lily/accidental-placement.cc:116: //TODO: add comment to this class On 2012/02/13 11:33:48, hanwenn wrote: drop Done. http://codereview.appspot.com/5651069/diff/5002/lily/accidental-placement.cc#newcode230 lily/accidental-placement.cc:230: //TODO: please add comment to this function On 2012/02/13 11:33:48, hanwenn wrote: this sets the skylines on the ape argument. Done. http://codereview.appspot.com/5651069/diff/5002/lily/accidental-placement.cc#newcode284 lily/accidental-placement.cc:284: //TODO: please add comment to this function On 2012/02/13 11:33:48, hanwenn wrote: this function does exactly what its name says it does: it extract the noteheads and stems from its argument. Done. http://codereview.appspot.com/5651069/diff/5002/lily/include/grob-interface.hh File lily/include/grob-interface.hh (right): http://codereview.appspot.com/5651069/diff/5002/lily/include/grob-interface.hh#newcode31 lily/include/grob-interface.hh:31: bool cl::has_interface (Grob *me) /* TODO: please add comment to this function */ \ On 2012/02/13 11:33:48, hanwenn wrote: either do it or leave, otherwise we could adorn the entire codebase with TODO. Done. http://codereview.appspot.com/5651069/diff/5002/lily/note-collision.cc File lily/note-collision.cc (right): http://codereview.appspot.com/5651069/diff/5002/lily/note-collision.cc#newcode191 lily/note-collision.cc:191: */ On 2012/02/12 01:29:14, Keith wrote: Protect the comment formatting with a column of '*'s Otherwise, someone might let emacs re-align the comment, as happened at line 543. Done. http://codereview.appspot.com/5651069/diff/5002/lily/note-collision.cc#newcode373 lily/note-collision.cc:373: update_offsets (offsets, clash_groups, shift_amount); On 2012/02/13 11:33:48, hanwenn wrote: there is not much value in abstracting into a function unless you can use the function at least twice. What I was taught on the university is to write short and simple functions that do only one thing. Kind of a measure can be a test if a function takes more than one screen. I know that this function is a small one, but it encloses a separate functionality. May I leave it or revert to previous state? http://codereview.appspot.com/5651069/diff/5002/lily/note-collision.cc#newcode528 lily/note-collision.cc:528: Drul_arrayvectorSlice extents; // vertical extends On 2012/02/13 11:33:48, hanwenn wrote: extents, not extends Done. http://codereview.appspot.com/5651069/diff/5002/lily/note-collision.cc#newcode536 lily/note-collision.cc:536: s[LEFT]--; // why LEFT and RIGHT instead of UP and
Re: Issue 2389 in lilypond: Patch: Fix make error in regressiontests coming from midi2ly
- Original Message - From: Carl Sorensen c_soren...@byu.edu To: Lily-Devel List lilypond-devel@gnu.org Sent: Wednesday, March 21, 2012 5:50 PM Subject: FW: Issue 2389 in lilypond: Patch: Fix make error in regressiontests coming from midi2ly On 3/20/12 9:08 PM, lilyp...@googlecode.com lilyp...@googlecode.com wrote: Updates: Labels: -Patch-countdown Patch-push Comment #11 on issue 2389 by ColinPKCampbell: Patch: Fix make error in regression tests coming from midi2ly http://code.google.com/p/lilypond/issues/detail?id=2389 Counted down to 20120320, please push. This patch has been tested on my system, but not by patchy-merge. It passed the tests before, but broke staging. In order to avoid messing up staging, I think it would be best to have this patch tested by patchy-merge as the only patch on staging. Could we set up a separate branch (e.g. staging-test) on which to test this patch? Thanks, Carl Clean here. Push to staging. -- Phil Holmes ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Two questions about vertical layout and lyrics spacing
On Wed, Mar 21, 2012 at 9:46 AM, Frank Steinmetzger war...@gmx.de wrote: On Sat, Mar 17, 2012 at 01:53:10PM -0700, Joe Neeman wrote: There are two competing desires here [...]. Could you suggest, therefore, an extra parameter (or a modification to the [vertical spacing] algorithm) that would give you the trade-off you want? Well, what came to my mind, being a (quite) seasoned user who's done a few dozens scores for my choir, in different styles and arrangements, is a variable to guide lily in its decisions, just like two-sided or ragged-* do. So for example a variable called spacing-priority with a few possible values, such as system-distance (what 2.14 does now), system-padding (what I would need in padding.png), or system-lyrics-padding (for the lyrics.png situation). Internally, I guess, setting this variable would just imply some other default values for the spacing alists, as Keith proposed. The reason behind this approach is that, while lilypond should do as much as possible automagically (which it is generally very good at), it just cannot predict every possible situation. So we're giving it a hint as to the direction in which to go. I am convinced that this is a wrong approach, because it complicates things (trying to create more options and exceptions to the current rules). Instead, i think we should create different spacing rules that better reflect human judgement of good spacing. I suggest that the vertical spacing alrogithm should calculate the area between the objects (staves, systems, lyrics etc). See here how it would work: http://www.freeimagehosting.net/tsr21 Please also look at these two scores of Tota pulchra es Maria: www.anonstorage.net/PStorage/871.tota-pulchra-default.pdf www.anonstorage.net/PStorage/993.tota-pulchra-reduced-padding.pdf In the first one (created with all default spacing settings), there are places where a single dynamic object pushes the staves far apart (marked with light blue). See pages 2 and 4 - there is too much space between the staves, especially compared with space between systems. It is important to keep systems visually separate - the last page is completely illegible in this regard. When i reduce padding between a lyric line and the staff below it, the spacing between systems is definitely improved. However, some lyrics are now too close to the staves below them (because of the reduced padding) - see the other pdf file, light blue markings again. This problem isn't very big, but it's noticeable (especially when a lyric line gets closer to the unrelated staff than to the related staff). I think that's impossible to achieve the desired effect (very small padding between lyrics and a single sticking-out dynamic, but overall not too small distance between lyrics and unrealted staff) with current spacing system. The area system, however, will be able to produce desired result, because it takes into account the amount of actual overall whitespace between objects, instead of focusing on the points where objects (staff and lyrics) are closest to themselves. How do you like it? cheers, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 1320: Rewriting bar-line::print
Le 20 mars 2012 à 09:39, Marc Hohl a écrit : Hello list, I want to rewrite most if not all definitions currently settled in lily/bar-line.cc in scheme. Please see the attached file for my progress so far; I don't get any error messages, but no bar lines either :-( Is this a feasible approach? What am I currently doing wrong? I've done something like that, to create baroque repeat bars, and suggested repeat bars: https://github.com/nsceaux/nenuvar/blob/master/common/custom-bars.ily Nicolas ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Two questions about vertical layout and lyrics spacing
I suggest that the vertical spacing alrogithm should calculate the area between the objects (staves, systems, lyrics etc). See here how it would work: http://www.freeimagehosting.net/tsr21 Looks right: The area between horizontal skylines should influence the spacing. However, I think we need a parameter to adjust the amount of influence. Werner ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
vertical spacing - skyline integrals (was: Two questions about ...)
On Wed, Mar 21, 2012 at 9:41 PM, Werner LEMBERG w...@gnu.org wrote:
I suggest that the vertical spacing alrogithm should calculate the
area between the objects (staves, systems, lyrics etc). See here
how it would work: http://www.freeimagehosting.net/tsr21
Looks right: The area between horizontal skylines should influence the
spacing. However, I think we need a parameter to adjust the amount of
influence.
I'm not sure what you mean.
I suggest that this would be added to spacing alists, for example
\paper {
system-system-spacing #'average-gap = #4
}
would mean that the desired whitespace area is 4 staffspace * line width.
Actually, i think that it may be possible to remove basic-distance and
minimum-distance after area spacing is implemented. In other words,
i think average-gap, padding and stretchability could turn out to be
enough parameters for vertical spacing purposes, with average-gap
taking the role of basic-distance. But that will show in testing, we
need to have a working prototype first (Joe said he will do this).
I've identified two potential problems that may appear, and ideas for solutions:
www.anonstorage.net/PStorage/621.skyline-integrals-teeth-marked.pdf
www.anonstorage.net/PStorage/328.integrals-pitfall-two-marked.pdf
Are the descriptions clear enough?
cheers,
Janek
PS Joe, how did you like my Credo example?
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: vertical spacing - skyline integrals
Looks right: The area between horizontal skylines should influence the
spacing. However, I think we need a parameter to adjust the amount of
influence.
I'm not sure what you mean.
I mean that some people probably want a more rigid spacing, only
slightly influenced by the area inbetween. This is a matter of taste
I suggest that this would be added to spacing alists, for example
\paper {
system-system-spacing #'average-gap = #4
}
would mean that the desired whitespace area is 4 staffspace * line
width.
Yes, but I would like to have an additional parameter, say,
system-system-spacing #'rigidity = X
where X is a number between 0 and 1. Value 1 essentially means no
skyline integrals, while value 0 exactly the opposite.
I've identified two potential problems that may appear, and ideas
for solutions:
www.anonstorage.net/PStorage/621.skyline-integrals-teeth-marked.pdf
www.anonstorage.net/PStorage/328.integrals-pitfall-two-marked.pdf
A possible solution is to `dampen' the skyline for the area
computation, this is, to apply the beam logic to the skyline. For
example, this
--
|
|
|
|
-
becomes this:
-
\
\
\
\
Werner
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Verifications for 2.15.34 release are complete
No remaining issues to verify for the 2.15.34 release. Cheers, Colin. -- Colin Hall ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Don't reload initialization files when processing multiple files (issue 5874044)
This feels like a dangerous change. Obviously we don't /want/ bleed-through of parameters or memory from one file to the next, but I would be shocked if we don't have any of that right now. I also have vague memories of some problems with this in the past. I guesstimate that not reloading init files will expose 2-5 critical regressions even after the regtests themselves compile ok. Note that I use the word expose, not create or cause -- the bugs are there already. We'll just get reports dribbling in over a few weeks. If you want 2.16 to be as good as possible, then speeding up the regtest compile and exposing those potential bugs is certainly a good thing. If you want 2.16 to be out soon, then you may want to consider sitting on this patch until 2.16 is out. I'm content with either option. http://codereview.appspot.com/5874044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
