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

Allow customizing file loading using the new Context trait #85

Merged
merged 15 commits into from
Jan 17, 2021

Conversation

alvra
Copy link
Contributor

@alvra alvra commented Jan 8, 2021

This change allows customizing how files are loaded.

Changes:

  • Add a new trait Context, implemented by FileContext.
  • Change the public signature of the method Format.write_root() (and related private ones) to accepting this trait instead of a FileContext directly.
  • Add a new function parse_scss_readable(), like do_parse_scss_file() but accepting a Read.
    I was considering renaming the original functions from X_file() to X_path() since they accepts paths instead of files. Then the new ones that accept Read can become X_file().
    This would be a breaking change, however.
  • Add a function parse_imported_scss_readable(), like parse_imported_scss_file()
    This function isn't used or public so it causes a warning, it can probably be removed.
  • Change a field of Error::Input, from a PathBuf to a String.
    This should be the only backwards incompatible change.

Example of how to use this

given:

use std::collections::HashMap;
use rsass::{Context, Error};

#[derive(Clone, Debug)]
struct StaticContext<'a> {
    files: HashMap<String, &'a[u8]>,
}

impl<'a> Context for StaticContext<'a> {
    type File = &'a [u8];

    fn find_file(
        &self, name: &str
    ) -> Result<Option<(Self, String, Self::File)>, Error> {
        if let Some(file) = self.files.get(name).map(|data| *data) {
            Ok(Some((self.clone(), name.to_string(), file)))
        } else {
            Ok(None)
        }
    }
}

then:

let format = ...
let entry = "import 'a'; import 'b';".as_bytes();
let file_a = "a { color: red }".as_bytes();
let file_b = "p { color: blue }".as_bytes();
let mut context = StaticContext { files: HashMap::new() };
context.files.insert("a".to_string(), file_a);
context.files.insert("b".to_string(), file_b);

output = format.write_root(
    parse_scss_data()?,
    &mut rsass::GlobalScope::new(self.format),
    context)?;
assert_eq!(
    output,
    format!("{}\n{}",
        std::str::from_utf8(file_a).unwrap(),
        std::str::from_utf8(file_b).unwrap()));

Copy link
Owner

@kaj kaj left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! This is something I have considered for some time but not yet implemented, so thanks for that.

src/output/style.rs Outdated Show resolved Hide resolved
src/file_context.rs Outdated Show resolved Hide resolved
src/parser/mod.rs Outdated Show resolved Hide resolved
Copy link
Owner

@kaj kaj left a comment

Choose a reason for hiding this comment

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

I think this is close to being merged now, just humor me with a few more nitpicks ...

src/file_context.rs Outdated Show resolved Hide resolved
src/parser/mod.rs Outdated Show resolved Hide resolved
src/output/style.rs Outdated Show resolved Hide resolved
src/output/style.rs Outdated Show resolved Hide resolved
src/output/style.rs Outdated Show resolved Hide resolved
src/output/style.rs Outdated Show resolved Hide resolved
src/parser/mod.rs Outdated Show resolved Hide resolved
@alvra
Copy link
Contributor Author

alvra commented Jan 17, 2021

I think I covered everything, to me it looks ready now.
Let me know if you need anything else :)

Copy link
Owner

@kaj kaj left a comment

Choose a reason for hiding this comment

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

Yes, I think this looks good now. Thank you for the contribution!

@kaj kaj merged commit 8eb5f8c into kaj:master Jan 17, 2021
kaj added a commit that referenced this pull request Jan 29, 2021
Progress: 2784 of 5936 tests passed in dart-sass compatiblilty mode.

## Breaking changes

* Update nom to 6.0 rasises the minimally supported compiler version
  to 1.44.0.  Also, the dependency is technically exposed.
* `sass::Item::Use` was modified by #84.
* `compile_scss_file` is renamed to `compile_scss_path`, and
  `FileContext` is now a trait, the default implementation is renamed
  to `FsFileContext` by #85.

## Improvements

* Allow customizing file loading by providing a custom impl of a
  `FileContext`, PR #85.
* Support the `@use name as *` syntax, PR #84.
* Make `Error::BadValue` a little closer to whats expected.
* Handle units in `@for` loops.
* Update `nom` to 6.0, PR #83.
* Update `rand` to 0.8, PR #86.
* Testing is now done with github actions rather than travis
  (Appveyor remains for window builds).
* sass-spec test suite updated to 2021-01-20.

Thanks to @paolobarbolini and @alvra for code contributions.

Tested with rustc 1.49.0 (e1884a8e3 2020-12-29),
1.44.1 (c7087fe00 2020-06-17), 1.46.0 (04488afe3 2020-08-24),
1.48.0 (7eac88abb 2020-11-16), 1.50.0-beta.8 (1cd030396 2021-01-20),
and 1.51.0-nightly (c0b64d97b 2021-01-28).
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