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

cleanup: improve wat detection #650

Merged
merged 3 commits into from
Jan 5, 2024
Merged

cleanup: improve wat detection #650

merged 3 commits into from
Jan 5, 2024

Conversation

zshipko
Copy link
Contributor

@zshipko zshipko commented Jan 5, 2024

  • Update to ensure is_wat is only true when the input is a valid string
  • Update to allow leading whitespace in a wat file

@zshipko zshipko merged commit cd4fc39 into main Jan 5, 2024
5 checks passed
@zshipko zshipko deleted the improve-wat-detection branch January 5, 2024 22:00
let s = std::str::from_utf8(&data);
let is_wat = s.is_ok_and(|data| {
let data = data.trim_start();
data.starts_with("(module") || data.starts_with(";;")
Copy link
Contributor

Choose a reason for hiding this comment

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

(Ooh, this could also start with (;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah thanks! i will open a new PR and link here. would (; be the start of a comment inside the opening paren?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a block comment (there's an example over here!)

Copy link
Contributor Author

@zshipko zshipko Jan 5, 2024

Choose a reason for hiding this comment

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

also, would a paren followed by whitespace be valid? something like this:

(
module

edit: looks like it is and that '(;' is a block comment (which i didn't even know existed!), sorry for the noise

zshipko added a commit that referenced this pull request Jan 5, 2024
Merged the other one too quickly, this PR is updated with some
additional feedback from @chrisdickinson:

              (Ooh, this could also start with `(;`)
#650 (comment)

It's also updated to accommodate for modules that start with an open
paren followed by some whitespace. I doubt this covers all of the
permitted syntax but it should be good enough.
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

2 participants