Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support bytes creation from hex and ascii #843

Merged
merged 2 commits into from Apr 18, 2019

Conversation

jgirardet
Copy link
Contributor

this allows b"blabla\x0f\x1dblabla"

parser/src/lexer.rs Outdated Show resolved Hide resolved
escape = false;
}
x => {
if hex_on {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this if statement must be around the match like this:

if hex_on {
  process digit
} else {
   match c {
     // the rest
   }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, with a bigger match, i think, it'll be clearer

}
} else {
if escape {
res.push(92);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always push 92 here? Not x?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes backslash (92) was seen at previous loop but not added since you need the following char to know if its a "pure" backslash or a backslash of a \xXX. So I add the "pure" backslash at next loop before to add c if we are not hex_on

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you could do '\\' as u8 to make it more clear what character you're adding.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought everyone knows ascii table by heart 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codecov-io
Copy link

Codecov Report

Merging #843 into master will increase coverage by 0.02%.
The diff coverage is 79.24%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #843      +/-   ##
==========================================
+ Coverage   63.16%   63.19%   +0.02%     
==========================================
  Files          87       87              
  Lines       14382    14467      +85     
  Branches     3257     3272      +15     
==========================================
+ Hits         9085     9143      +58     
- Misses       3160     3178      +18     
- Partials     2137     2146       +9
Impacted Files Coverage Δ
parser/src/lexer.rs 82.9% <79.24%> (-0.49%) ⬇️
vm/src/compile.rs 77.15% <0%> (-0.8%) ⬇️
vm/src/import.rs 65.95% <0%> (-0.71%) ⬇️
src/main.rs 20% <0%> (-0.54%) ⬇️
vm/src/exceptions.rs 47.54% <0%> (ø) ⬆️
wasm/lib/src/vm_class.rs 0% <0%> (ø) ⬆️
vm/src/builtins.rs 66.47% <0%> (+0.06%) ⬆️
vm/src/vm.rs 72.22% <0%> (+1.15%) ⬆️
vm/src/error.rs 13.79% <0%> (+13.79%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 240c1e4...eb2d0b0. Read the comment docs.

@windelbouwman windelbouwman merged commit 319bb80 into RustPython:master Apr 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants