Clear fret-diagram- and harp-pedal-input-strings from whitespace

Clear fret-diagram- and harp-pedal-input-strings from whitespace

Whitespace-characters are deleted before further processing.

Allows for
\markup \fret-diagram #"s:2;h:5;
6-3;5-5;4-5;3-4;2-3;1-x;"

Also adding errors for typos in fret-diagram with a meaningful message:
\markup \fret-diagram #"s:2;g:5;
6-3;5-5;4-5;3-4;2-3;1-x;"
will return:
fatal error: Unhandled entry in fret-diagram: g:5

This error would not apply, if something for #\g would be defined
in fret-parse-definition-string somewhere in the future.

Something similiar is already in harp-pedals.scm

[thomasmorley651, 2015-08-23 14:09:59]

please review

[dak, 2015-08-23 15:11:08]

https://codereview.appspot.com/257580043/diff/1/scm/fret-diagrams.scm
File scm/fret-diagrams.scm (right):

https://codereview.appspot.com/257580043/diff/1/scm/fret-diagrams.scm#newcode45
scm/fret-diagrams.scm:45: (define (not-number-error input output)
"input" insinuates a port input, like "output" does.  The doc string does not
even mention "output", and from the error message it is supposed to be an
"unhandled entry in fret-diagram" which would rather seem like some kind of
input rather than output.

[pkx166h, 2015-08-24 10:56:02]

Passes make, make check and a full make doc.

PATCH-REVIEW

[pkx166h, 2015-08-26 07:08:59]

Patch on countdown for August 29th

PATCH_COUNTDOWN

[dak, 2015-08-26 07:12:03]

On 2015/08/23 15:11:08, dak wrote:
> https://codereview.appspot.com/257580043/diff/1/scm/fret-diagrams.scm
> File scm/fret-diagrams.scm (right):
> 
>
https://codereview.appspot.com/257580043/diff/1/scm/fret-diagrams.scm#newcode45
> scm/fret-diagrams.scm:45: (define (not-number-error input output)
> "input" insinuates a port input, like "output" does.  The doc string does not
> even mention "output", and from the error message it is supposed to be an
> "unhandled entry in fret-diagram" which would rather seem like some kind of
> input rather than output.

Ping?

[pkx166h, 2015-08-26 07:13:23]

Yes sorry. I am still trying to get used to reviewing this way. One does get
stuck in a system.

I am putting this back to PATCH_REVIEW for now.

[dak, 2015-08-26 08:02:34]

On 2015/08/26 07:13:23, J_lowe wrote:
> Yes sorry. I am still trying to get used to reviewing this way. One does get
> stuck in a system.
> 
> I am putting this back to PATCH_REVIEW for now.

Actually, this was more of a reminder for Thomas.

[dak, 2015-08-26 08:55:14]

https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm
File scm/lily-library.scm (right):

https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm#newcode793
scm/lily-library.scm:793: char-set:whitespace))
A mostly theoretic musing: in the predicates documented in NR, we have
`char-whitespace?' listed in "R5RS secondary predicates".

I don't know how it's implemented, but if people compare results,
char-whitespace? might be the expected point of reference and it's not like this
is performance-critical.

Using that would mean one less function in our codebase for the same
functionality.

I just thought I'd mention it: it's by no means a necessary change or even a
showstopper.

[thomasmorley651, 2015-08-26 09:24:54]

On 2015/08/26 08:55:14, dak wrote:
> https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm
> File scm/lily-library.scm (right):
> 
>
https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm#newcode793
> scm/lily-library.scm:793: char-set:whitespace))
> A mostly theoretic musing: in the predicates documented in NR, we have
> `char-whitespace?' listed in "R5RS secondary predicates".
> 
> I don't know how it's implemented, but if people compare results,
> char-whitespace? might be the expected point of reference and it's not like
this
> is performance-critical.
> 
> Using that would mean one less function in our codebase for the same
> functionality.
> 
> I just thought I'd mention it: it's by no means a necessary change or even a
> showstopper.

Sorry, too busy with non LilyPond work right now. Maybe I can continue the
upcoming weekend. For now setting

PATCH-NEEDS_WORK

[thomasmorley651, 2015-08-29 17:19:13]

no predefined error-def, apply to barre-list, too

[thomasmorley651, 2015-08-29 17:25:44]

On 2015/08/23 15:11:08, dak wrote:
> https://codereview.appspot.com/257580043/diff/1/scm/fret-diagrams.scm
> File scm/fret-diagrams.scm (right):
> 
>
https://codereview.appspot.com/257580043/diff/1/scm/fret-diagrams.scm#newcode45
> scm/fret-diagrams.scm:45: (define (not-number-error input output)
> "input" insinuates a port input, like "output" does.  The doc string does not
> even mention "output", and from the error message it is supposed to be an
> "unhandled entry in fret-diagram" which would rather seem like some kind of
> input rather than output.

In the new patch-set I deleted this definition, because checking barre-list with
this would have need more additional code. Not worth the effort.
Instead I used single errors.

[thomasmorley651, 2015-08-29 17:28:33]

On 2015/08/26 08:55:14, dak wrote:
> https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm
> File scm/lily-library.scm (right):
> 
>
https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm#newcode793
> scm/lily-library.scm:793: char-set:whitespace))
> A mostly theoretic musing: in the predicates documented in NR, we have
> `char-whitespace?' listed in "R5RS secondary predicates".
> 
> I don't know how it's implemented, but if people compare results,
> char-whitespace? might be the expected point of reference and it's not like
this
> is performance-critical.
> 
> Using that would mean one less function in our codebase for the same
> functionality.
> 
> I just thought I'd mention it: it's by no means a necessary change or even a
> showstopper.

Well, I could use `char-whitespace?' to write a different definition. Ofcourse a
new definition would still be there.
I've the feeling I don't really understand your proposal...

[thomasmorley651, 2015-08-29 17:31:42]

2015-08-26 11:24 GMT+02:00  <thomasmorley65@gmail.com>:

> Sorry, too busy with non LilyPond work right now. Maybe I can continue
> the upcoming weekend. For now setting
>
> PATCH-NEEDS_WORK
>
> https://codereview.appspot.com/257580043/

I've uploaded a new patch-set to
https://codereview.appspot.com/257580043/

Setting:

PATCH-NEW

[dak, 2015-08-29 18:13:13]

On 2015/08/29 17:28:33, thomasmorley651 wrote:
> On 2015/08/26 08:55:14, dak wrote:
> > https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm
> > File scm/lily-library.scm (right):
> > 
> >
>
https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm#newcode793
> > scm/lily-library.scm:793: char-set:whitespace))
> > A mostly theoretic musing: in the predicates documented in NR, we have
> > `char-whitespace?' listed in "R5RS secondary predicates".
> > 
> > I don't know how it's implemented, but if people compare results,
> > char-whitespace? might be the expected point of reference and it's not like
> this
> > is performance-critical.
> > 
> > Using that would mean one less function in our codebase for the same
> > functionality.
> > 
> > I just thought I'd mention it: it's by no means a necessary change or even a
> > showstopper.
> 
> Well, I could use `char-whitespace?' to write a different definition. Ofcourse
a
> new definition would still be there.
> I've the feeling I don't really understand your proposal...

The proposal is replacing char-set:whitespace with char-whitespace? which is
already being used in LilyPond and advertised to the user.  Just a
single-identifier change.

[thomasmorley651, 2015-08-29 19:11:17]

David's comment

[thomasmorley651, 2015-08-29 19:14:46]

On 2015/08/29 18:13:13, dak wrote:
> On 2015/08/29 17:28:33, thomasmorley651 wrote:
> > On 2015/08/26 08:55:14, dak wrote:
> > > https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm
> > > File scm/lily-library.scm (right):
> > > 
> > >
> >
>
https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm#newcode793
> > > scm/lily-library.scm:793: char-set:whitespace))
> > > A mostly theoretic musing: in the predicates documented in NR, we have
> > > `char-whitespace?' listed in "R5RS secondary predicates".
> > > 
> > > I don't know how it's implemented, but if people compare results,
> > > char-whitespace? might be the expected point of reference and it's not
like
> > this
> > > is performance-critical.
> > > 
> > > Using that would mean one less function in our codebase for the same
> > > functionality.
> > > 
> > > I just thought I'd mention it: it's by no means a necessary change or even
a
> > > showstopper.
> > 
> > Well, I could use `char-whitespace?' to write a different definition.
Ofcourse
> a
> > new definition would still be there.
> > I've the feeling I don't really understand your proposal...
> 
> The proposal is replacing char-set:whitespace with char-whitespace? which is
> already being used in LilyPond and advertised to the user.  Just a
> single-identifier change.

Hope I've got it. Thanks.

PATCH-NEW

[pkx166h, 2015-08-30 07:31:51]

Passes make, make check and a full make doc

PATCH_REVIEW