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

remove dead code & replace remaining parse_ident #180

Merged
merged 1 commit into from
Nov 4, 2019

Conversation

burrbull
Copy link
Collaborator

last commit from #177

@crlf0710
Copy link
Owner

Seems good to me, but since there's quite a few pointer operation stuff, it'll be best to have more eyes on this.

Copy link
Collaborator

@cormacrelf cormacrelf left a comment

Choose a reason for hiding this comment

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

Pointer stuff looks fine

k += 1
let q = p.parse_c_ident();
if q.is_none() {
return Err(());
Copy link
Collaborator

Choose a reason for hiding this comment

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

? operator works too

@@ -737,76 +720,60 @@ unsafe fn spc_handler_html_default(mut spe: *mut spc_env, mut ap: *mut spc_arg)
error
}
/* translate wsp* '(' wsp* number (comma-wsp number)? wsp* ')' */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another piece of code where nom might make things a lot cleaner. The parse traits would be usable, with some small wrappers that clone the slice pointer, mutate it, subtract the length from the input and return the remainder and output as a nom::IResult by converting to a basic nom error. Then functions like these would be a few lines long, using nom combinators (except the end bit that mutates, just return the data and use it to mutate elsewhere).

@crlf0710
Copy link
Owner

crlf0710 commented Nov 3, 2019

Need a rebase, i think...

@burrbull
Copy link
Collaborator Author

burrbull commented Nov 3, 2019

Done.

@crlf0710
Copy link
Owner

crlf0710 commented Nov 4, 2019

Merging~ Thanks~

@crlf0710 crlf0710 merged commit 6edc4a6 into crlf0710:oxidize Nov 4, 2019
@burrbull burrbull deleted the parse_ident branch November 18, 2019 06:17
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.

3 participants