Skip to content
This repository has been archived by the owner on Nov 9, 2019. It is now read-only.

Scanner #3

Open
wants to merge 19 commits into
base: latest
Choose a base branch
from
Open

Scanner #3

wants to merge 19 commits into from

Conversation

MilanLoveless
Copy link

@MilanLoveless MilanLoveless commented Jul 24, 2019

Here is our first pass at the require/import scanner for this issue: https://discourse.entropic.dev/t/looking-for-help-rust-based-js-import-scanner/218

TODO:

  • rename functions so they actually make sense. Currently we split off new functions every time we went another level deeper in match statements.
  • document all the things!
  • more tests!
  • figure out whether to expose ressa::Error as part of our Result<> return type or if we should remap it to our own internal error type.

@static-raccoon
Copy link

This pull request has been mentioned on Entropic. There might be relevant details there:

https://discourse.entropic.dev/t/looking-for-help-rust-based-js-import-scanner/218/47

Copy link
Author

@MilanLoveless MilanLoveless left a comment

Choose a reason for hiding this comment

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

it turns out, I don't know how to use github's review feature 😅

use ressa::Parser;
use std::vec::Vec;

pub fn parse_js<'a>(js: &'a str) -> Result<Vec<&'a str>, ressa::Error> {
Copy link
Author

Choose a reason for hiding this comment

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

Should we return ressa::Error here or make our own error type/enum?

@zkat
Copy link
Contributor

zkat commented Nov 9, 2019

Hey @MilanLoveless! I know things have been quiet for a while, but it looks like we're ramping up again. As part of that, I'm organizing our repo stuff a bit, and one of those changes is I'm going to merge the dstopic repo into the ds repo so we only need to clone one monorepo for both "sides" of ds. Do you mind recreating this PR against that repo? I've already move the code over there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants