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

Source structure to hold source text, source type, and file path #118

Open
overlookmotel opened this issue Oct 3, 2024 · 1 comment
Open

Comments

@overlookmotel
Copy link

@Boshen said in oxc-project/oxc#6248 (comment):

Source with path, text and related friends.

Let's do that. I propose that we could also combine it with #94.

Preferred API

This would be the API we recommend to users:

let source = Source::from_file(path);

This would:

  • Initialize a Source with the provided path, and SourceType as deduced from the path's file extension.
  • Read the file from file system (using fast SIMD UTF-8 validation).

A one-stop shop!

Full API

We can also provide other ways to create a Source for advanced use cases:

struct Source<'a> {
  source_text: &'a str,
  source_type: SourceType,
  path: Option<Path>, // or `PathBuf`?
}

// Methods to create a `Source`
impl<'a> Source<'a> {
  // Load source text into arena
  fn from_file(path: Path, allocator: &'a Allocator) -> Self;
  fn from_reader<R: std::io::Read>(reader: R, size: Option<usize>, allocator: &'a Allocator) -> Self;
  // From string already in arena
  fn from_arena_string(source_text: oxc_allocator::String<'a>) -> Self;
  // From borrowed types
  fn from_str<'s: 'a>(source_text: &'s str, allocator: &'a Allocator) -> Self;
  fn from_bytes<'s: 'a>(source_text: &'s [u8], allocator: &'a Allocator) -> Self;
}

// Methods to override source type and path
impl<'a> Source<'a> {
  fn with_type(&mut self, source_type: SourceType);
  fn with_path(&mut self, path: Path);
}

from_file and from_reader take an &Allocator. It's ideal if they load the source text into the arena, for the reasons discussed in #94 (re-use allocations, warm cache).

My suggestion is that from_str and from_bytes also take an &Allocator. They don't need to right now, but if we decide in future that we always need source text to be in the arena, then we could make that change without breaking changes to the public API.

@Boshen
Copy link
Member

Boshen commented Oct 3, 2024

We are never going to achieve v1 aren't we?

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

No branches or pull requests

2 participants