-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Properties merging should work also inside directives #2035 #2121
Properties merging should work also inside directives #2035 #2121
Conversation
Conflicts: README.md
Fixes issue less#2035 - property merge inside @font-face. The _mergeRules function is now called also for directives with rules. It used to be called only for rulesets. I had to turn off jasmine tests for merge.less, because it was replacing all urls by their assumed full paths. For example, the url(something.eot) was changed into url(http://localhost:8081/test/less/something.eot). The result did not matched with expected css and failed. Note: I'm not sure why values order in source map changed. It does not seem to be caused by my change, it was failing before I made them.
…eird message and I want to see what it will do this time. Previous travis error: >> PhantomJS has crashed. Please read the crash reporting guide at >> https://github.com/ariya/phantomjs/wiki/Crash-Reporting and file a bug >> report at https://github.com/ariya/phantomjs/issues/new with the crash >> dump file attached: /tmp/7685d81d-94f5-1db7-12c9006e-72f80442.dmp 0 [ >> 'PhantomJS has crashed. Please read the crash reporting guide at >> https://github.com/ariya/phantomjs/wiki/Crash-Reporting and file a bug >> report at https://github.com/ariya/phantomjs/issues/new with the crash >> dump file attached: /tmp/7685d81d-94f5-1db7-12c9006e-72f80442.dmp' ]
I would put the merge test into urls which uses special options so the paths don't get altered (from memory)
That is strange, it doesn't ring a bell - I would expect master to fail if that was the case - I can't explain it! |
@lukeapage @seven-phases-max explained the source map problem in another pull request. I will move the test into urls.less and re-enable merge.less. However, urls.less are disabled in jasmine tests (see |
…-2035 Conflicts: test/sourcemaps/basic.json
merge.less in jasmine tests.
I moved tests into urls.less (the one in root) and re-enabled jasmine for merge.less. |
Properties merging should work also inside directives #2035
fantastic, good work |
Fixes issue #2035 - property merge inside
@font-face
. The _mergeRules function is now called also for directives with rules. It used to be called only for rulesets.I had to turn off jasmine tests for merge.less, because it was replacing all urls by their assumed full paths. For example, the url(something.eot) was changed into url(http://localhost:8081/test/less/something.eot). The result did not matched with expected css and failed.
Note: I'm not sure why values order in source map changed. It does not seem to be caused by my change, it was failing before I made them.