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

md fixes #30

Merged
merged 21 commits into from
Oct 5, 2023
Merged

md fixes #30

merged 21 commits into from
Oct 5, 2023

Conversation

chris-ha458
Copy link
Contributor

Strips should_strip_sig_or_bom
some blackmagic boolean logic 4899bf3

I doubled checked this with pen and paper, and tests all pass
But still it would be a good idea to get another set of eyes here.

if (!A AND B)
then
assign A = true
is equivalent to
A |= B

(A is only false when A and B are false in both cases)
tests pass as well
@chris-ha458
Copy link
Contributor Author

i kept the clippy error messages.
If it is true that atleast in rust that should_strip_sig_or_bom always returns true,
strip_sig_or_bom is always equal to bom_or_sig_available.
This means that bom_or_sig_available && !strip_sig_or_bom becomes
bom_or_sig_available && !bom_or_sig_available which is always false.
this occurs twice and clippy did not find them before but could find them with these commits.

src/lib.rs Outdated Show resolved Hide resolved
@nickspring
Copy link
Owner

Strips should_strip_sig_or_bom some blackmagic boolean logic 4899bf3

I doubled checked this with pen and paper, and tests all pass But still it would be a good idea to get another set of eyes here.

Don't see it in changes, only in commit. It looks correct.

A or false  -> A
if false { A } else {B}
-> B
we had an intermediate owner `decoded_payload_result` but it is unnecessary.
prioritized_encodings does not need to own a String
otherwise we unwrap the error message and return true since it is ascii..
@nickspring nickspring merged commit 684787e into nickspring:main Oct 5, 2023
3 checks passed
@chris-ha458 chris-ha458 deleted the lib branch October 5, 2023 23:48
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.

2 participants