bar-line interface part 2/2: New bar line definition standard

The new bar line interface allows for easier extension and modification of the bar lines:

* the input string of a bar line called by \bar "<bar string>" resembles the output one-by-one; a repeat sign is called by ":|."

* (define-bar-line ...) or \defineBarLine allows for new definitions. These functions have four arguments, namely
  the bar line itself, the bar line used at the end of line, the bar line used at the begin of a new line and the span bar line.

* Annotations for barlines can be added to distinguish between two identical barlines with different break
  apperance or span bar lines.

* To align span bars properly, the space character " " has a
  special meaning. When it occurs, the width of the resembling glyph in the bar line definition is used.

* New glyphs can be added easily.

* Volta brackets take the bar line dimensions into account.

[MikeSol, 2012-08-29 18:18:51]

LGTM

[thomasmorley65, 2012-08-29 23:46:05]

Great work so far.
Some minor suggestions:

http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm
File scm/bar-line.scm (right):

http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode203
scm/bar-line.scm:203: 
If a user defines some curious bar-lines like:
#(define-bar-line "|SS...SS|" "|SS.." "..SS|" "|==...==|")
LilyPond will create some weird output without any hint whats wrong, I'd suggest
adding a warning like:

(map (lambda (x) (if (not (assoc-get x bar-glyph-alist))
  		  (ly:warning (_ "bar-glyph ~a has to be defined before, in separate
definition") x)))
  	    (list eol-glyph bol-glyph))

and to change the order of the predefined bar-lines at the bottom of the file.

http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode212
scm/bar-line.scm:212: 
If a user defines his own bar-line and chooses a glyph with string-length
greater than 1, compiling will fail without a useful log-message.
I'd suggest:

(define-public (add-bar-glyph-print-procedure glyph proc)
  "Specify the single glyph @var{glyph} that calls print procedure @var{proc}.
The procedure @var{proc} has to be defined in the form
@code{(make-...-bar-line grob extent)} even if the @var{extent}
is not used within the routine."
  (if (or (not (string? glyph))
          (> (string-length glyph) 1))
      (ly:warning (_ "glyph ~a is not of string-length 1") glyph)
  (set! bar-glyph-print-procedures
    (acons glyph proc bar-glyph-print-procedures))))

or sth like this

http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode878
scm/bar-line.scm:878: (define-bar-line ":.|.:" ":|." ".|:" " .|.")
The following to lines should be moved to the position directly after ;;repeats
(see my comment above)

http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode882
scm/bar-line.scm:882: (define-bar-line ":|]" "|" ":|]" " |")
I think it should be:
(define-bar-line ":|]" ":|]" ""  " |")

[marc, 2012-08-30 13:11:38]

Am 30.08.2012 01:46, schrieb thomasmorley65@googlemail.com:
> Great work so far.
> Some minor suggestions:
>
>
>
>
> http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm
> File scm/bar-line.scm (right):
>
> http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode203 
>
> scm/bar-line.scm:203:
> If a user defines some curious bar-lines like:
> #(define-bar-line "|SS...SS|" "|SS.." "..SS|" "|==...==|")
> LilyPond will create some weird output without any hint whats wrong, I'd
> suggest adding a warning like:
>
> (map (lambda (x) (if (not (assoc-get x bar-glyph-alist))
>             (ly:warning (_ "bar-glyph ~a has to be defined before, in 
> separate
> definition") x)))
>           (list eol-glyph bol-glyph))
>
> and to change the order of the predefined bar-lines at the bottom of the
> file.
Thanks for spotting this!

I decided to give a warning and an empty stencil; this has the advantage 
of the
order of the definitions is not relevant.
>
> http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode212 
>
> scm/bar-line.scm:212:
> If a user defines his own bar-line and chooses a glyph with
> string-length greater than 1, compiling will fail without a useful
> log-message.
> I'd suggest:
>
> (define-public (add-bar-glyph-print-procedure glyph proc)
>   "Specify the single glyph @var{glyph} that calls print procedure
> @var{proc}.
> The procedure @var{proc} has to be defined in the form
> @code{(make-...-bar-line grob extent)} even if the @var{extent}
> is not used within the routine."
>   (if (or (not (string? glyph))
>           (> (string-length glyph) 1))
>       (ly:warning (_ "glyph ~a is not of string-length 1") glyph)
>   (set! bar-glyph-print-procedures
>     (acons glyph proc bar-glyph-print-procedures))))
>
> or sth like this
>
Of course! Done.
> http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode878 
>
> scm/bar-line.scm:878: (define-bar-line ":.|.:" ":|." ".|:" " .|.")
> The following to lines should be moved to the position directly after
> ;;repeats
> (see my comment above)
>
> http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode882 
>
> scm/bar-line.scm:882: (define-bar-line ":|]" "|" ":|]" " |")
> I think it should be:
> (define-bar-line ":|]" ":|]" ""  " |")
>
Oh well ...

Done.
> http://codereview.appspot.com/6498052/
>

[thomasmorley65, 2012-08-31 00:05:06]

http://codereview.appspot.com/6498052/diff/10001/Documentation/snippets/print...
File
Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly
(right):

http://codereview.appspot.com/6498052/diff/10001/Documentation/snippets/print...
Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly:13:
A @code{|:} bar line can be printed at the beginning of a piece, by
Should be:
@code{.|:}

http://codereview.appspot.com/6498052/diff/10001/input/regression/span-bar-br...
File input/regression/span-bar-break.ly (right):

http://codereview.appspot.com/6498052/diff/10001/input/regression/span-bar-br...
input/regression/span-bar-break.ly:5: texidoc = "At the beginning of a system,
the @code{|:} repeat
Should be:
texidoc = "At the beginning of a system, the @code{.|:} repeat
barline is drawn between the staves, but the @code{:|.} is not."

[marc, 2012-09-01 16:09:59]

Am 31.08.2012 02:05, schrieb thomasmorley65@googlemail.com:
>
>
http://codereview.appspot.com/6498052/diff/10001/Documentation/snippets/print...

>
> File
> Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly 
>
> (right):
>
>
http://codereview.appspot.com/6498052/diff/10001/Documentation/snippets/print...

>
>
Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly:13:

>
> A @code{|:} bar line can be printed at the beginning of a piece, by
> Should be:
> @code{.|:}
Uh, this is part of LSR. Should this be changed for a development 
release, or when
LSR is updated to 2.18?
>
>
http://codereview.appspot.com/6498052/diff/10001/input/regression/span-bar-br...

>
> File input/regression/span-bar-break.ly (right):
>
>
http://codereview.appspot.com/6498052/diff/10001/input/regression/span-bar-br...

>
> input/regression/span-bar-break.ly:5: texidoc = "At the beginning of a
> system, the @code{|:} repeat
> Should be:
> texidoc = "At the beginning of a system, the @code{.|:} repeat
> barline is drawn between the staves, but the @code{:|.} is not."
>
> http://codereview.appspot.com/6498052/
Ok, done. I do not upload changes yet, because the patchy test will fail 
anyway.

Regards,

Marc

[thomasmorley65, 2012-09-02 20:24:47]

More nit-picking:

http://codereview.appspot.com/6498052/diff/7003/python/convertrules.py
File python/convertrules.py (right):

http://codereview.appspot.com/6498052/diff/7003/python/convertrules.py#newcod...
python/convertrules.py:3395: @rule ((2, 17, 2), r"New bar line interface")

There's no converting rule for the old "|s"
(Note the small `s')

I'd suggest to do:
"|s" -> "|-s"
and to add
#(define-bar-line "|-s" "" "|" "|")
to /scm/bar-line.scm

[marc, 2012-09-03 15:31:36]

Am 02.09.2012 22:24, schrieb thomasmorley65@googlemail.com:
> More nit-picking:
>
>
> http://codereview.appspot.com/6498052/diff/7003/python/convertrules.py
> File python/convertrules.py (right):
>
>
http://codereview.appspot.com/6498052/diff/7003/python/convertrules.py#newcod...

>
> python/convertrules.py:3395: @rule ((2, 17, 2), r"New bar line
> interface")
>
> There's no converting rule for the old "|s"
> (Note the small `s')
>
> I'd suggest to do:
> "|s" -> "|-s"
> and to add
> #(define-bar-line "|-s" "" "|" "|")
> to /scm/bar-line.scm
>
> http://codereview.appspot.com/6498052/
>
You are right. I'll include them in my next patch set,
probably after reworking the volta bracket stuff.

Thanks,

Marc

[Ian Hulin (gmail), 2012-09-27 22:21:23]

Apart from a typo in changes.tely (q.v.), LGTM.

http://codereview.appspot.com/6498052/diff/24001/Documentation/changes.tely
File Documentation/changes.tely (right):

http://codereview.appspot.com/6498052/diff/24001/Documentation/changes.tely#n...
Documentation/changes.tely:67: as @code{:|}. The command @code{\defineBarLine}
provides an easy way
it needs to be "as @code{.|:}" if you mean a repeat sign that appears on the
left-hand end of a repeated section. 
This is what you have used in the new snippet for this patch,
Documentation/snippets/new/printing-a-repeat-sign-at-the-beginning-of-the-piece.

[thomasmorley65, 2012-09-27 22:54:54]

One tiny addition:

http://codereview.appspot.com/6498052/diff/24001/scm/bar-line.scm
File scm/bar-line.scm (right):

http://codereview.appspot.com/6498052/diff/24001/scm/bar-line.scm#newcode1053
scm/bar-line.scm:1053: (define-bar-line ":|]" ":|]" #f " |")
When we provide a bracket-repeat-sign for left and right repeats, we should
provide a double-repeat-sign, too.
I suggest:
(define-bar-line ":|][|:" ":|]" "[|:" " |  |")

[marc, 2012-09-28 07:59:13]

Am 28.09.2012 00:21, schrieb ianhulin44@gmail.com:
> Apart from a typo in changes.tely (q.v.), LGTM.
>
>
> http://codereview.appspot.com/6498052/diff/24001/Documentation/changes.tely 
>
> File Documentation/changes.tely (right):
>
>
http://codereview.appspot.com/6498052/diff/24001/Documentation/changes.tely#n...

>
> Documentation/changes.tely:67: as @code{:|}. The command
> @code{\defineBarLine} provides an easy way
> it needs to be "as @code{.|:}" if you mean a repeat sign that appears on
> the left-hand end of a repeated section.
> This is what you have used in the new snippet for this patch,
>
Documentation/snippets/new/printing-a-repeat-sign-at-the-beginning-of-the-piece.

>
>
> http://codereview.appspot.com/6498052/
>
Of course! Seems like I don't understand my interface ;-)

Thanks for spotting this! I include it in the next patch round, when I 
have found the
error that prevents 'make check' to finish properly.

Regards,

Marc

[marc, 2012-09-28 08:00:54]

Am 28.09.2012 00:54, schrieb thomasmorley65@googlemail.com:
> One tiny addition:
>
>
>
>
> http://codereview.appspot.com/6498052/diff/24001/scm/bar-line.scm
> File scm/bar-line.scm (right):
>
> http://codereview.appspot.com/6498052/diff/24001/scm/bar-line.scm#newcode1053

>
> scm/bar-line.scm:1053: (define-bar-line ":|]" ":|]" #f " |")
> When we provide a bracket-repeat-sign for left and right repeats, we
> should provide a double-repeat-sign, too.
> I suggest:
> (define-bar-line ":|][|:" ":|]" "[|:" " |  |")
>
> http://codereview.appspot.com/6498052/
>
Good idea! This shouls how easy it is to include new bar line styles: no 
compilation,
just add one line!
BTW, I'd allow a volta hook for ":|][|:" as well.

Regards,

Marc

[Keith, 2012-09-29 05:11:46]

Looks good so far.

In one pdf previewer (evince) at low resolution, the span bars look a little
thicker than the regular bar lines.  Maybe a rounding fault of the viewer, but
it would be better if you know how to avoid it.

http://codereview.appspot.com/6498052/diff/24001/scm/bar-line.scm
File scm/bar-line.scm (right):

http://codereview.appspot.com/6498052/diff/24001/scm/bar-line.scm#newcode953
scm/bar-line.scm:953: (ly:grob-property right-bar-line 'glyph-name)))
Sometimes I get a crash that I traced back to a null -- that is, '() --
left-bar-glyph-name here.  The piece in question has a repeat alternative that
starts a on new line.

http://codereview.appspot.com/6498052/diff/24001/scm/bar-line.scm#newcode1029
scm/bar-line.scm:1029: 
;; "normal bar -annotation" "end of line" "start of line" "span bar"

[marc, 2012-09-29 09:14:08]

Am 29.09.2012 07:11, schrieb k-ohara5a5a@oco.net:
> Looks good so far.
>
> In one pdf previewer (evince) at low resolution, the span bars look a
> little thicker than the regular bar lines. Maybe a rounding fault of
> the viewer, but it would be better if you know how to avoid it.
If you zoom it, this will disappear, of course (I looked at bar lines 
with Adobe Reader at
very high zoom levels), but I use evince, too and do not see these 
artifacts.
What I spot is a thin white line between staff and span bar that 
disappears when zooming in,
due to the span bars being a different object.

I don't know how this can be avoided, though.
>
>
> http://codereview.appspot.com/6498052/diff/24001/scm/bar-line.scm
> File scm/bar-line.scm (right):
>
> http://codereview.appspot.com/6498052/diff/24001/scm/bar-line.scm#newcode953 
>
> scm/bar-line.scm:953: (ly:grob-property right-bar-line 'glyph-name)))
> Sometimes I get a crash that I traced back to a null -- that is, '() --
> left-bar-glyph-name here. The piece in question has a repeat
> alternative that starts a on new line.
Thanks! I found out that this is exactly the error that lets 'make test' 
fail –
in input/regression/instrument-name-volta.ly we have exactly this situation.

>
> http://codereview.appspot.com/6498052/diff/24001/scm/bar-line.scm#newcode1029

>
> scm/bar-line.scm:1029:
> ;; "normal bar -annotation" "end of line" "start of line" "span bar"
You are right, this should be documented in the comment; will do in the 
next patch release.

Regards,

Marc
>
> http://codereview.appspot.com/6498052/
>

[Keith, 2012-09-30 01:59:05]

On 2012/09/29 09:14:08, marc wrote:
> Am 29.09.2012 07:11, schrieb k-ohara5a5a@oco.net:
> > In one pdf previewer (evince) at low resolution, the span bars look a
> > little thicker than the regular bar lines. 

Disregard.  The slight mis-rendering on screen was there before your patch as
well.

[Keith, 2012-09-30 02:11:47]

Now looking at the log files, I do get "undead" errors for our regression test
'bar-line-define-bar-glyph.ly' but not from any other files that I tried.  (I
don't know why the scripts from `make check` did not flag it.)  

There are four messages, but the particular undead objects vary from run to run.

[dak, 2012-09-30 08:01:48]

http://codereview.appspot.com/6498052/diff/32001/scm/bar-line.scm
File scm/bar-line.scm (right):

http://codereview.appspot.com/6498052/diff/32001/scm/bar-line.scm#newcode225
scm/bar-line.scm:225: (define-public bar-glyph-alist '())
This interface definition is asking for trouble.  It is an important feature of
LilyPond that if you use standard commands, their effects to not bleed over from
one run to the next.  Information-carrying data structures, for that reason,
should be initialized in a .ly file loaded from init.ly.  Even then, the basic
data structure should be one that can be replaced by overwriting with a saved
copy.  That means that hash-tables have to be either read/only, or start out as
_empty_ in a session (hash tables defined in the init.ly session get cleared at
the end of session IIRC).

[marc, 2012-09-30 08:25:30]

Am 30.09.2012 04:11, schrieb k-ohara5a5a@oco.net:
> Now looking at the log files, I do get "undead" errors for our
> regression test 'bar-line-define-bar-glyph.ly' but not from any other
> files that I tried. (I don't know why the scripts from `make check` did
> not flag it.)
Keith, thanks for your work on this! It makes it much easier for me
to see which file causes errors, especially in this case where these
'undead objects' messages seem to float around very randomly.
>
> There are four messages, but the particular undead objects vary from run
> to run.
>
I can confirm that. When I compile the regtest above two times, the 
errors disappear.
When I compile another file and the regtest afterwards, I get different 
messages.
It looks as there are dynamics and/or articulations involved, which is 
strange because
neither the regtest nor the files I compiled in between have anything of 
that kind.

I must admit that I am an absolute newbie with regard to C++ debugging.
Can anyone help me here? I don't know where these undead objects come from –
the function (add-bar-glyph-print-procedure ...) used in the regtest is 
used in
scm/bar-line.scm, too, and seems to cause no problems there.

Regards,

Marc


> http://codereview.appspot.com/6498052/
>

[marc, 2012-09-30 08:33:16]

Am 30.09.2012 10:01, schrieb dak@gnu.org:
>
> http://codereview.appspot.com/6498052/diff/32001/scm/bar-line.scm
> File scm/bar-line.scm (right):
>
> http://codereview.appspot.com/6498052/diff/32001/scm/bar-line.scm#newcode225 
>
> scm/bar-line.scm:225: (define-public bar-glyph-alist '())
> This interface definition is asking for trouble.  It is an important
> feature of LilyPond that if you use standard commands, their effects to
> not bleed over from one run to the next.  Information-carrying data
> structures, for that reason, should be initialized in a .ly file loaded
> from init.ly.  Even then, the basic data structure should be one that
> can be replaced by overwriting with a saved copy.  That means that
> hash-tables have to be either read/only, or start out as _empty_ in a
> session (hash tables defined in the init.ly session get cleared at the
> end of session IIRC).
I am not sure if I understand you correctly.

Do you propose that the initialisation of the alists should be moved
to ly/init.ly and that's it, more or less?

Regards,

Marc
>
> http://codereview.appspot.com/6498052/
>

[mike7, 2012-09-30 08:35:12]

On 30 sept. 2012, at 10:25, Marc Hohl <marc@hohlart.de> wrote:

> Am 30.09.2012 04:11, schrieb k-ohara5a5a@oco.net:
>> Now looking at the log files, I do get "undead" errors for our
>> regression test 'bar-line-define-bar-glyph.ly' but not from any other
>> files that I tried. (I don't know why the scripts from `make check` did
>> not flag it.)
> Keith, thanks for your work on this! It makes it much easier for me
> to see which file causes errors, especially in this case where these
> 'undead objects' messages seem to float around very randomly.
>> 
>> There are four messages, but the particular undead objects vary from run
>> to run.
>> 
> I can confirm that. When I compile the regtest above two times, the errors
disappear.
> When I compile another file and the regtest afterwards, I get different
messages.
> It looks as there are dynamics and/or articulations involved, which is strange
because
> neither the regtest nor the files I compiled in between have anything of that
kind.
> 
> I must admit that I am an absolute newbie with regard to C++ debugging.
> Can anyone help me here? I don't know where these undead objects come from –
> the function (add-bar-glyph-print-procedure ...) used in the regtest is used
in
> scm/bar-line.scm, too, and seems to cause no problems there.
> 
> Regards,
> 
> Marc
> 

These messages are annoying...they often have to do with marking objects for
garbage collection (print_smob and mark_smob functions).  Given that you're not
creating any new smobs in this patch, it is tough to see where things may be
going wrong.  You can, however, do all sorts of stuff with the message itself to
get more useful info.  It gets printed out in scm/lily.scm, so you can modify
the printout to get more information about the object.

Cheers,
MS

[marc, 2012-09-30 08:57:03]

Am 30.09.2012 10:01, schrieb dak@gnu.org:
>
> http://codereview.appspot.com/6498052/diff/32001/scm/bar-line.scm
> File scm/bar-line.scm (right):
>
> http://codereview.appspot.com/6498052/diff/32001/scm/bar-line.scm#newcode225 
>
> scm/bar-line.scm:225: (define-public bar-glyph-alist '())
> This interface definition is asking for trouble.  It is an important
> feature of LilyPond that if you use standard commands, their effects to
> not bleed over from one run to the next.  Information-carrying data
> structures, for that reason, should be initialized in a .ly file loaded
> from init.ly.  Even then, the basic data structure should be one that
> can be replaced by overwriting with a saved copy.  That means that
> hash-tables have to be either read/only, or start out as _empty_ in a
> session (hash tables defined in the init.ly session get cleared at the
> end of session IIRC).
>
> http://codereview.appspot.com/6498052/
>
Update: a 'git grep "(define-public .* '())" shows

scm/bar-line.scm:(define-public bar-glyph-alist '())
scm/bar-line.scm:(define-public span-bar-glyph-alist '())
scm/bar-line.scm:(define-public volta-bracket-allow-volta-hook-list '())
scm/define-context-properties.scm:(define-public 
all-translation-properties '())
scm/define-note-names.scm:(define-public pitchnames '())
scm/lily.scm:(define-public lilypond-declarations '())
scm/midi.scm:(define-public absolute-volume-alist '())
scm/midi.scm:(define-public instrument-equalizer-alist '())

Since the faulty regtest adds a function to the list provided by

(define-public bar-glyph-print-procedures `())

I assume that *this* declaration should be moved to ly/init.ly?

Regards,

Marc

[dak, 2012-09-30 09:02:48]

On 2012/09/30 08:33:16, marc wrote:
> Am 30.09.2012 10:01, schrieb dak@gnu.org:
> >
> > http://codereview.appspot.com/6498052/diff/32001/scm/bar-line.scm
> > File scm/bar-line.scm (right):
> >
> > http://codereview.appspot.com/6498052/diff/32001/scm/bar-line.scm#newcode225

> >
> > scm/bar-line.scm:225: (define-public bar-glyph-alist '())
> > This interface definition is asking for trouble.  It is an important
> > feature of LilyPond that if you use standard commands, their effects to
> > not bleed over from one run to the next.  Information-carrying data
> > structures, for that reason, should be initialized in a .ly file loaded
> > from init.ly.  Even then, the basic data structure should be one that
> > can be replaced by overwriting with a saved copy.  That means that
> > hash-tables have to be either read/only, or start out as _empty_ in a
> > session (hash tables defined in the init.ly session get cleared at the
> > end of session IIRC).
> I am not sure if I understand you correctly.
> 
> Do you propose that the initialisation of the alists should be moved
> to ly/init.ly and that's it, more or less?

First, the define-public is asking for trouble.  You are exposing an internal
Scheme data structure to users and make it overwritable by the user.  If the
user follows this invitation, the effects will bleed over from session to
session.  Never do that.

Consider _everything_ written in a .scm file as readonly conceptually.  For an
alist, you might want to define a _function_ returning the initial list, and the
accessors should not be modifying it destructively (adding to the front of an
alist does not change the original alist).

Then you assign this initial value to a session variable in something included
in init.ly, like
#(define bar-glyph-alist (initial-bar-glyph-alist))

Your functions working with bar-glyph-alist have to take the version of
bar-glyph-alist defined in the parser module.  That means that when you call
them from .ly files, you'll likely need to pass bar-glyph-alist from the .ly
file into your Scheme function.  Or get them via ly:parser-lookup via the parser
variable.

If you put a new bar line definition into a.ly, and when calling
lilypond a.ly b.ly
it is available in b.ly, then you have done something wrong.

[marc, 2012-09-30 19:44:49]

Am 30.09.2012 11:02, schrieb dak@gnu.org:
> [...]
> First, the define-public is asking for trouble. You are exposing an
> internal Scheme data structure to users and make it overwritable by the
> user. If the user follows this invitation, the effects will bleed over
> from session to session. Never do that.
Ok.
>
> Consider _everything_ written in a .scm file as readonly conceptually.
> For an alist, you might want to define a _function_ returning the
> initial list, and the accessors should not be modifying it destructively
> (adding to the front of an alist does not change the original alist).
>
> Then you assign this initial value to a session variable in something
> included in init.ly, like
> #(define bar-glyph-alist (initial-bar-glyph-alist))
Ok.
>
> Your functions working with bar-glyph-alist have to take the version of
> bar-glyph-alist defined in the parser module. That means that when you
> call them from .ly files, you'll likely need to pass bar-glyph-alist
> from the .ly file into your Scheme function. Or get them via
> ly:parser-lookup via the parser variable.
I am not sure whether I understand this completely, but I'll give
it a try.
>
> If you put a new bar line definition into a.ly, and when calling
> lilypond a.ly b.ly
> it is available in b.ly, then you have done something wrong.
I did this and can confirm that a bar line defined in a.ly *is*
available in b.ly.

So I have to replace all three public alists in bar-line.scm the way you
described it above.

[OT: it seems that I have a strong attraction to problems that look
not very difficult at first glance and turn out to involve *a lot of*
changes and obstacles until they get solved.
On the other hand, this is a *perfect* way to learn about the internals
of lilypond – in such situations where everything breaks, I remember
Jan working on GUB during Waltrop and his statement when the
compilation broke again: "That's nice." I admire such a serenity and
try to adopt it in situations where I am about to throw the computer
out of the window ;-) I am not in this state yet, but I am working on it...]

Regards,

Marc
>
> http://codereview.appspot.com/6498052/
>

[dak, 2012-09-30 20:03:16]

On 2012/09/30 19:44:49, marc wrote:
> Am 30.09.2012 11:02, schrieb dak@gnu.org:
> > [...]
> > First, the define-public is asking for trouble. You are exposing an
> > internal Scheme data structure to users and make it overwritable by the
> > user. If the user follows this invitation, the effects will bleed over
> > from session to session. Never do that.
> Ok.

No, it's not ok.  Hold your horses, this is another case too stupid for
documenting and walking people through.  Give me two days, and then you
replace your define-public for the alists with define-session, and
that's it.  The rest of the code stays as it is.

I'll make define-session do everything that is needed.

Just that it is possible to learn all the internals of LilyPond session does not
mean that it makes sense that you are required to do so just in order to
accomplish a simple task.

[marc, 2012-10-01 08:46:09]

Am 30.09.2012 22:03, schrieb dak@gnu.org:
> On 2012/09/30 19:44:49, marc wrote:
>> Am 30.09.2012 11:02, schrieb dak@gnu.org:
>> > [...]
>> > First, the define-public is asking for trouble. You are exposing an
>> > internal Scheme data structure to users and make it overwritable by
> the
>> > user. If the user follows this invitation, the effects will bleed
> over
>> > from session to session. Never do that.
>> Ok.
>
> No, it's not ok.  Hold your horses, this is another case too stupid for
> documenting and walking people through.  Give me two days, and then you
> replace your define-public for the alists with define-session, and
> that's it.  The rest of the code stays as it is.
That's good news! I think I owe you something, thanks in advance!

Marc
>
> I'll make define-session do everything that is needed.
>
> Just that it is possible to learn all the internals of LilyPond session
> does not mean that it makes sense that you are required to do so just in
> order to accomplish a simple task.
>
> http://codereview.appspot.com/6498052/
>

[dak, 2012-10-01 09:00:34]

On 2012/10/01 08:46:09, marc wrote:
> Am 30.09.2012 22:03, schrieb dak@gnu.org:
> > On 2012/09/30 19:44:49, marc wrote:
> >> Am 30.09.2012 11:02, schrieb dak@gnu.org:
> >> > [...]
> >> > First, the define-public is asking for trouble. You are exposing an
> >> > internal Scheme data structure to users and make it overwritable by
> > the
> >> > user. If the user follows this invitation, the effects will bleed
> > over
> >> > from session to session. Never do that.
> >> Ok.
> >
> > No, it's not ok.  Hold your horses, this is another case too stupid for
> > documenting and walking people through.  Give me two days, and then you
> > replace your define-public for the alists with define-session, and
> > that's it.  The rest of the code stays as it is.
> That's good news! I think I owe you something, thanks in advance!

Or define-session-public.  define-session will be for data that is reset per
session, but not publicly accessible.  Which is probably what you want if all
the accessor functions are already in your .scm file.

[marc, 2012-10-01 09:08:43]

Am 01.10.2012 11:00, schrieb dak@gnu.org:
> On 2012/10/01 08:46:09, marc wrote:
>> Am 30.09.2012 22:03, schrieb dak@gnu.org:
>> > On 2012/09/30 19:44:49, marc wrote:
>> >> Am 30.09.2012 11:02, schrieb dak@gnu.org:
>> >> > [...]
>> >> > First, the define-public is asking for trouble. You are exposing
> an
>> >> > internal Scheme data structure to users and make it overwritable
> by
>> > the
>> >> > user. If the user follows this invitation, the effects will bleed
>> > over
>> >> > from session to session. Never do that.
>> >> Ok.
>> >
>> > No, it's not ok.  Hold your horses, this is another case too stupid
> for
>> > documenting and walking people through.  Give me two days, and then
> you
>> > replace your define-public for the alists with define-session, and
>> > that's it.  The rest of the code stays as it is.
>> That's good news! I think I owe you something, thanks in advance!
>
> Or define-session-public.  define-session will be for data that is reset
> per session, but not publicly accessible.  Which is probably what you
> want if all the accessor functions are already in your .scm file.
I think that -public is needed for data to be accessible for a user?
Or is it sufficient that the function accessing the data is -public?
>
> http://codereview.appspot.com/6498052/
>

[dak, 2012-10-01 09:16:51]

Marc Hohl <marc@hohlart.de> writes:

> Am 01.10.2012 11:00, schrieb dak@gnu.org:
>>
>> Or define-session-public.  define-session will be for data that is reset
>> per session, but not publicly accessible.  Which is probably what you
>> want if all the accessor functions are already in your .scm file.
> I think that -public is needed for data to be accessible for a user?
> Or is it sufficient that the function accessing the data is -public?

"public" just publishes the variable binding.  The binding does not need
to get published to be accessible to functions in the same source file:
the binding is only needed when the functions referring to it are
_defined_, not when they are being used.

Of course, you can't do session management on a variable you can't even
access, but publishing the variable for everybody else to tamper with is
not going to make our problem smaller.  So we need to be able to do
session-management on the variable _without_ publishing it, and that
means in the module where it is being defined.  And the easiest way is
to have separate defining commands for that purpose.

-- 
David Kastrup

[janek, 2012-10-03 15:25:35]

On Sunday, September 30, 2012, Davod Kastrup wrote:

> Hold your horses, this is another case too stupid for
> documenting and walking people through.  Give me two days, and then you
> replace your define-public for the alists with define-session, and
> that's it.  The rest of the code stays as it is.
>
> I'll make define-session do everything that is needed.
>

Ah, so this is what we need sessions for!  I didn't understand your patch
to issue 2872 because i didn't read this thread.  Now things are clear.
Btw: great teamwork!

cheers,
Janek

[janek, 2012-10-05 16:55:27]

Marc wrote:
> (define-bar-line ...) or \defineBarLine allows for new 
> definitions. These functions have four arguments,
> namely the bar line itself, the bar line used at the end 
> of line, the bar line used at the begin of a new line
> and the span bar line.

It just occured to me: is there any way to specify different span bar lines (at
the end of the line and at the beginning of the line)?

Janek

[thomasmorley65, 2012-10-05 19:42:28]

2012/10/5  <janek.lilypond@gmail.com>:
> Marc wrote:
>>
>> (define-bar-line ...) or \defineBarLine allows for new
>> definitions. These functions have four arguments,
>> namely the bar line itself, the bar line used at the end
>> of line, the bar line used at the begin of a new line
>> and the span bar line.
>
>
> It just occured to me: is there any way to specify different span bar
> lines (at the end of the line and at the beginning of the line)?
>
> Janek

Marc and me, we discussed this some time ago and decided not to
provide that functionality.
It would make things more complicated for the user and I think it is a
rarely needed feature.

BTW, you can change the SpanBar at line-end/begin even with 2.14.2
using some scheme:

\version "2.14.2"

#(define (change-span-bar-at-line-begin g)
  (if (and (ly:item? g)
	   (equal? (ly:item-break-dir g) RIGHT))
      (ly:grob-set-property! g 'glyph-name ":")))

cSp = \once\override Score.SpanBar #'after-line-breaking =
#change-span-bar-at-line-begin


\new StaffGroup <<
\new Staff \relative c' { c1 \bar ":|:" \cSp \break d }
\new Staff \relative c' { c1 \break d }
>>

-Harm

[janek, 2012-10-06 09:23:40]

On Fri, Oct 5, 2012 at 9:42 PM, Thomas Morley
<thomasmorley65@googlemail.com> wrote:
> 2012/10/5  <janek.lilypond@gmail.com>:
>> It just occured to me: is there any way to specify different span bar
>> lines (at the end of the line and at the beginning of the line)?
>
> Marc and me, we discussed this some time ago and decided not to
> provide that functionality.
> It would make things more complicated for the user and I think it is a
> rarely needed feature.

Sorry for missing that part of the discussion.
If it's not too late to speak my mind, i think that it would be a
shame to have such an awesome and powerful barline interface without
giving users simple way to choose span barline behaviour.

I would imagine the user interface ("syntax") to be like this:
(define-bar-line (a b c) (x y z))
or
(define-bar-line ((a b c) . (x y z)))

where (a b c) are the barlines (mid-system, end-of-system,
beginning-of-system respectively) and (x y z) are the span barlines,
in the same order as barlines.
And it would be nice if
(define-bar-line (a b c) (x))
was equivalent to
(define-bar-line (a b c) (x x x))

cheers,
Janek

[marc, 2012-10-06 11:47:56]

Am 06.10.2012 11:23, schrieb Janek Warchoł:
> On Fri, Oct 5, 2012 at 9:42 PM, Thomas Morley
> <thomasmorley65@googlemail.com> wrote:
>> 2012/10/5  <janek.lilypond@gmail.com>:
>>> It just occured to me: is there any way to specify different span bar
>>> lines (at the end of the line and at the beginning of the line)?
>> Marc and me, we discussed this some time ago and decided not to
>> provide that functionality.
>> It would make things more complicated for the user and I think it is a
>> rarely needed feature.
> Sorry for missing that part of the discussion.
> If it's not too late to speak my mind, i think that it would be a
> shame to have such an awesome and powerful barline interface without
> giving users simple way to choose span barline behaviour.
>
> I would imagine the user interface ("syntax") to be like this:
> (define-bar-line (a b c) (x y z))
> or
> (define-bar-line ((a b c) . (x y z)))
>
> where (a b c) are the barlines (mid-system, end-of-system,
> beginning-of-system respectively) and (x y z) are the span barlines,
> in the same order as barlines.
> And it would be nice if
> (define-bar-line (a b c) (x))
> was equivalent to
> (define-bar-line (a b c) (x x x))
Any other opinions about this feature?
It may make sense to extend the interface to allow for different
span bars as well, but since I am quite busy for the next few
days, it will take some time...

Regards,

Marc
>
> cheers,
> Janek
>

[marc, 2012-10-06 11:51:54]

Am 30.09.2012 22:03, schrieb dak@gnu.org:
> On 2012/09/30 19:44:49, marc wrote:
>> Am 30.09.2012 11:02, schrieb dak@gnu.org:
>> > [...]
>> > First, the define-public is asking for trouble. You are exposing an
>> > internal Scheme data structure to users and make it overwritable by
> the
>> > user. If the user follows this invitation, the effects will bleed
> over
>> > from session to session. Never do that.
>> Ok.
>
> No, it's not ok.  Hold your horses, this is another case too stupid for
> documenting and walking people through.  Give me two days, and then you
> replace your define-public for the alists with define-session, and
> that's it.  The rest of the code stays as it is.
Hi David,

I just saw that define-session is available now.

I replaced the definition of the empty alists in my patch with 
define-session-public,
but when I compile input/regression/bar-line-define-bar-glyph.ly, I 
still get the
Parsed object should be dead error messages.

Do I use the define-session-public commands in the right way?

As I understand, define-session-public works as define-public, but for 
the current session
only, define-session is the per-session equivalent to define.

Regards,

Marc

>
> I'll make define-session do everything that is needed.
>
> Just that it is possible to learn all the internals of LilyPond session
> does not mean that it makes sense that you are required to do so just in
> order to accomplish a simple task.
>
> http://codereview.appspot.com/6498052/
>

[dak, 2012-10-06 12:05:35]

Marc Hohl <marc@hohlart.de> writes:

> Am 30.09.2012 22:03, schrieb dak@gnu.org:
>> On 2012/09/30 19:44:49, marc wrote:
>>> Am 30.09.2012 11:02, schrieb dak@gnu.org:
>>> > [...]
>>> > First, the define-public is asking for trouble. You are exposing an
>>> > internal Scheme data structure to users and make it overwritable by
>> the
>>> > user. If the user follows this invitation, the effects will bleed
>> over
>>> > from session to session. Never do that.
>>> Ok.
>>
>> No, it's not ok.  Hold your horses, this is another case too stupid for
>> documenting and walking people through.  Give me two days, and then you
>> replace your define-public for the alists with define-session, and
>> that's it.  The rest of the code stays as it is.
> Hi David,
>
> I just saw that define-session is available now.
>
> I replaced the definition of the empty alists in my patch with
> define-session-public,
> but when I compile input/regression/bar-line-define-bar-glyph.ly, I
> still get the
> Parsed object should be dead error messages.
>
> Do I use the define-session-public commands in the right way?
>
> As I understand, define-session-public works as define-public, but for
> the current session
> only, define-session is the per-session equivalent to define.

Correct.

I did not really do a review of the barline code, just pointed out
things that struck me as wrong while glancing over it.

The "parsed objects should be dead" message implies that objects with
data _types_ that should not exist at the start of a session have
survived beyond the end of a session.

So the absence of such messages does not imply that your sessions are
clean, just that there are no blunders that can be detected from the
_types_ of the surviving objects alone.

-- 
David Kastrup

[marc, 2012-10-07 19:43:22]

Am 05.10.2012 21:42, schrieb Thomas Morley:
> 2012/10/5  <janek.lilypond@gmail.com>:
>> Marc wrote:
>>> (define-bar-line ...) or \defineBarLine allows for new
>>> definitions. These functions have four arguments,
>>> namely the bar line itself, the bar line used at the end
>>> of line, the bar line used at the begin of a new line
>>> and the span bar line.
>>
>> It just occured to me: is there any way to specify different span bar
>> lines (at the end of the line and at the beginning of the line)?
>>
>> Janek
> Marc and me, we discussed this some time ago and decided not to
> provide that functionality.
IIRC, I was the one which *proposed* the idea of having different
span bars but decided to continue with one span bar per bar line
only, because I treid to adopt the old c++ way in which the span bars
werde defined and handled.

So this was more or less *my* fault.

Thinking a bit more in detail, I think the input format
\define BarLine #'(bar end-of-line begin-of-line) #'(span-bar 
end-of-line begin-of-line)
or alternatively
\define BarLine #'(bar end-of-line begin-of-line) #'(span-bar)
which expands internally to
\define BarLine #'(bar end-of-line begin-of-line) #'(span-bar span-bar 
span-bar)
is indeed the cleanest solution.

With my current approach, Lilypond raises a warning if the corresponding
span bar to [begin|end]-of-line-bar is not defined.
So your proposal is definitively better.

> It would make things more complicated for the user and I think it is a
> rarely needed feature.
>
>
This was my first opinion, too, but on the other hand, a cryptic warning and
the need to define one or two additional bar lines to get rid of it is not
user-friedly either.

Regards,

Marc

[marc, 2012-10-10 20:08:04]

Am 06.10.2012 11:23, schrieb Janek Warchoł:
> On Fri, Oct 5, 2012 at 9:42 PM, Thomas Morley
> <thomasmorley65@googlemail.com> wrote:
>> 2012/10/5  <janek.lilypond@gmail.com>:
>>> It just occured to me: is there any way to specify different span bar
>>> lines (at the end of the line and at the beginning of the line)?
>> Marc and me, we discussed this some time ago and decided not to
>> provide that functionality.
>> It would make things more complicated for the user and I think it is a
>> rarely needed feature.
> Sorry for missing that part of the discussion.
> If it's not too late to speak my mind, i think that it would be a
> shame to have such an awesome and powerful barline interface without
> giving users simple way to choose span barline behaviour.
Thinking *a lot* more about that and trying to recode my work to
fit your needs I stumbled upon quite a lot of problems:

The internal structure of the properties and its names does not
allow for this extension in a clean way. We have BarLine #'glyph
(that's the string given by the user) and BarLine #'glyph-name
(which is the string for the bar line to be drawn, i.e. after line
breaking decisions).
We have SpanBar #'glyph-name which is computed by
ly:span-bar::calc-glyph-name and the glyph-name we use here
will be that of the corresponding bar line, because we need both
informations about the bar line and the span bar line to get
the spacing correctly.

If I store a span bar triple '(span-bar-glyph end-of-line begin-next-line),
I'll have to carry the whole direction computation stuff in almost
every callback, and moreover, ly:span-bar::calc-glyph-name would
either return a list or the BarLine #'glyph, not the #'glyph-name,
so to be 100% clean, one would have to rename the property into
SpanBar#'glyph-list or SpanBar #'glyph.

Just a side remark: have a look at scm/define-grob-properties.scm.
(glyph-name ,string? "The glyph name within the font.") Huh?
(At least there is some clarification on this issue included in my patch ;-)

Being realistic, there are two possibilities:
a) I just leave the span bar stuff as it is. If it turns out that one 
wants to
change the appearance of span bars depending on the line breaking,
he has to write a custom callback similar to Harm's proposal or has to
rewrite parts of the interface again.
b) I try to get this done in a concise and clean way. This will probably 
take
half a year (just an assumption based on the time I spent on the patch
upto now and the estimated time available for lilypond) – probably I will
fail, but even if not: is it worth the effort? Who needs such span bars?

If no-one convinces me to do otherwise, I'll take (a).

Regards,

Marc

[janek, 2012-10-14 17:17:46]

I dedicate all my code reviews to Graham Percival.

Marc,
your change is so big that i have trouble grasping it and understanding
everything, but i have a few specific comments.

Janek

http://codereview.appspot.com/6498052/diff/50001/Documentation/snippets/new/p...
File
Documentation/snippets/new/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly
(right):

http://codereview.appspot.com/6498052/diff/50001/Documentation/snippets/new/p...
Documentation/snippets/new/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly:1:
\version "2.17.5"
I see this snippet both as new snippet in Documentation/snippets/new and as
modified snippet in Documentation/snippets.  Is this correct?

http://codereview.appspot.com/6498052/diff/50001/Documentation/snippets/print...
File
Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly
(right):

http://codereview.appspot.com/6498052/diff/50001/Documentation/snippets/print...
Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly:20:

is this empty line intended?

http://codereview.appspot.com/6498052/diff/50001/lily/volta-bracket.cc
File lily/volta-bracket.cc (right):

http://codereview.appspot.com/6498052/diff/50001/lily/volta-bracket.cc#newcod...
lily/volta-bracket.cc:141: ly_string2scm (str)));
I'm sorry (that's probably a noob question) - what |= is?

http://codereview.appspot.com/6498052/diff/50001/python/convertrules.py
File python/convertrules.py (right):

http://codereview.appspot.com/6498052/diff/50001/python/convertrules.py#newco...
python/convertrules.py:3394: 
is this blank line intentional?

[mail_philholmes.net, 2012-10-14 17:51:55]

----- Original Message ----- 
From: <janek.lilypond@gmail.com>
To: <marc@hohlart.de>; <mtsolo@gmail.com>; <thomasmorley65@googlemail.com>; 
<ianhulin44@gmail.com>; <k-ohara5a5a@oco.net>; <dak@gnu.org>; 
<mike@mikesolomon.org>
Cc: <reply@codereview-hr.appspotmail.com>; <lilypond-devel@gnu.org>
Sent: Sunday, October 14, 2012 6:17 PM
Subject: Re: bar-line interface part 2/2: New bar line definition 
standard(issue 6498052)


>
http://codereview.appspot.com/6498052/diff/50001/Documentation/snippets/new/p...
>
Documentation/snippets/new/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly:1:
> \version "2.17.5"
> I see this snippet both as new snippet in Documentation/snippets/new and
> as modified snippet in Documentation/snippets.  Is this correct?


The rule is: any snippet that will run in 2.14 - change it by editing the 
LSR - if you can't do this, ask.  Any snippet that will only run on a later 
version: change it by adding it to snippets/new.  If you want it to be 
immediately usable, run makelsr, which will update it into snippets.  NEVER 
change a snippet in the snippets directory - when I run makelsr, any other 
changes will be obliterated.

--
Phil Holmes 

[marc, 2012-10-14 18:01:32]

Am 14.10.2012 19:17, schrieb janek.lilypond@gmail.com:
> I dedicate all my code reviews to Graham Percival.
>
> Marc,
> your change is so big that i have trouble grasping it and understanding
> everything, but i have a few specific comments.
>
> Janek
>
>
>
http://codereview.appspot.com/6498052/diff/50001/Documentation/snippets/new/p...

>
> File
>
Documentation/snippets/new/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly

>
> (right):
>
>
http://codereview.appspot.com/6498052/diff/50001/Documentation/snippets/new/p...

>
>
Documentation/snippets/new/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly:1:

>
> \version "2.17.5"
> I see this snippet both as new snippet in Documentation/snippets/new and
> as modified snippet in Documentation/snippets.  Is this correct?
The CG says so, otherwise it would be removed by the next makelsr call.
>
>
http://codereview.appspot.com/6498052/diff/50001/Documentation/snippets/print...

>
> File
> Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly 
>
> (right):
>
>
http://codereview.appspot.com/6498052/diff/50001/Documentation/snippets/print...

>
>
Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly:20:

>
>
> is this empty line intended?
You don't like it? ;-)

Sorry, will remove it before pushing to staging.
>
> http://codereview.appspot.com/6498052/diff/50001/lily/volta-bracket.cc
> File lily/volta-bracket.cc (right):
>
>
http://codereview.appspot.com/6498052/diff/50001/lily/volta-bracket.cc#newcod...

>
> lily/volta-bracket.cc:141: ly_string2scm (str)));
> I'm sorry (that's probably a noob question) - what |= is?
It's the same as a+= 2, just with a bitwise or.
>
> http://codereview.appspot.com/6498052/diff/50001/python/convertrules.py
> File python/convertrules.py (right):
>
>
http://codereview.appspot.com/6498052/diff/50001/python/convertrules.py#newco...

>
> python/convertrules.py:3394:
> is this blank line intentional?
See above.
>
> http://codereview.appspot.com/6498052/
>