search

SymfonyCasts / sass-bundle

Delightful Sass Support for Symfony + AssetMapper
https://symfony.com/bundles/SassBundle/current/index.html
MIT License
37 stars 18 forks source link

Resolve relative sass_root paths earlier #65

Open wryk opened 8 months ago

wryk commented 8 months ago

Revert SassCssCompiler before #64. Supports relative paths by resolving/prefixing them at extension loading. I don't know if it should be implemented this way (it feels hacky?) but I don't see how it could be better.

What do you think of this change @smnandre ?

Tests are missing because I don't know enough how asset-mapper works to design an integration test for an AssetCompiler. Testing the builder instead would be even more work for me because my dev env doesn't supports the default sass binary from the bundle.

I'll not have more time in the next weeks to work on it, feel free to modify this PR as much as you want.

smnandre commented 8 months ago

I'll look later this week, no immediate feedback except: i'd still start with a ".sass" check :)

wryk commented 8 months ago

Yeah sorry, I ignored this part on purpose and I took the conservative choice to only rollback SassCssCompiler. I don't understand the reasoning of the current supports implementation (comparison of two realpaths) but someone might wrote it for a good reason. Because of the missing or broken tests (in my env) I'd rather not prematurely update code not required for relative path support.

smnandre commented 8 months ago

I mande a PR on your PR with small change suggestions :)

https://github.com/wryk/sass-bundle/pull/1