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
