Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3609)

Issue 6445056: Unify the lexer's idea of words and commands across all modes. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 8 months ago by dak
Modified:
11 years, 7 months ago
Reviewers:
Keith, Trevor Daniels, t.daniels
CC:
lilypond-devel_gnu.org
Base URL:
http://git.savannah.gnu.org/gitweb/?p=lilypond.git/trunk/
Visibility:
Public.

Description

Unify the lexer's idea of words and commands across all modes. A "word" (a string recognized even when not quoted) and a "command" (something starting with \ and followed by letters and other folderol, indicating a Scheme control sequence or similar) get the same syntax in all modes: A "word" is a sequence of alphabetic characters possibly containing single dashes or underlines inside (not at the beginning or end). A "command" is a "word" preceded by a backslash. This is a large syntax change. It should not be put on countdown automatically.

Patch Set 1 #

Total comments: 1

Patch Set 2 : CHANGES entry, change uses. #

Patch Set 3 : Add explanation of {RESTNAME}/[-_] pattern #

Total comments: 4

Patch Set 4 : Add comments to backup rules #

Unified diffs Side-by-side diffs Delta from patch set Stats (+67 lines, -31 lines) Patch
M Documentation/changes.tely View 1 1 chunk +21 lines, -0 lines 0 comments Download
M Documentation/de/notation/fretted-strings.itely View 1 1 chunk +1 line, -1 line 0 comments Download
M Documentation/es/notation/fretted-strings.itely View 1 1 chunk +1 line, -1 line 0 comments Download
M Documentation/fr/notation/fretted-strings.itely View 1 1 chunk +1 line, -1 line 0 comments Download
M Documentation/ja/notation/fretted-strings.itely View 1 1 chunk +1 line, -1 line 0 comments Download
M Documentation/notation/fretted-strings.itely View 1 1 chunk +1 line, -1 line 0 comments Download
M input/regression/safe.ly View 1 2 3 2 chunks +3 lines, -4 lines 0 comments Download
M lily/lexer.ll View 1 2 3 9 chunks +29 lines, -17 lines 0 comments Download
M ly/music-functions-init.ly View 1 2 chunks +2 lines, -2 lines 0 comments Download
M ly/string-tunings-init.ly View 1 2 chunks +2 lines, -2 lines 0 comments Download
M python/convertrules.py View 1 1 chunk +5 lines, -1 line 0 comments Download

Messages

Total messages: 8
Trevor Daniels
Just a query really, to help my understanding. Trevor http://codereview.appspot.com/6445056/diff/1/lily/lexer.ll File lily/lexer.ll (right): http://codereview.appspot.com/6445056/diff/1/lily/lexer.ll#newcode390 lily/lexer.ll:390: ...
11 years, 8 months ago (2012-07-31 08:47:53 UTC) #1
dak
On 2012/07/31 08:47:53, Trevor Daniels wrote: > Just a query really, to help my understanding. ...
11 years, 8 months ago (2012-07-31 10:52:41 UTC) #2
t.daniels_treda.co.uk
<dak@gnu.org> wrote Tuesday, July 31, 2012 11:52 AM >> http://codereview.appspot.com/6445056/diff/1/lily/lexer.ll#newcode390 >> lily/lexer.ll:390: <chords,notes,figures>{RESTNAME}/[-_] | >> ...
11 years, 8 months ago (2012-07-31 16:56:51 UTC) #3
Trevor Daniels
I can't test this, but eyeballing LGTM.
11 years, 8 months ago (2012-08-02 07:42:17 UTC) #4
dak
On 2012/07/31 16:56:51, t.daniels_treda.co.uk wrote: > <mailto:dak@gnu.org> wrote Tuesday, July 31, 2012 11:52 AM > ...
11 years, 8 months ago (2012-08-02 10:36:25 UTC) #5
Trevor Daniels
LGTM
11 years, 8 months ago (2012-08-02 18:38:45 UTC) #6
Keith
I'm pouting a little that this moves further from the ability to write \violin1mvt3a = ...
11 years, 7 months ago (2012-08-04 06:34:57 UTC) #7
dak
11 years, 7 months ago (2012-08-04 07:15:29 UTC) #8
On 2012/08/04 06:34:57, Keith wrote:
> I'm pouting a little that this moves further from the ability to write
>   \violin1mvt3a = { c d }
> but it still looks fine to me.

While I agree that allowing digits into identifiers would not make sense unless
one gets them allowed in all modes (or the whole point would have been lost), we
always would have had the ambiguity of c4 having to be distinguished from a word
c4, and having to check both c and c4 for being a notename would be quite
strange.

violin1mvt3a is not necessarily ruled out though as it may be covered by issue
2072 instead: it does not need to be just one single identifier.

> http://codereview.appspot.com/6445056/diff/8001/lily/lexer.ll
> File lily/lexer.ll (left):
> 
> http://codereview.appspot.com/6445056/diff/8001/lily/lexer.ll#oldcode159
> lily/lexer.ll:159: DASHED_WORD		{A}({AN}|-)*
> We will get complaints if anyone said 
>   \paper { indent3 = 3 \cm  indent = \indent3 }
> but I hope no-one ever did such a thing.

I doubt that people found this rather confined place where identifiers may
contain digits after all.  The rules were rather haphazard and never spelled
out.

> http://codereview.appspot.com/6445056/diff/8001/lily/lexer.ll
> File lily/lexer.ll (right):
> 
> http://codereview.appspot.com/6445056/diff/8001/lily/lexer.ll#newcode158
> lily/lexer.ll:158: WORD		{A}([-_]{A}|{A})*
> It would be so nice to say {A}({N}*[-_]?{A})*
> and address issue 1670 ...  except for chord-mode

This would give you violin1mvt3a but not violin1mvt2 and I have my doubts you
could explain this to anybody.  Digits in identifiers in _intermediate_ position
only don't really make a lot of sense.

> I guess I'll give up on using digits, and on issue one thousand six hundred
> seventy.
> 
> http://codereview.appspot.com/6445056/diff/8001/lily/lexer.ll#newcode397
> lily/lexer.ll:397: <chords,notes,figures>{RESTNAME}/[-_]	|
> Probably also the comment  // backup rule
> because it follows (indirectly) from the rule about no backup states.

I'll put pseudo-backup rule here.

> http://codereview.appspot.com/6445056/diff/8001/lily/lexer.ll#newcode483
> lily/lexer.ll:483: {WORD}/[-_]	|
> Certainly here // backup rule
> because all the other silly states we put in to follow that silly rule have
the
> comment, so it would be confusing to be inconsistent.
> 
> Similarly line four hundred eighty-eight, line five hundred forty-two ...

Thanks.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b