std.d.lexer: pre-voting review / discussion

[Jacob Carlborg]

On 2013-09-11 17:01, Dicebot wrote:
> std.d.lexer is standard module for lexing D code, written by Brian Schott

Finally :)

* How does it handler errors, just returns TokenType.invalid?

* Personally I think the module is too big. I would go with:

- std.d.lexer.token
- std.d.lexer.tokentype
- std.d.lexer.lexer - contains the rest
- std.d.lexer.config - IterationStyle, TokenStyle, LexerConfig
- CircularRange, StringCache, possibly put somewhere else. I assume this 
can be used for other things than lexing?
- Trie related code, same as above

* I see that errorMessage throws an exception. Do we really want that? I 
would except it just returns an invalid token.

If we do decide it should throw, it should absolutely _not_ throw a 
plain Exception. Create a new type, LexException or similar. I hate when 
code throws plain Exceptions, it makes it useless to catch them.

I would also expect this LexException to contain a Token. It shouldn't 
be needed to parse the exception message to get line and column information.

* I like that you overall use clear and descriptive variable and 
function names. Except "sbox": 
https://github.com/Hackerpilot/phobos/blob/master/std/d/lexer.d#L3265

* Could we get some unit tests for string literals, comments and 
identifies out side of the ASCII table

* I would like to see a short description for each unit test, what it's 
testing. Personally I have started with this style:

@describe("byToken")
{
     @context("valid string literal")
     {
         @it("should return a token with the type 
TokenType.stringLiteral") unittest
         {
             // test
         }

         @it("should return a token with the correct lexeme") unittest
         {
             // test
         }
     }
}

Better formatted: http://pastebin.com/Dx78Vw6r

People here might think that would be a bit too verbose. The following 
would be ok as well:

@describe("short description of the unit test") unittest { }

* Could you remove debug code and other code that is commented out:

- 344
- 1172
- 1226, is that needed?
- 3165-3166
- 3197-3198
- 3392
- 3410
- 3434

Spell errors:

* "forwarad" - 292
* "commemnt" - 2031
* "sentenels" - 299
* "messsage" - 301
* "underliying" - 2454
* "alloctors" - 3230
* "strightforward" - 2276

-- 
/Jacob Carlborg