Add newline to the parsing grammar to resolve some bugs

[jmbeck15]

Summary

By adding newline (\n) to the grammar, this pull request will:

• Resolve There's a funny bug with the way newlines are handled #44
• Resolve Duplicate arugments when a newline preceeds arguments without an explicit command #55
• Fix some erroneous tests
• Add some new tests

It may be easier to review by commit than in bulk.

Description

Newlines are special for GCode; they represent the end of the current command and the start of a new one. This commit makes newlines a first-class token, and parses with them under consideration.

How I tested

I added some tests, un-ignored or fixed other tests, and executed cargo test.

I also ran a bunch of gcode files through the parser as a sanity check. In retrospect, I should have created tests from them, so I may go back and do that if I need to make further changes.

[Michael-F-Bryan]

Thanks for making this PR @jmbeck15! I think adding newlines as an explicit token in the language totally makes sense.

Overall I'm pretty happy with the PR, but there were a couple small questions I'd like to get your thoughts on.

[Michael-F-Bryan]

If the only way this function can be called is when we're looking at a \n character, would it be enough to add something like debug_assert!(self.rest().starts_with("\n")) and return a Token<'input>?

Otherwise, we should actually check for the newline character and return None if it isn't found.

Either way, I think we should try to use the newline characters from the original stream instead of writing let value = "\n".

[jmbeck15]

I think what you're getting at is "this code has a weird smell", and I think you're right. 😁 But I'm honestly not sure how to construct this better. If we don't explicitly set value = "\n", we'd have to chomp() or somehow otherwise pull the newline from the string, which is a computational expense that seems superfluous.

As for your suggestion

would it be enough to add something like debug_assert!(self.rest().starts_with("\n")) and return a Token<'input>

I don't think I understand. The debug_assert is something I could add; seems reasonable. But how would you return the Token<'input>?

[Michael-F-Bryan]

It would probably be more readable if we rewrite the test to be something like this:

let expected = vec![
  ("G", 0),
  ("0", 0),
  ("X", 0),
  ...
];

let actual: Vec<_> = Lexer::new("...")
  .map(|tok| (tok.value, tok.span.line))
  .collect();

assert_eq!(actual, expected);

[jmbeck15]

For long g-code example tests, that's probably the way to go. Maybe we could create some helper methods that make testing easier. We're really missing these long-form g-code verification tests, so I took this one from the pull request @dr0ps made.

How about we keep this test as is for now, and rework these kinds of tests in the future?

[jmbeck15]

@Michael-F-Bryan I pushed two more commits, one of which fixes #55. I was going to wait until after this pull request was merged to fix #55, but it turns out very much related to the newline handling.

If you prefer these fixes in separate pull requests, just let me know.

[jpursell]

Howdy! In case anyone is still out there, I tried using this library and ran into a bug. The library panicked on the gcode generated by DrawingBotV3-Free. Anyways, I am interested to know if this change would fix it. I'm hoping this message will motivate someone to resolve this merge request.

[jmbeck15]

@jpursell if you send me some minimal example code that panics the library, I'll check it out. I'm curious.