Re: Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)

2012-12-31 Thread dak

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: PO: remove duplicates entries for hh and cc from ALL_PO_SOURCES (issue 7029043)

2012-12-31 Thread benko . pal

LGTM

https://codereview.appspot.com/7029043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Doc: NR 5.3 Add \single command (issue 6742057)

2012-12-31 Thread graham


https://codereview.appspot.com/6742057/diff/7001/Documentation/notation/changing-defaults.itely
File Documentation/notation/changing-defaults.itely (right):

https://codereview.appspot.com/6742057/diff/7001/Documentation/notation/changing-defaults.itely#newcode1626
Documentation/notation/changing-defaults.itely:1626: * Set and unset::
On 2012/12/30 09:14:20, dak wrote:

Not sure whether @code{...}
can be used within section titles (possibly causing problems in PDF

indices?).

If it can, using @code{\set} etc would be appropriate.


It can't be used for node names, but it _can_ be used in the section
titles, whcih is the part that generates the visible output.  We already
use this trick, and James keeps on using it in the patch.

https://codereview.appspot.com/6742057/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: IR: Improve performer documentation (issue 6868047)

2012-12-31 Thread src

David,

On 2012/12/02 11:11:54, dak wrote:

https://codereview.appspot.com/6868047/diff/1/scm/document-translation.scm

File scm/document-translation.scm (right):


Sorry for resurrecting this a month late, I think I only quite
understood yesterday what you actually meant. It is somewhat obsolete
for this patch, since I have gone on to work on a more complete one (see
http://code.google.com/p/lilypond/issues/detail?id=3003#c3), but your
point will be relevant for my new work again, so I guess I better start
defending my case now.


https://codereview.appspot.com/6868047/diff/1/scm/document-translation.scm#newcode21

scm/document-translation.scm:21: ;; type predicates ly:engraver? and
ly:performer?.
I think that this approach is not really helping as it is inflexible

and it

becomes harder to see crossovers.  At one point of time, we will

likely get

translators that are used to output MusicXML, and several translators

can be

used as either translator or performer (and some, like the autobeamer,

will also

be used in MusicXML preparation).  So I'd rather make separate

chapters for

translators used in $defaultlayout (default engravers) and those used

in

$defaultmidi (default performers), allow for duplicates but mention

when the

engraver is also being used as performer and vice versa, and make a

separate

list of translators not used by default (are there any?).


Classifying translators by the output definitions in which they appear
by default is not a good idea in my opinion.

* Translators do not need to belong to any context by default, since
they may implement optional functionality. Yes, there are already some,
for example Ambitus_engraver, Completion_heads_engraver,
Grid_line_span_engraver or Page_turn_engraver.

These still clearly belong to one particular backend; all current
examples to the graphical one, but I see no reason to assume there will
never be optional performers (and, in the future, XML transcribers,
however you will call them) too. I don't think lumping these all
together in a separate category makes any sense.

* Allowing for duplicate translator documentation nodes would not be
logical. Currently this would affect only Timing_translator, but again I
see no reason to assume this will always remain the case.

Each translator is uniquely defined, has a single set of associated
documentation data and, judging from Timing_translator, will work
identically wherever it makes sense to \consist it. Specifically, if a
translator can appear in more than one output definition, it will
operate on a backend-independent level. Then why document it at
backend-specific levels, with duplicated nodes that would have to have
identical text?

[The situation is different for contexts. Layout and MIDI contexts of
the same name have separate definitions with separate docstrings, since
each output definition comes with its own complete context hierarchy.
They are mostly isomorphic, but conceptually this is just a manually
enforced convention to allow you to use the same LilyPond score
definitions for both graphical and MIDI output.

Deviations already exist: GregorianTranscriptionStaff,
GregorianTranscriptionVoice, NoteNames, PetrucciStaff and PetrucciVoice
are defined as layout contexts, but not as MIDI contexts; ChordNameVoice
exists as a MIDI context only. Some of these might actually be
oversights (?), but probably not all.

That it why, in my current personal development version of the Internals
Reference, I have started to document the context hierarchies
separately. You can have a look at the current state at
http://src.johannesrohrer.de/lilypond/Documentation/internals/contexts.html.]


My opinion is that you should classify translators based on what they do
(if at all), and looking for where you \consist them by default is not a
reliable way to find this out. The best ones I currently see are still
to either look at a translator's base class (requires new specific type
predicates, which I hesitate to add myself), or at its name. Hence I
stand by my original code.


Best regards, and happy new year to everyone,

Johannes Rohrer

https://codereview.appspot.com/6868047/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Issue 3073: Context definitions and context mods ignore \unset (issue 7038044)

2012-12-31 Thread lemzwerg

LGTM

https://codereview.appspot.com/7038044/

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

2012-12-31 Thread david . nalesnik

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: Issue 3073: Context definitions and context mods ignore \unset (issue 7038044)

2012-12-31 Thread pkx166h


https://codereview.appspot.com/7038044/diff/1/input/regression/property-unset.ly
File input/regression/property-unset.ly (right):

https://codereview.appspot.com/7038044/diff/1/input/regression/property-unset.ly#newcode8
input/regression/property-unset.ly:8: systems here should revert to the
@samp{Score}-level violin clef.
This fails make check, I am wondering if there needs to be some
'escaping' of texinfo command chars? or is it the space between @code
and {\\unset}?

https://codereview.appspot.com/7038044/

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

2012-12-31 Thread thomasmorley65

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


invalid read in Context::properties_dict()

2012-12-31 Thread Dan Eble
I have a reproducible segfault, but not a tiny example.  (If anyone wants to 
reproduce this, ask me to prepare a tgz for you to download from my web site.)  
Is there an expert here who can tell me how likely it is that the Context 
properties actually referred to the object released in Skyline::Skyline, or if 
something more obscure happened?

This is with the latest source built in LilyDev, but the same input causes a 
bus error with 2.16 on OS X.  Valgrind and gdb output follow.

Thanks,
-- 
Dan  

VALGRIND OUTPUT (ABRIDGED)

==13752== Invalid read of size 4
==13752==at 0x80CA0E9: Context::properties_dict() const (context.cc:68)
==13752==by 0x80CBC53: Context::internal_get_property(scm_unused_struct*) 
const (context.cc:445)
==13752==by 0x80CBC91: Context::internal_get_property(scm_unused_struct*) 
const (context.cc:449)
==13752==by 0x80CBC91: Context::internal_get_property(scm_unused_struct*) 
const (context.cc:449)
==13752==by 0x80CBC91: Context::internal_get_property(scm_unused_struct*) 
const (context.cc:449)
==13752==by 0x80C5DF3: ly_context_property(scm_unused_struct*, 
scm_unused_struct*, scm_unused_struct*) (context-scheme.cc:104)
==13752==by 0x4094759: scm_gsubr_apply (in /usr/lib/libguile.so.17.3.1)
==13752==by 0x407DB73: scm_dapply (in /usr/lib/libguile.so.17.3.1)
==13752==by 0x407E726: ??? (in /usr/lib/libguile.so.17.3.1)
==13752==by 0x407F02C: ??? (in /usr/lib/libguile.so.17.3.1)
==13752==by 0x407DBEA: scm_dapply (in /usr/lib/libguile.so.17.3.1)
==13752==by 0x407C507: scm_apply (in /usr/lib/libguile.so.17.3.1)
==13752==  Address 0xcc33d90 is 32 bytes inside a block of size 40 free'd
==13752==at 0x4024851: operator delete(void*) (vg_replace_malloc.c:387)
==13752==by 0x824F921: Skyline::Skyline(std::vectorSkyline_pair, 
std::allocatorSkyline_pair  const, Direction) (new_allocator.h:95)
==13752==by 0x824881C: Skyline_pair::Skyline_pair(std::vectorSkyline_pair, 
std::allocatorSkyline_pair  const) (skyline-pair.cc:42)
==13752==by 0x807292E: Axis_group_interface::skyline_spacing(Grob*, 
std::vectorGrob*, std::allocatorGrob* ) (axis-group-interface.cc:884)
==13752==by 0x807351D: 
Axis_group_interface::calc_skylines(scm_unused_struct*) 
(axis-group-interface.cc:396)
==13752==by 0x407D63E: scm_dapply (in /usr/lib/libguile.so.17.3.1)
==13752==by 0x407C507: scm_apply (in /usr/lib/libguile.so.17.3.1)
==13752==by 0x4081A40: scm_call_1 (in /usr/lib/libguile.so.17.3.1)
==13752==by 0x811C1BB: Grob::try_callback_on_alist(scm_unused_struct**, 
scm_unused_struct*, scm_unused_struct*) (grob-property.cc:231)
==13752==by 0x811C392: Grob::internal_get_property(scm_unused_struct*) 
const (grob-property.cc:188)
==13752==by 0x806F544: Axis_group_interface::generic_group_extent(Grob*, 
Axis) (axis-group-interface.cc:440)
==13752==by 0x806F5B8: Axis_group_interface::height(scm_unused_struct*) 
(axis-group-interface.cc:365)

GDB OUTPUT (ABRIDGED)

Calculating page and line breaks (20 possible page 
breaks)...[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20]
Drawing systems...
Program received signal SIGSEGV, Segmentation fault.
0x080cd5be in Scheme_hash_table::unsmob (s=0x30) at 
/home/dan/lilypond-git/lily/include/scm-hash.hh:62
62DECLARE_SMOBS (Scheme_hash_table);
(gdb) print *this
No symbol this in current context.
(gdb) up
#1  0x080ca0f4 in Context::properties_dict (this=0x8d4daa8) at 
/home/dan/lilypond-git/lily/context.cc:68
68return Scheme_hash_table::unsmob (properties_scm_);
(gdb) print *this
$1 = {_vptr.Context = 0x38254fb6, static smob_name_ = 0x8329c00 Context, 
static smob_tag_ = 10623, 
  self_scm_ = 0x400dabb6, protection_cons_ = 0x689174e, client_count_ = 
1074802671, infant_event_ = 0xdbbf9fb1, 
  daddy_context_ = 0x40104870, definition_ = 0x0, definition_mods_ = 0x0, 
properties_scm_ = 0x30, 
  context_list_ = 0x31, accepts_list_ = 0x8d4db00, aliases_ = 0x9239790, 
implementation_ = 0x0, id_string_ = {
static npos = 4294967295, 
_M_dataplus = {std::allocatorchar = {__gnu_cxx::new_allocatorchar = 
{No data fields}, No data fields}, 
  _M_p = 0x8000 Address 0x8000 out of bounds}}, event_source_ = 
0xa99b6f5c, events_below_ = 0x3ff50f10, 
  ancestor_lookup_ = 0x2ed35222}
(gdb) up
#2  0x080cbc54 in Context::internal_get_property (this=0x8d4daa8, 
sym=0xb7306af0)
at /home/dan/lilypond-git/lily/context.cc:445
445   if (properties_dict ()-try_retrieve (sym, val))
(gdb) print *this
$2 = {_vptr.Context = 0x38254fb6, static smob_name_ = 0x8329c00 Context, 
static smob_tag_ = 10623, 
  self_scm_ = 0x400dabb6, protection_cons_ = 0x689174e, client_count_ = 
1074802671, infant_event_ = 0xdbbf9fb1, 
  daddy_context_ = 0x40104870, definition_ = 0x0, definition_mods_ = 0x0, 
properties_scm_ = 0x30, 
  context_list_ = 0x31, accepts_list_ = 0x8d4db00, aliases_ = 0x9239790, 
implementation_ = 0x0, id_string_ = {
static npos = 4294967295, 
_M_dataplus = 

Re: invalid read in Context::properties_dict()

2012-12-31 Thread Dan Eble
When the segfault occurs, multiple contexts in the call stack have already had 
their mark_smob() method called.  I guess that means they're all invalid, and 
their memory might have been reallocated.  Is that correct?

For a while, I have been using the following scheme code to define instrument 
names for voices together with the notes for the voice, e.g.

aNotes = \relative g {
  #(set-voice-name T1)
  c4 d e f . . .
}

Did something change in 2.16 that would make this start to segfault, or was it 
always invalid and just now started failing by chance?  Thanks.

#(define-markup-command (on-the-fly-context layout props procedure context arg) 
(symbol? markup?)
  Apply the @var{procedure} markup command to
@var{arg}. @var{procedure} should take a single argument.
  (let* ((anonymous-with-signature (lambda (layout props context arg) 
(procedure layout props context arg
(set-object-property! anonymous-with-signature
  'markup-signature
  (list ly:context? markup?))
(interpret-markup layout props (list anonymous-with-signature context 
arg

#(define (set-voice-name newName)
   (let ((m (make-music 'ApplyContext)))

 (define (do-it context)
   (let* ((voiceNames (ly:context-property context 'voiceNames)))

 (if (list? voiceNames)
 (set! voiceNames (append voiceNames (list newName)))
 (set! voiceNames (list newName)))

 (ly:context-set-property! context 'voiceNames voiceNames)))

 (set! (ly:music-property m 'procedure) do-it)

 (context-spec-music m 'Staff)))

#(define-public (print-voice-names-or-default layout props context default)
   (let ((voiceNames (ly:context-property context 'voiceNames)) % segfault 
occurs in ly:context-property
 (props (cons (list '(baseline-skip . 2.4)) props)))
 (if (null? voiceNames)
 (interpret-markup layout props (make-column-markup default))
 (interpret-markup layout props (make-column-markup voiceNames)

#(define-public (set-auto-instrument-name defaultNames)
   (let ((m (make-music 'ApplyContext)))

 (define (do-it context)
 (ly:context-set-property!
  context 'instrumentName
  (cons on-the-fly-context-markup
(list print-voice-names-or-default context defaultNames

 (set! (ly:music-property m 'procedure) do-it)
 (context-spec-music m 'Staff)))

-- 
Dan


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel