Accidentals patch

[Han-Wen Nienhuys]

address@hidden writes:
> I've restructured the accidentals engraver, and added property
> autoReminders, automatically creating reminder accidentals.
> Please comment on the new structure of the engraver.
> Still needs lots of clean up - as I do not entirely understand the idea
> of the tie-break handling. - Afaics it handles tie breaks the same way
> as before. -

> But I do not really see the idea - i.e. in the example
> added to test/ I do not understand why the 2nd note of measure 6 should
> get a sharp...

Unless I'm mistaken, it says

       dis ~  | \break dis dis

the effect of an accidental alteration is not carried into the next
measure by a tied note. You have to restate the accidental on the 2nd
note in the measure without the break.  I agree that it looks a little
silly when the first note also has an accidental (due to the line
break), but technically it is correct, since the 2nd one is needed to
determine the pitch of the note.

I have a few minor gripes .. 

> +/** calculates the number of accidentals on basis of the current local time 
> sig

key sig.

> +Accidental_engraver::number_accidentals (SCM sig, Note_req *
> note_l)

this should be a static function, as it doesn't access any member variables.

> +{
> +  int n = unsmob_pitch (note_l->get_mus_property ("pitch"))->notename_i_;
> +  int o = unsmob_pitch (note_l->get_mus_property ("pitch"))->octave_i () ;
> +  int a = unsmob_pitch (note_l->get_mus_property ("pitch"))->alteration_i_;

looks as if you've copied an example of our search & replace
programming. This should really be

  Pitch *p = unsmob_pitch (note_l->get_mus_property ("pitch"));

  int n =p->notename_i_; //etc.

>         /*
>           We should not record the accidental if it is the first
>           note and it is tied from the previous measure.
>  
>           Checking whether it is tied also works mostly, but will it
>           always do the correct thing?
> -
> +         (???? -Rune )

good question. The code that followed should probably check wether
we're on the first beat of measure to be completely correct (or
rather: to be in accordance with the first part of the comment).

>           {
>             /*
>               Remember an alteration that is different both from
>               that of the tied note and of the key signature.
> -
> +             (????? -Rune )
>              */

hmm. Good question. This stuff is related to
http://mail.gnu.org/pipermail/gnu-music-discuss/2000-September/date.html
(a fix in 1.3.86), and to a fix by mats (1.3.122)
http://mail.gnu.org/pipermail/bug-gnu-music/2001-January/000098.html

Perhaps it would be worth the trouble to document *all* cases that
should (and are)  handled, each in one small regression test. 

> +           localsig = scm_assoc_set_x (localsig, ON, SCM_BOOL_T); 

please: lower case letters for variables.

> diff -urN lilypond-1.5.15/scm/translator-description.scm 
> lilypond-1.5.15.rz1/scm/translator-description.scm
> --- lilypond-1.5.15/scm/translator-description.scm    Mon Sep 24 01:04:29 2001
> +++ lilypond-1.5.15.rz1/scm/translator-description.scm        Sun Oct  7 
> 23:42:20 2001

Ah. That reminds me, this file has to go. This stuff goes into the
ENTER_DESCRIPTION argument (which is --come to think of it-- a pretty
silly name for this macro).
 


-- 

Han-Wen Nienhuys   |   address@hidden    | http://www.cs.uu.nl/~hanwen/