Re: [PATCH] Add scheme engraver for StaffTab notation

[David Kastrup]

Werner LEMBERG <address@hidden> writes:

>>> It's not clear to me how an alist of tuning names and the
>>> associated string tunings is `utterly incomprehensible'...  Oh
>>> well.
>> 
>> So what is the purpose of having _strings_ here?
>
> For me this is the most natural form to present input that gets
> massaged later on.
>
>> What does the . mean?  What do the parens mean?
>
> Uh, oh, ... alist ...  Please don't pretend to be a noob.
>
>> To hide from the editor and reader what one is doing?  So that you
>> don't get syntax highlighting and become unable to convert notenames
>> or absolute/relative?  Not everything that is possible to do is a
>> good idea.
>
> Huh?  Are we still talking about string tunings?

We are talking about organizing input to LilyPond.

> What you do here is trick number 3 in Schopenhauer's `Eristic
> Dialectic': Generalize your opponent's specific statements.

Maintenance of a code base is facilitated by consistent use of tools and
techniques.  There is nothing specific about using generated input
streams here in particular.

> Please don't do that.
>
>> One error already introduced by yourself is that
>> guitar-drop-c-tuning will be undefined since instead you are
>> defining guitar-drop-c-tuning-tuning.
>
> Yep, copy-and-paste error.

No.  The error was that you _did_ a proper copy and paste without
further mangling of the input.

>> Which stresses my point about it being a bad idea to use different
>> words in definition and use.
>
> Well, lilypond happily accepts a line like
>
>   \makeDefaultStringTuning #'guitar-asus4-tunink \stringTuning <e, a, d e a 
> e'>
>
> without any error message.  Your argument is thus weak.

M-/ in my editor completes a command whenever the command is found
elsewhere in the reach of the editor.  When definition and use don't
match, this does not work, introducing the possibility for error that
would otherwise not be there.

It also makes it hard to find the definition using "git grep" or other
tools when only a partial string is used.

>> The next thing to note is that the error messages are entirely
>> incomprehensible
>
> Again, I wouldn't call an error message `entirely incomprehensible' if
> the offending line and position in the line is displayed.

They aren't.  Have you even looked at the error messages I quoted?

I repeat:

<included string>:1:83: error: syntax error, unexpected >>
\makeDefaultStringTuning guitar-drop-c-tuning-tuning \stringTuning <c, g, c f a 
d'
                                                                                
  >>
<included string>:1:77: error: syntax error, unexpected >
\makeDefaultStringTuning guitar-drop-d-tuning \stringTuning <<d, a, d g b e'

What do you think will happen when I click on the latter error message
in my compilation buffer?  Why would Emacs position me at the line
looking like

      ("guitar-drop-d" . "<d, a, d g b e'")

in a file that is not mentioned in the error message on a line that is
not mentioned in the error message?

>> Of course, this also means that the input will be read, parsed, and
>> tokenized, several times in a row, making this also quite less
>> efficient.
>
> This, however, is indeed true and a valid argument.  Just as a side
> note for others who are reading this thread: the same is true for
> `#{ ... #}', which is just syntactical sugar around
> `ly:parser-parse-string', a variant of `ly:parser-include-string'.

Yes and no.  The content of #{ ... #} is not passed through the Scheme
reader.  While its internals are stored in strings, there is no string
input syntax inside of #{ ... #} and you do not need to extra-quote " by
writing \" instead.  Errors in the code inside will point to the correct
line and file, and point-and-click will work.  It is a fully implemented
facility rather than a local hack.

>> It is a good practice to have the syntactical context of input be
>> its location in the file, without juggling it around between
>> interpreters and input stacks manually.
>> 
>> Otherwise, error messages cannot properly track the source
>> locations, and the editor cannot help with formatting and
>> highlighting the source code appropriately.  And you avoid
>> recombining tokens and several tokenizers working in sequence on the
>> same input, requiring escaping of quote characters and other stuff.
>
> I agree.  *However*, there are IMHO situations where some sweeps with
> a magic wand makes everything looks more nicely.
>
> Just for fun: Attached are the original and my massaged version of
> `string-tunings-init.ly'.  Is my version really *that*
> incomprehensible?

Your version is more compact.  The highlighting of the editor does not
recognize the pitches as pitches, and the names of the tunings do not
correspond to the actual tunings as documented.  You cannot copy, paste,
and modify any line to user-code and hope that it still works.  You
cannot add your own variation of such a line in user code.

I can do that with any of the following lines, either using existing
names or new ones:

> %% guitar tunings
> \makeDefaultStringTuning #'guitar-tuning \stringTuning <e, a, d g b e'>
> \makeDefaultStringTuning #'guitar-seven-string-tuning \stringTuning <b,, e, 
> a, d g b e'>

I cannot do it with the following:

> #(for-each
>   (lambda (elem)
>     (let* ((sym (car elem))
>            (tuning (cdr elem)))
>       (ly:parser-include-string
>        parser
>        (string-append "\\makeDefaultStringTuning #'" sym "-tuning"
>                     " \\stringTuning <" tuning ">"))))
>
>   '(
>     ;; guitar tunings
>     ("guitar" . "e, a, d g b e'")
>     ("guitar-seven-string" . "b,, e, a, d g b e'")
[...]
>     ))

If I modify the file (which is very much not recommended) and make a
mistake, I don't get a useful error message.

If you take a look at the history of the file, you'll hit issue 1987
with the code review at <URL:https://codereview.appspot.com/5318046>.

You'll see that the original form of the file in
<URL:https://codereview.appspot.com/5318046/patch/3007/17009> looked
like

%% Define alist of default string tunings
defaultStringTunings =
#`(
 ;; guitar tunings
 (guitar-tuning . ,#{<e, a, d g b e'>#})
 (guitar-seven-string-tuning . ,#{<b,, e, a, d g b e'>#})
 (guitar-drop-d-tuning . ,#{<d, a, d g b e'>#})

That was similarly compact to your proposal (and as un-extensible, but
at least making it obvious how to add stuff to defaultStringTunings),
but did not mess around with strings and constructed input.
guitar-tuning was present as a full symbol, and the stuff intended to
end up in LilyPond syntax was entered as #{<e, a, d g b e'>#} at least.

This file actually was a significant source of debug output at that
time (since #{...#} was tracked as an input switch in verbose mode), and
it also had performance impact due to the frequent mode switching.

The current solution does not switch modes apart from jumping into
Scheme mode for #'guitar-tuning and we could easily get rid of that #'
as well (yes, that will actually cause a measurable speedup as I found
out in the course of issue 2883).

I consider your proposal worse than what we had in 2.14.0.  One reason
is that \makeDefaultStringTuning becomes rather pointless as an
individual command when the original data is placed into one humongous
data structure anyway.  So you get the complexity of the 2.16 solution
without its flexibility.  And if you make a syntax error in either the
Scheme or LilyPond parts of the 2.14 definition, the error messages will
point to the correct lines.

-- 
David Kastrup