Issue 8506043: parser: more specific error messages; issue 3300 (Closed)
Descriptionparser: more specific error messages; issue 3300
Patch Set 1 #
Total comments: 2
Patch Set 2 : add the "(without \\lyricmode)" #Patch Set 3 : "unexpected string, outside of \lyricmode" #
Total comments: 3
Patch Set 4 : shorter messages #MessagesTotal messages: 9
https://codereview.appspot.com/8506043/diff/1/lily/parser.yy File lily/parser.yy (right): https://codereview.appspot.com/8506043/diff/1/lily/parser.yy#newcode2323 lily/parser.yy:2323: | STRING This not fully equivalent, but I think that LYRIC_ELEMENT is not expected to occur here, so it is likely a reasonable choice of yours not to mention it here. Ok. https://codereview.appspot.com/8506043/diff/1/lily/parser.yy#newcode3118 lily/parser.yy:3118: lyric_element optional_notemode_duration post_events { Here is the problem I have with this change: for \new Lyrics { hello, Dolly } the error message will become quite less helpful. I have no good idea how to distinguish these two error classes (this one, and the one you are thinking of) reliably in the syntax, so it seems that we should design an error message that covers both cases. Apropos: do you have an example of where your refactoring leads to a better error behavior (independent from the error message text itself)?
Sign in to reply to this message.
On 2013/04/08 16:05:20, dak wrote:
>
> Here is the problem I have with this change: for
> \new Lyrics { hello, Dolly }
> the error message will become quite less helpful.
Yep. Version 2.16 printed the same "unexpected STRING".
I see no safe short-term way to give the parser a clue that \new Lyrics probably
wanted \lyricmode (given that users are allowed to change contexts). The best I
can think of is
"unexpected string (without \\lyricmode)"
> Apropos: do you have an example of where your refactoring leads to
> a better error behavior (independent from the error message text
> itself)?
No. Finding the errors earlier simply allows shorter accurate messages.
(remember the struggle to find a non-confusing message for issue 3049)
Sign in to reply to this message.
On 2013/04/08 17:58:43, Keith wrote:
> On 2013/04/08 16:05:20, dak wrote:
> >
> > Here is the problem I have with this change: for
> > \new Lyrics { hello, Dolly }
> > the error message will become quite less helpful.
>
> Yep. Version 2.16 printed the same "unexpected STRING".
>
> I see no safe short-term way to give the parser a clue that \new Lyrics
probably
> wanted \lyricmode (given that users are allowed to change contexts). The best
I
> can think of is
> "unexpected string (without \\lyricmode)"
In TeX, this error message would likely have looked like
"unexpected string (did you forget \\lyricmode ?)"
> > Apropos: do you have an example of where your refactoring leads to
> > a better error behavior (independent from the error message text
> > itself)?
>
> No. Finding the errors earlier simply allows shorter accurate messages.
You are aware that those two sentences flatly contradict each other?
Sign in to reply to this message.
On 2013/04/08 18:07:50, dak wrote: > On 2013/04/08 17:58:43, Keith wrote: > > On 2013/04/08 16:05:20, dak wrote: > > > > Apropos: do you have an example of where your refactoring leads to > > > a better error behavior (independent from the error message text > > > itself)? > > > > No. Finding the errors earlier simply allows shorter accurate messages. > > You are aware that those two sentences flatly contradict each other? I simply couldn't fail to disagree with you less. Nobody didn't never use no canceling negatives nor negative qualifiers on yes/no questions where I grew up. Nevertheless, I think I might have answered correctly above. No, I do not have any example of where my *fancy-word* leads to a better error behavior, other than to allow the error message text itself to be more specific.
Sign in to reply to this message.
On 2013/04/12 04:07:15, Keith wrote: > On 2013/04/08 18:07:50, dak wrote: > > On 2013/04/08 17:58:43, Keith wrote: > > > On 2013/04/08 16:05:20, dak wrote: > > > > > > Apropos: do you have an example of where your refactoring leads to > > > > a better error behavior (independent from the error message text > > > > itself)? > > > > > > No. Finding the errors earlier simply allows shorter accurate messages. > > > > You are aware that those two sentences flatly contradict each other? > > I simply couldn't fail to disagree with you less. > > Nobody didn't never use no canceling negatives nor negative qualifiers on yes/no > questions where I grew up. Nevertheless, I think I might have answered > correctly above. > > No, I do not have any example of where my *fancy-word* leads to a better error > behavior, other than to allow the error message text itself to be more specific. I think the misunderstanding here might be your use of "earlier": in either case, exactly the same expression is being flagged, so the difference you can produce is not in the error location but just in the text. Your change will flag the same expression as previously but describe its _type_ better, at the cost of omitting to mention a possible _reason_ for the problem. That is not helpful for diagnosing the problem. It is merely helpful for suggesting that the error is not a random obstinacy without rhyme or reason. The change you made, namely "unexpected markup (without \\lyricmode)" does not do much to suggest LilyPond is not just being obtuse. Maybe something like "unexpected markup (missing \\lyricmode ?)" is better here? After all, \lyricmode is just a reasonable guess, not more or less. Or something more verbose like "a markup here would require \\lyricmode".
Sign in to reply to this message.
On 2013/04/12 05:58:58, dak wrote: > Or something more verbose like > "a markup here would require \\lyricmode". That exact wording would likely let the uset write \lyricmode \markup ... which will not be helpful. So perhaps rather "a markup here would require to be in \\lyricmode"
Sign in to reply to this message.
On 2013/04/12 06:05:42, dak wrote:
> On 2013/04/12 05:58:58, dak wrote:
> > Or something more verbose like
> > "a markup here would require \\lyricmode".
>
> That exact wording would likely let the uset write \lyricmode \markup ...
which
> will not be helpful. So perhaps rather
>
> "a markup here would require to be in \\lyricmode"
Or at least "unexpected markup (outside of \\lyricmode)" instead of "unexpected
markup (without \\lyricmode)". Yes, this conflates command and mode, but that
is more likely to give the user a hint as to what is wrong as the current
message does ("Lyric mode" does not help much with diagnosing what is wrong with
\new Lyrics { Hello }).
Sign in to reply to this message.
https://codereview.appspot.com/8506043/diff/13001/lily/parser.yy File lily/parser.yy (right): https://codereview.appspot.com/8506043/diff/13001/lily/parser.yy#newcode2995 lily/parser.yy:2995: parser->parser_error (@1, _ ("unexpected markup, without any ^ _ or -, and outside of \\lyricmode")); The error message is a bit too cumbersome. Maybe "markup outside of text script or \\lyricmode". https://codereview.appspot.com/8506043/diff/13001/lily/parser.yy#newcode3000 lily/parser.yy:3000: parser->parser_error (@1, _ ("unexpected string, or unrecognized note-name, outside of \\lyricmode")); Strings can also be in text scripts.
Sign in to reply to this message.
https://codereview.appspot.com/8506043/diff/13001/lily/parser.yy File lily/parser.yy (right): https://codereview.appspot.com/8506043/diff/13001/lily/parser.yy#newcode2995 lily/parser.yy:2995: parser->parser_error (@1, _ ("unexpected markup, without any ^ _ or -, and outside of \\lyricmode")); On 2013/08/30 08:52:27, dak wrote: > The error message is a bit too cumbersome. Maybe "markup outside of text script > or \\lyricmode". Yep. That might let the whole message fit on a terminal line.
Sign in to reply to this message.
|
