Re: New format for autobeaming rules

2009-07-24 Thread Carl Sorensen 



On 7/22/09 4:15 PM, joenee...@gmail.com joenee...@gmail.com wrote:

 I think this is pretty much ready to commit
 
 
 http://codereview.appspot.com/88155/diff/3101/4032
 File lily/beam-scheme.cc (right):
 
 http://codereview.appspot.com/88155/diff/3101/4032#newcode2
 Line 2: beam-scheme.cc -- Retrieving beam settings
 could you call this beam-grouping-scheme.cc or something like that?
 beam-scheme sounds like it contains routines for manipulating Beam
 grobs.

Changed to beam-setting-scheme.cc.  Changed beam-grouping.hh to
beam-settings.hh.

 
 http://codereview.appspot.com/88155/diff/3101/4032#newcode12
 Line 12: LY_DEFINE (ly_beam_settings, ly:beam-settings,
 is this function really necessary?

Probably not.  Instead of ly_beam_settings(context) we can do
context-get_property(beamSettings); there's no error checking in the
current function.  So I guess I'll drop it.

 
 http://codereview.appspot.com/88155/diff/3101/4032#newcode49
 Line 49: ly_grouping_rules(settings,time_signature,rule_type),
 formatting
Fixed.

 
 http://codereview.appspot.com/88155/diff/3101/4032#newcode61
 Line 61: SCM settings = ly_beam_settings(context);
 formatting

Fixed
 
 http://codereview.appspot.com/88155
On 7/22/09 5:23 PM, n.putt...@gmail.com n.putt...@gmail.com wrote:

 
 
 http://codereview.appspot.com/88155/diff/3101/4027
 File input/new/grouping-beats.ly (right):
 
 http://codereview.appspot.com/88155/diff/3101/4027#newcode7
 Line 7: Beaming patterns may be altered with the @code{beatGrouping}
 property:
 new texidoc using \overrideBeamSettings
 
 http://codereview.appspot.com/88155/diff/3101/4032
 File lily/beam-scheme.cc (right):
 
 http://codereview.appspot.com/88155/diff/3101/4032#newcode10
 Line 10: #include beam-grouping.hh
 swap

Fixed

 
 http://codereview.appspot.com/88155/diff/3101/4032#newcode26
 Line 26:  @var{rule-type} in @var{context}.)
 no context arg, doc settings
 

Fixed, 2 places (ly_grouping_rules and ly_beam_grouping).

 http://codereview.appspot.com/88155/diff/3101/4032#newcode28
 Line 28: LY_ASSERT_TYPE (ly_is_pair, time_signature, 2);
 scm_is_pair

Fixed 
 
 http://codereview.appspot.com/88155/diff/3101/4032#newcode30
 Line 30:
 type check also for settings?

Settings needs a list? type check, and I haven't seen one available
in c++.  It shouldn't segfault, because we do a pair check before we
use it.

I can't use a pair check for the argument, because '() is valid for
settings.

I could use pair_or_empty, but it doesn't exist, and when I tried to
add it to flower/include/guile-compatibility.hh, where all the rest of
the types are defined, it gave me errors.

I'll put a FIXME in.

So I'm not doing a type check for settings, at least right now.

 
 http://codereview.appspot.com/88155/diff/3101/4032#newcode34
 Line 34: ly_assoc_get ((scm_list_2 (time_signature, rule_type)),
 excess parens

D'OH!  I'm not in scheme anymore!
Fixed.
 
 http://codereview.appspot.com/88155/diff/3101/4032#newcode43
 Line 43: Return grouping for beams of @var{ beam-type} in
 @var{beam-type}
 

Fixed

 http://codereview.appspot.com/88155/diff/3101/4032#newcode45
 Line 45:  @var{rule-type} in @var{context}.)
 no context arg, doc settings

Fixed

 
 http://codereview.appspot.com/88155/diff/3101/4032#newcode46
 Line 46: {
 type checks?

Put in for time_signature, rule_type.

Can't do one for beam_type, because it needs to be pair-or-symbol,
and I couldn't figure out how to add it.

I don't think it would segfault, because it's not dereferenced.

I'll put a FIXME in.

 
 http://codereview.appspot.com/88155/diff/3101/4032#newcode57
 Line 57: {
 LY_ASSERT_SMOB (Context, context, 1);
 

Added.

 If you don't do this, then unsmob_context () will return NULL if this
 function is passed invalid arguments, leading to a null dereference for
 get_property (timeSignatureFraction) - segfault

Thanks for teaching me about this.

 
 http://codereview.appspot.com/88155/diff/3101/4033
 File lily/beaming-pattern.cc (right):
 
 http://codereview.appspot.com/88155/diff/3101/4033#newcode18
 Line 18: #include beam-grouping.hh
 sort

OK, done
 
 http://codereview.appspot.com/88155/diff/3101/4034
 File lily/include/beam-grouping.hh (right):
 
 http://codereview.appspot.com/88155/diff/3101/4034#newcode8
 Line 8:
 To prevent multiple includes:
 
 #ifndef BEAM_GROUPING_HH
 #define BEAM_GROUPING_HH
 
 (+ line 14)
 
 http://codereview.appspot.com/88155/diff/3101/4034#newcode14
 Line 14:
 #endif // BEAM_GROUPING_HH
 

OK, done.

 http://codereview.appspot.com/88155/diff/3101/4035
 File lily/measure-grouping-engraver.cc (right):
 
 http://codereview.appspot.com/88155/diff/3101/4035#newcode14
 Line 14: #include beam-grouping.hh
 sort

When I try to sort, it breaks compile because SCM is not defined
when beam-grouping.hh is included.

The best thing to do would be to include the proper file to define
SCM if it hasn't already been loaded.

But I couldn't find the header file that defined SCM through git
grep.  Do you know which file I need to include?

Re: New format for autobeaming rules

2009-07-24 Thread Neil Puttock 
2009/7/24 Carl Sorensen c_soren...@byu.edu:

 type check also for settings?

 Settings needs a list? type check, and I haven't seen one available
 in c++.  It shouldn't segfault, because we do a pair check before we
 use it.

ly_is_list (which is a wrapper for scm_list_p) (but see below)


 I can't use a pair check for the argument, because '() is valid for
 settings.

 I could use pair_or_empty, but it doesn't exist

You could use ly_cheap_is_list, which does exactly this.

, and when I tried to
 add it to flower/include/guile-compatibility.hh, where all the rest of
 the types are defined, it gave me errors.

You'd need to add it to lily-guile.hh.  guile-compatibility.hh is for
compatibility with guile 1.6.

 type checks?

 Put in for time_signature, rule_type.

 Can't do one for beam_type, because it needs to be pair-or-symbol,
 and I couldn't figure out how to add it.

The long way:

Add this to lily-guile.hh,

inline bool ly_is_symbol_or_pair (SCM x) { return scm_is_symbol (x) ||
scm_is_pair (x); }

and this to init_func_doc () in function-documentation.cc,

ly_add_type_predicate ((void*) ly_is_symbol_or_pair, symbol or pair);

then you can use LY_ASSERT_TYPE with the new predicate:

LY_ASSERT_TYPE (ly_is_symbol_or_pair, beam_type, 4);

The shorter way (and better since this is unlikely to be used
elsewhere) is to do this:

SCM_ASSERT_TYPE (scm_is_symbol (beam_type) || scm_is_pair (beam_type),
   beam_type, SCM_ARG4, __FUNCTION__, symbol or pair);

 sort

 But I couldn't find the header file that defined SCM through git
 grep.  Do you know which file I need to include?

I think it's libguile.h, which is included in lily-guile.hh.

The following works for me:

beam-setting-scheme.cc:

#include beam-settings.hh
#include context.hh

beam-settings.hh:

(inside #define)

#include lily-guile.hh

 decided not to restore original setting?

 I had decided to restore it in a different form.  1/8 beams are (6), which
 is equivalent to (3) in (3 . 4).  All shorter beams will be (1 1 1).

Ah, so it is.  I'll get the hang of the new settings eventually. :)

Regards,
Neil


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

Re: New format for autobeaming rules

2009-07-22 Thread Carl . D . Sorensen 

Reviewers: Neil Puttock, joeneeman,

Message:
OK, I've responded to Joe's comments.

I've created a new file lily/beam-scheme.cc, and put c++ and scheme
callable routines in it.  Then I've used those calls from auto-beam.scm,
measure-grouping-engraver.cc, and beaming-pattern.cc.  I also added a
new file lily/include/beam-grouping.hh to provide those functions to
c++.

I've never done this much mucking in the c++ code before, so please make
sure I've done things right.

Thanks,

Carl



http://codereview.appspot.com/88155/diff/3092/2039
File ly/music-functions-init.ly (right):

http://codereview.appspot.com/88155/diff/3092/2039#newcode699
Line 699: (make-music 'SequentialMusic 'void #t))
On 2009/07/21 18:24:35, joeneeman wrote:

I'd feel better if this were finished. At least add FIXME so it can be

easily

grepped for.


OK -- fixed in new patch set.

http://codereview.appspot.com/88155/diff/3092/2047
File scm/music-functions.scm (right):

http://codereview.appspot.com/88155/diff/3092/2047#newcode546
Line 546: (define (make-default-beaming-rule context)
On 2009/07/21 18:24:35, joeneeman wrote:

this doesn't seem to be used...

Thanks for the catch -- removed

Description:
New format for autobeaming rules
Change autobeaming rules so they are all set in one place and they
remain if the time signature is changed.

Eliminates override-auto-beam-settings and revert-auto-beam-settings

autoBeamSettings property changed to beamSettings property

beatGrouping property eliminated (default autobeam rule is now
used for beatGrouping)

Please review this at http://codereview.appspot.com/88155

Affected files:
  M Documentation/de/user/rhythms.itely
  M Documentation/es/user/rhythms.itely
  M Documentation/fr/user/rhythms.itely
  M Documentation/topdocs/NEWS.tely
  M Documentation/user/music-glossary.tely
  M Documentation/user/rhythms.itely
  M input/lsr/automatic-beams-two-per-two-in-4-4-or-2-2-time-signature.ly
  M input/lsr/beam-endings-in-score-context.ly
  M input/lsr/beam-grouping-in-7-8-time.ly
  M input/lsr/chordchanges-for-fretboards.ly
  M input/lsr/compound-time-signatures.ly
  M input/lsr/conducting-signs,-measure-grouping-signs.ly
  M input/lsr/grouping-beats.ly
  M input/lsr/making-slurs-with-complex-dash-structure.ly
  M input/lsr/non-default-tuplet-numbers.ly
  M input/lsr/non-traditional-key-signatures.ly
  M input/lsr/reverting-default-beam-endings.ly
  M input/manual/fretted-headword.ly
  M input/mutopia/claop.py
  A input/new/beam-endings-in-score-context.ly
  A input/new/beam-grouping-in-7-8-time.ly
  A input/new/compound-time-signatures.ly
  A input/new/conducting-signs,-measure-grouping-signs.ly
  A input/new/grouping-beats.ly
  A input/new/reverting-default-beam-endings.ly
  M input/regression/auto-beam-beat-grouping.ly
  M input/regression/beam-beat-grouping.ly
  M lily/auto-beam-engraver.cc
  A lily/beam-scheme.cc
  M lily/beaming-pattern.cc
  A lily/include/beam-grouping.hh
  M lily/measure-grouping-engraver.cc
  M ly/bagpipe.ly
  M ly/engraver-init.ly
  M ly/music-functions-init.ly
  M python/convertrules.py
  M scm/auto-beam.scm
  A scm/beam-settings.scm
  M scm/c++.scm
  M scm/define-context-properties.scm
  M scm/define-music-display-methods.scm
  M scm/lily.scm
  M scm/music-functions.scm




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

Re: New format for autobeaming rules

2009-07-22 Thread joeneeman 

I think this is pretty much ready to commit


http://codereview.appspot.com/88155/diff/3101/4032
File lily/beam-scheme.cc (right):

http://codereview.appspot.com/88155/diff/3101/4032#newcode2
Line 2: beam-scheme.cc -- Retrieving beam settings
could you call this beam-grouping-scheme.cc or something like that?
beam-scheme sounds like it contains routines for manipulating Beam
grobs.

http://codereview.appspot.com/88155/diff/3101/4032#newcode12
Line 12: LY_DEFINE (ly_beam_settings, ly:beam-settings,
is this function really necessary?

http://codereview.appspot.com/88155/diff/3101/4032#newcode49
Line 49: ly_grouping_rules(settings,time_signature,rule_type),
formatting

http://codereview.appspot.com/88155/diff/3101/4032#newcode61
Line 61: SCM settings = ly_beam_settings(context);
formatting

http://codereview.appspot.com/88155


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

Re: New format for autobeaming rules

2009-07-22 Thread n . puttock 


http://codereview.appspot.com/88155/diff/3101/4027
File input/new/grouping-beats.ly (right):

http://codereview.appspot.com/88155/diff/3101/4027#newcode7
Line 7: Beaming patterns may be altered with the @code{beatGrouping}
property:
new texidoc using \overrideBeamSettings

http://codereview.appspot.com/88155/diff/3101/4032
File lily/beam-scheme.cc (right):

http://codereview.appspot.com/88155/diff/3101/4032#newcode10
Line 10: #include beam-grouping.hh
swap

http://codereview.appspot.com/88155/diff/3101/4032#newcode26
Line 26:  @var{rule-type} in @var{context}.)
no context arg, doc settings

http://codereview.appspot.com/88155/diff/3101/4032#newcode28
Line 28: LY_ASSERT_TYPE (ly_is_pair, time_signature, 2);
scm_is_pair

http://codereview.appspot.com/88155/diff/3101/4032#newcode30
Line 30:
type check also for settings?

http://codereview.appspot.com/88155/diff/3101/4032#newcode34
Line 34: ly_assoc_get ((scm_list_2 (time_signature, rule_type)),
excess parens

http://codereview.appspot.com/88155/diff/3101/4032#newcode43
Line 43: Return grouping for beams of @var{ beam-type} in
@var{beam-type}

http://codereview.appspot.com/88155/diff/3101/4032#newcode45
Line 45:  @var{rule-type} in @var{context}.)
no context arg, doc settings

http://codereview.appspot.com/88155/diff/3101/4032#newcode46
Line 46: {
type checks?

http://codereview.appspot.com/88155/diff/3101/4032#newcode57
Line 57: {
LY_ASSERT_SMOB (Context, context, 1);

If you don't do this, then unsmob_context () will return NULL if this
function is passed invalid arguments, leading to a null dereference for
get_property (timeSignatureFraction) - segfault

http://codereview.appspot.com/88155/diff/3101/4033
File lily/beaming-pattern.cc (right):

http://codereview.appspot.com/88155/diff/3101/4033#newcode18
Line 18: #include beam-grouping.hh
sort

http://codereview.appspot.com/88155/diff/3101/4034
File lily/include/beam-grouping.hh (right):

http://codereview.appspot.com/88155/diff/3101/4034#newcode8
Line 8:
To prevent multiple includes:

#ifndef BEAM_GROUPING_HH
#define BEAM_GROUPING_HH

(+ line 14)

http://codereview.appspot.com/88155/diff/3101/4034#newcode14
Line 14:
#endif // BEAM_GROUPING_HH

http://codereview.appspot.com/88155/diff/3101/4035
File lily/measure-grouping-engraver.cc (right):

http://codereview.appspot.com/88155/diff/3101/4035#newcode14
Line 14: #include beam-grouping.hh
sort

http://codereview.appspot.com/88155/diff/3101/4035#newcode66
Line 66: SCM time_signature_fraction = get_property
(timeSignatureFraction);
move to if { } block, then it's only retrieved if required

http://codereview.appspot.com/88155/diff/3101/4038
File ly/music-functions-init.ly (right):

http://codereview.appspot.com/88155/diff/3101/4038#newcode20
Line 20: (_i Define @@var{music} as a quotable music expression named
rogue extra @'s throughout file

http://codereview.appspot.com/88155/diff/3101/4039
File python/convertrules.py (right):

http://codereview.appspot.com/88155/diff/3101/4039#newcode2930
Line 2930: str = re.sub(\\set\w+#\'beatGrouping, \\setBeatGrouping,
str)
won't get here due to search above (and regexp is broken)

http://codereview.appspot.com/88155/diff/3101/4041
File scm/beam-settings.scm (right):

http://codereview.appspot.com/88155/diff/3101/4041#newcode48
Line 48: ;; in 3 4 time:
decided not to restore original setting?

http://codereview.appspot.com/88155/diff/3101/4041#newcode155
Line 155:  Functions for overriding beam settings
indentation of function bodies

http://codereview.appspot.com/88155/diff/3101/4043
File scm/define-context-properties.scm (right):

http://codereview.appspot.com/88155/diff/3101/4043#newcode126
Line 126: (beatGrouping ,list? A list of beatgroups, e.g., in 5/8 time
remove

http://codereview.appspot.com/88155


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

PATCH: Re: New format for autobeaming rules

2009-07-21 Thread Carl Sorensen 


I'm getting ready to push the new autobeaming code.  Are there any more
comments before I do so?

http://codereview.appspot.com/88155

Thanks,

Carl



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

Re: PATCH: Re: New format for autobeaming rules

2009-07-21 Thread Patrick McCarty 
On Tue, Jul 21, 2009 at 11:03 AM, Carl Sorensenc_soren...@byu.edu wrote:


 I'm getting ready to push the new autobeaming code.  Are there any more
 comments before I do so?

 http://codereview.appspot.com/88155

Did you address Neil's last comment on the patchset, from 5 days ago?

-Patrick


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

Re: PATCH: Re: New format for autobeaming rules

2009-07-21 Thread Patrick McCarty 
On Tue, Jul 21, 2009 at 11:06 AM, Patrick McCartypnor...@gmail.com wrote:
 On Tue, Jul 21, 2009 at 11:03 AM, Carl Sorensenc_soren...@byu.edu wrote:


 I'm getting ready to push the new autobeaming code.  Are there any more
 comments before I do so?

 http://codereview.appspot.com/88155

 Did you address Neil's last comment on the patchset, from 5 days ago?

Never mind.  :-)  Sorry for the noise.

-Patrick


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

Re: New format for autobeaming rules

2009-07-21 Thread joeneeman 

very nice!


http://codereview.appspot.com/88155/diff/3092/2039
File ly/music-functions-init.ly (right):

http://codereview.appspot.com/88155/diff/3092/2039#newcode699
Line 699: (make-music 'SequentialMusic 'void #t))
I'd feel better if this were finished. At least add FIXME so it can be
easily grepped for.

http://codereview.appspot.com/88155/diff/3092/2047
File scm/music-functions.scm (right):

http://codereview.appspot.com/88155/diff/3092/2047#newcode546
Line 546: (define (make-default-beaming-rule context)
this doesn't seem to be used...

http://codereview.appspot.com/88155


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

Re: New format for autobeaming rules

2009-07-17 Thread Graham Percival 
On Wed, Jul 15, 2009 at 07:43:56AM -0600, Carl Sorensen wrote:
 
 On 7/14/09 3:57 PM, n.putt...@gmail.com n.putt...@gmail.com wrote:
 
  http://codereview.appspot.com/88155/diff/95/1147#newcode69
  Line 69: section 1.2.4 Beams, for more information.
  Is it possible to use @ruser{} here?
 
 I'm not sure.  I thought NEWS was supposed to be a standalone document.
 Graham, you're the source of all truth about documents; what do you think?

Until now, it's been a standalone document, but perhaps that
should change.  Only after the new unified doc process, unified
macros, etc, though.  So just leave it as-is for now.  :)

  http://codereview.appspot.com/88155/diff/95/1149#newcode1743
  Line 1743: Beam settings can be reverted to get back to default
  behavior.  This
  This looks like it should be an input/new snippet.
 
 I'm not sure I understand why you think it should it be in input/new instead
 of just being in the docs.  It doesn't use \set or \override.  It explains
 the use of a LilyPond command.  That's why I thought it should be an inline
 snippet.

In most doc sections, we'd move tweaking-type stuff (and I think
that \overrideBeamSettings would qualify) into snippets.

HOWEVER, certain NR sections go into more detail... my traditional
example of this is the page about changing automatic beam
settings.  So I definitely think it's ok to have this inline here.

Cheers,
- Graham


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

Re: New format for autobeaming rules

2009-07-15 Thread John Mandereau 
Le mercredi 15 juillet 2009 à 07:43 -0600, Carl Sorensen a écrit :
 On 7/14/09 3:57 PM, n.putt...@gmail.com n.putt...@gmail.com wrote:
  http://codereview.appspot.com/88155/diff/95/1147#newcode69
  Line 69: section 1.2.4 Beams, for more information.
  Is it possible to use @ruser{} here?
 
 I'm not sure.  I thought NEWS was supposed to be a standalone document.
 Graham, you're the source of all truth about documents; what do you think?

There is no @ruser defined in NEWS. However, see top of NEWS.tely: a
macro named @usermanref is defined, but it's currently unused in both
2.12 and 2.13, so please check whether it works before pushing.

Fortunately, Graham's proposal of reorganization will lead to clean up
and unify our Texinfo xref cooking.

Best,
John



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

Re: New format for autobeaming rules

2009-07-15 Thread Carl Sorensen 



On 7/15/09 9:30 AM, John Mandereau john.mander...@gmail.com wrote:

 Le mercredi 15 juillet 2009 à 07:43 -0600, Carl Sorensen a écrit :
 On 7/14/09 3:57 PM, n.putt...@gmail.com n.putt...@gmail.com wrote:
 http://codereview.appspot.com/88155/diff/95/1147#newcode69
 Line 69: section 1.2.4 Beams, for more information.
 Is it possible to use @ruser{} here?
 
 I'm not sure.  I thought NEWS was supposed to be a standalone document.
 Graham, you're the source of all truth about documents; what do you think?
 
 There is no @ruser defined in NEWS. However, see top of NEWS.tely: a
 macro named @usermanref is defined, but it's currently unused in both
 2.12 and 2.13, so please check whether it works before pushing.

I tried, and it didn't work (but it didn't appear to break compilation,
either).  So for right now, I think I'll leave the reference out.

Carl




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

Re: New format for autobeaming rules

2009-07-15 Thread Neil Puttock 
2009/7/15 Carl Sorensen c_soren...@byu.edu:

 I'm not sure I understand why you think it should it be in input/new instead
 of just being in the docs.  It doesn't use \set or \override.  It explains
 the use of a LilyPond command.  That's why I thought it should be an inline
 snippet.

It's positioned in the middle of a @snippet block, so it looks like a
snippet waiting to be moved to LSR.

Regards,
Neil


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

Re: New format for autobeaming rules

2009-07-15 Thread Carl Sorensen 



On 7/15/09 3:45 PM, n.putt...@gmail.com n.putt...@gmail.com wrote:

 
 
 http://codereview.appspot.com/88155/diff/2005/3086
 File scm/music-functions.scm (right):
 
 http://codereview.appspot.com/88155/diff/2005/3086#newcode519
 Line 519: (make-simultaneous-music output)
 This breaks all the Festival regression tests which use \time
 (song-associated-voice.ly, song-basic.ly, song-breathe.ly,
 song-melisma.ly, song-stanzas.ly  song-tempo.ly):
 
 /usr/local/share/lilypond/2.13.4/scm/song.scm:707:52: In procedure last
 in expression (last lst-1):
 /usr/local/share/lilypond/2.13.4/scm/song.scm:707:52: Wrong type
 argument in position 1 (expecting pair): ()
 
 http://codereview.appspot.com/88155

Aargh -- I thought I had run the regtests after that change, but apparently
not.

I've got that fixed now, and the regtests run without error.

Thanks for saving my bacon,

Carl



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

Re: New format for autobeaming rules

2009-07-14 Thread Carl Sorensen 
I've posted a new patch set on Rietveld which I think is a release
candidate.

It has changes in all the translated rhythms.itely files so that all of the
docs build properly.

It has the revised snippets in input/new added to the repository.

It has cleaned up all of the issues that Neil identified, with the exception
of default autobeaming for 3/4 time.  I never got any concurrence for
setting it back to (3), so it's left at (1 1 1).

Please review and let me know of any further issues.

Thanks,

Carl



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

Re: New format for autobeaming rules

2009-07-14 Thread n . puttock 

Carl, I haven't commenting on them directly, but there are quite a few
indentation errors in the .scm files.


http://codereview.appspot.com/88155/diff/95/1147
File Documentation/topdocs/NEWS.tely (right):

http://codereview.appspot.com/88155/diff/95/1147#newcode69
Line 69: section 1.2.4 Beams, for more information.
Is it possible to use @ruser{} here?

http://codereview.appspot.com/88155/diff/95/1149
File Documentation/user/rhythms.itely (right):

http://codereview.appspot.com/88155/diff/95/1149#newcode1743
Line 1743: Beam settings can be reverted to get back to default
behavior.  This
This looks like it should be an input/new snippet.

http://codereview.appspot.com/88155/diff/95/1155
File input/lsr/conducting-signs,-measure-grouping-signs.ly (right):

http://codereview.appspot.com/88155/diff/95/1155#newcode77
Line 77: }
missing %begin verbatim

http://codereview.appspot.com/88155/diff/95/1155#newcode88
Line 88: } % begin verbatim
rogue %begin verbatim

http://codereview.appspot.com/88155/diff/95/1170
File lily/measure-grouping-engraver.cc (right):

http://codereview.appspot.com/88155/diff/95/1170#newcode71
Line 71: scm_list_2 (time_signature_fraction,
same line as ly_assoc_get (

http://codereview.appspot.com/88155/diff/95/1170#newcode72
Line 72: ly_symbol2scm (end)),
indentation

http://codereview.appspot.com/88155/diff/95/1170#newcode75
Line 75: grouping_rules, SCM_EOL);
indentation

http://codereview.appspot.com/88155/diff/95/1177
File scm/c++.scm (right):

http://codereview.appspot.com/88155/diff/95/1177#newcode30
Line 30: (define-public (pair-or-symbol? x)
also unused

http://codereview.appspot.com/88155


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

Re: New format for autobeaming rules

2009-07-14 Thread Neil Puttock 
2009/7/14 Carl Sorensen c_soren...@byu.edu:

 It has cleaned up all of the issues that Neil identified, with the exception
 of default autobeaming for 3/4 time.  I never got any concurrence for
 setting it back to (3), so it's left at (1 1 1).

I think (3) is preferable, at least for quavers.

The Baerenreiter Bach 'cello suite example shows how it should be
(though it's got manual beaming just to make sure).

Regards,
Neil


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

Re: New format for autobeaming rules

2009-07-12 Thread Carl Sorensen 



On 7/12/09 11:13 AM, n.putt...@gmail.com n.putt...@gmail.com wrote:

 
 
 http://codereview.appspot.com/88155/diff/43/1044
 File Documentation/user/rhythms.itely (right):
 
 http://codereview.appspot.com/88155/diff/43/1044#newcode1662
 Line 1662: of the beam, e.g. @code{#'(1 . 16)}, or @code{#'*} to
 indicate a
 This could be confusing to users unfamiliar with Scheme, since it
 implies the hash and quote are required inside the alist.

Good catch.  This was written in an earlier incarnation, when I didn't have
an alist, and they needed the hash for each argument.  Thanks!
 
 http://codereview.appspot.com/88155/diff/43/1045
 File
 input/lsr/automatic-beams-two-per-two-in-4-4-or-2-2-time-signature.ly
 (right):
 
 http://codereview.appspot.com/88155/diff/43/1045#newcode1
 Line 1: %% Do not edit this file; it is auto-generated from input/new
 If you're keeping the revised rule for 4/4, this file's obsolete (it's
 already been unticked in LSR for docs).
 
 The changes you've made contradict the original intent of this snippet
 (and the title's incorrect), so it would be better to have a new snippet
 for this.
 
 Frankly, I'd much prefer returning to the old settings (also for 3/4);
 not only are they more in keeping with LilyPond's declared aims with
 regard to typesetting style, they also produce better looking examples
 in the documentation.

I have no problem with returning to the old settings.  I tried to make this
patch rule-neutral (but I made a mistake in 4 8, which you pointed out
earlier).

IIRC, Trevor Daniels took charge of cleaning up the old (2.12) autobeaming
stuff once beatGrouping started working.

I guess there's no sense in trying to recreate the history; let's just fix
it.  What do you think the default beam grouping for 4 4 and 3 4 should be?

for 4 4: (* . (2 2))  ?

and for 3 4: (* . (3)) ?



 
 http://codereview.appspot.com/88155/diff/43/1045#newcode53
 Line 53: } % begin verbatim
 There should only be one of these inserted automatically (as above, at
 the closing brace for the header).
 

Thanks, I'll fix that for all the snippets.

 
 http://codereview.appspot.com/88155/diff/43/1060
 File lily/measure-grouping-engraver.cc (right):
 
 http://codereview.appspot.com/88155/diff/43/1060#newcode65
 Line 65: SCM time_signature_fraction =
 get_property(timeSignatureFraction);
 get_property (

Oh, yes, thanks.
 
 http://codereview.appspot.com/88155/diff/43/1060#newcode69
 Line 69:
 remove blank line

Done.
 
 http://codereview.appspot.com/88155/diff/43/1060#newcode74
 Line 74: scm_from_locale_string (end))),
 ly_symbol2scm (end)

OK, thanks.  I remember seeing that function elsewhere, but I couldn't find
it when I was coding.

I wish we had a way of telling people where in the source tree the scheme
utility functions are.  Maybe in the CG...?

 
 http://codereview.appspot.com/88155/diff/43/1060#newcode77
 Line 77: scm_from_locale_string (*)),
 ly_symbol2scm (*)

Thanks.  Fixed
 
 http://codereview.appspot.com/88155/diff/43/1067
 File scm/c++.scm (right):
 
 http://codereview.appspot.com/88155/diff/43/1067#newcode34
 Line 34: (or (number? x) (symbol? x)))
 unused?

Yep, detritus from an earlier incarnation when '* or (1 . 16) were
stand-alone arguments, rather than part of an alist.

Removed.

Thanks,

Carl



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

Re: New format for autobeaming rules

2009-07-12 Thread Carl Sorensen 



On 7/12/09 11:13 AM, n.putt...@gmail.com n.putt...@gmail.com wrote:


 
 http://codereview.appspot.com/88155/diff/43/1045
 File
 input/lsr/automatic-beams-two-per-two-in-4-4-or-2-2-time-signature.ly
 (right):
 
 http://codereview.appspot.com/88155/diff/43/1045#newcode1
 Line 1: %% Do not edit this file; it is auto-generated from input/new
 If you're keeping the revised rule for 4/4, this file's obsolete (it's
 already been unticked in LSR for docs).
 
 The changes you've made contradict the original intent of this snippet
 (and the title's incorrect), so it would be better to have a new snippet
 for this.
 
 Frankly, I'd much prefer returning to the old settings (also for 3/4);
 not only are they more in keeping with LilyPond's declared aims with
 regard to typesetting style, they also produce better looking examples
 in the documentation.
 
I've returned the default grouping to (2 2) for 4/4.  We still don't need
this snippet; there's an example in the docs that demonstrates this.

I've eliminated this snippet.

Thanks,

Carl



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

Re: New format for autobeaming rules

2009-07-12 Thread Carl Sorensen 



On 7/12/09 11:13 AM, n.putt...@gmail.com n.putt...@gmail.com wrote:


 
 http://codereview.appspot.com/88155/diff/43/1045
 File
 input/lsr/automatic-beams-two-per-two-in-4-4-or-2-2-time-signature.ly
 (right):
 
 http://codereview.appspot.com/88155/diff/43/1045#newcode1
 Line 1: %% Do not edit this file; it is auto-generated from input/new
 If you're keeping the revised rule for 4/4, this file's obsolete (it's
 already been unticked in LSR for docs).
 
 The changes you've made contradict the original intent of this snippet
 (and the title's incorrect), so it would be better to have a new snippet
 for this.
 
 Frankly, I'd much prefer returning to the old settings (also for 3/4);
 not only are they more in keeping with LilyPond's declared aims with
 regard to typesetting style, they also produce better looking examples
 in the documentation.
 
I've returned the default grouping to (2 2) for 4/4.  We still don't need
this snippet; there's an example in the docs that demonstrates this.

I've eliminated this snippet.

Thanks,

Carl



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