Use &str instead of &[u8] in reader.rs, and other concerned areas #24
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
added 5 commits April 24, 2020 18:08
This removes a lot of ambiguity and allows us to ensure that `ClojureRS` will still work with non-ascii characters. Additionaly, a bug lurking in `consume_clojure_whitespaces` was fixed. This bug triggered an error when the function had no whitespaces to consume, while it should not.
A bug was introduced with last commit. User input was not wiped between two call to `read_line` in `main`. As such, everything the user entered was re-evaluated on each return keypress. This commit clears the content of user input between two `read_line` calls. `identifier_parser` was also refactored. This allows us to skip one `format!` call and many clones, making the function execution faster. Its content was also split, resulting in the creation of `first_char`, `cons_str`, `is_identifier_char` and `is_non_numeric_identifier_char`. Next commits will focus on refactoring further the content of `reader.rs`.
Documentation in `reader.rs` said that functions parse `&[u8]` into other types. However, commit `f04d995` modified code so that it parses `&str`. This commit fixes the corresponding documentation.
These three functions had the same similar pattern, which can be simplified by using an `if-let` expression instead of a match as well as the `?` operator, which simplifies error handling.
scrabsha changed the title scrabsha changed the title 
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
As seen in the PR title,
src/reader.rshas been modified so that parsing functions accept&strinstead of&[u8], which will allow us to handle non-ascii characters as well. Code that called thetry_readfunction was modified so that is uses&stras well.Additionally,
src/reader.rswas refactored and some functions have been refactored in order to make them more idiomatic.This PR should fix #5. Some edits have to be done in order to merge the PR.