Re: Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)
author Thomas Morley thomasmorle...@googlemail.com Mon, 7 Jan 2013 15:37:33 + (15:37 +) committer James Lowe pkx1...@gmail.com Mon, 7 Jan 2013 15:39:54 + (15:39 +) commitfa9c6713e581994ed2882a0d447f40452caa713f I assumed Thomas didn't have push privileges based on last few commits by other devs. So have pushed this myself. Please close Rietveld Issue. Thank you. James https://codereview.appspot.com/7029045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)
On 2012/12/31 00:07:12, thomasmorley65 wrote:
I thought about extending \draw-line with a 'style-property.
But in extreme cases you would have to write:
\markup {
\override #'(style . dashed)
\override #'(on . 0.5)
\override #'(off . 0.2)
\override #'(thickness . 2)
\draw-line #'(4 . 1)
}
This seems a bit overlaoded to me.
I don't consider
\markup {
\override #'(on . 0.5)
\override #'(off . 0.2)
\override #'(thickness . 2)
\draw-dashed-line #'(4 . 1)
}
considerably less overloaded.
Ok, this will reduce the needed \overrides only by one, but I think
any \override less would be nice.
Hm.
And I'm thinking about writing additional commands for zigzag- and
trill-style lines...
After all,
lines are not the only things that might be desirable to do with a
different line style.
Do you have anything particular in mind?
Boxes, circles, underlines, ... No, nothing particular, but it's not
uncommon.
https://codereview.appspot.com/7029045/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)
Harm--
This looks great! Thank you for the 'full-length option. I can't be
alone in hating lines ending with incomplete dashes.
All I have is a suggestion or two, and some quibbles.
Besides what I've pointed out inline, I should mention that I got a
number of whitespace errors when I applied your patch. There are a
number of spots where you should trim the ends of lines (mostly in the
.scm file).
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm
File scm/define-markup-commands.scm (right):
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode155
scm/define-markup-commands.scm:155: If @code{full-length} is set
@code{#t} (default) the dashed-line extends to the
set to #t
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode156
scm/define-markup-commands.scm:156: whole length given by @var{dest},
without white space at begin/end.
beginning or end.
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode169
scm/define-markup-commands.scm:169: (let* (;; Get the line-thickness.
I think your variable name is sufficient--no comment needed.
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode181
scm/define-markup-commands.scm:181: (begin
Branches of if expression should be aligned with test (here and
elsewhere).
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode195
scm/define-markup-commands.scm:195: (new-off (/ (- line-length corr (*
(+ 1 guess) on)) guess))
Why not use (1+ guess)
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode231
scm/define-markup-commands.scm:231: #f)
Could omit the boolean--value will be unspecified.
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode233
scm/define-markup-commands.scm:233: ;; If `on´ or `off´ is negative, or
both, `on´ and `off´ equals zero a
Possibly ...or the sum of `on' and `off' equals [is] zero...
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode244
scm/define-markup-commands.scm:244: #f)
Here again, you could omit the alternate.
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode248
scm/define-markup-commands.scm:248: (interval-widen (ordered-cons 0 x)
half-thick)
You do such a thorough job of commenting everything, but you don't
explain why you need `half-thick' here.
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode264
scm/define-markup-commands.scm:264: Manual settings for @code{off} is
possible to get larger or smaller space
are possible
https://codereview.appspot.com/7029045/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)
On 2012/12/31 14:59:45, david.nalesnik wrote:
Harm--
This looks great! Thank you for the 'full-length option. I can't be
alone in
hating lines ending with incomplete dashes.
Hi David,
thanks for reviewing!
All I have is a suggestion or two, and some quibbles.
Besides what I've pointed out inline, I should mention that I got a
number of
whitespace errors when I applied your patch. There are a number of
spots where
you should trim the ends of lines (mostly in the .scm file).
Done. (Hope so)
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm
File scm/define-markup-commands.scm (right):
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode155
scm/define-markup-commands.scm:155: If @code{full-length} is set
@code{#t}
(default) the dashed-line extends to the
set to #t
Done
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode156
scm/define-markup-commands.scm:156: whole length given by @var{dest},
without
white space at begin/end.
beginning or end.
Done
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode169
scm/define-markup-commands.scm:169: (let* (;; Get the line-thickness.
I think your variable name is sufficient--no comment needed.
Ok. Deleted.
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode181
scm/define-markup-commands.scm:181: (begin
Branches of if expression should be aligned with test (here and
elsewhere).
Done
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode195
scm/define-markup-commands.scm:195: (new-off (/ (- line-length corr (*
(+ 1
guess) on)) guess))
Why not use (1+ guess)
Done
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode231
scm/define-markup-commands.scm:231: #f)
Could omit the boolean--value will be unspecified.
Done
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode233
scm/define-markup-commands.scm:233: ;; If `on´ or `off´ is negative,
or both,
`on´ and `off´ equals zero a
Possibly ...or the sum of `on' and `off' equals [is] zero...
Changed
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode244
scm/define-markup-commands.scm:244: #f)
Here again, you could omit the alternate.
Done
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode248
scm/define-markup-commands.scm:248: (interval-widen (ordered-cons 0 x)
half-thick)
You do such a thorough job of commenting everything, but you don't
explain why
you need `half-thick' here.
Comment added.
https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode264
scm/define-markup-commands.scm:264: Manual settings for @code{off} is
possible
to get larger or smaller space
are possible
Done.
https://codereview.appspot.com/7029045/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)
Reviewers: , Message: Please review Description: Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. Adding a reg-test for them. Please review this at https://codereview.appspot.com/7029045/ Affected files: A input/regression/markup-line-styles.ly M scm/define-markup-commands.scm ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)
Excuse my ignorance but is the \xxx-xxx-xxx ok in terms of consistency of name of command and/or is 'draw' redundant? https://codereview.appspot.com/7029045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)
On 2012/12/30 16:30:45, J_lowe wrote: Excuse my ignorance but is the \xxx-xxx-xxx ok in terms of consistency of name of command and/or is 'draw' redundant? \line arranges a text line, \draw-line draws an actual line, so the name choice seems ok. Another possibility would be to use properties to specify dotting/dashing and just use \draw-line (or \draw-circle and so on). After all, lines are not the only things that might be desirable to do with a different line style. https://codereview.appspot.com/7029045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)
On 2012/12/30 16:42:56, dak wrote:
On 2012/12/30 16:30:45, J_lowe wrote:
Excuse my ignorance but is the \xxx-xxx-xxx ok in terms of
consistency of name
of command and/or is 'draw' redundant?
\line arranges a text line, \draw-line draws an actual line, so the
name choice
seems ok. Another possibility would be to use properties to specify
dotting/dashing and just use \draw-line (or \draw-circle and so on).
I thought about extending \draw-line with a 'style-property.
But in extreme cases you would have to write:
\markup {
\override #'(style . dashed)
\override #'(on . 0.5)
\override #'(off . 0.2)
\override #'(thickness . 2)
\draw-line #'(4 . 1)
}
This seems a bit overlaoded to me. Therefore I decided to define
separate markup-commands.
Ok, this will reduce the needed \overrides only by one, but I think any
\override less would be nice.
And I'm thinking about writing additional commands for zigzag- and
trill-style lines...
After all,
lines are not the only things that might be desirable to do with a
different
line style.
Do you have anything particular in mind?
https://codereview.appspot.com/7029045/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
