Fix multiple open issues #927

mgreter · 2015-03-08T04:44:05Z

Addresses/Fixes #317 and ~~#871~~

Added a lot of other small fixes ("low hanging fruits")!

Adds is_optional flag for selectors
Add media_block flag for selectors
Prints error message on invalid extends
Changes format how errors are reported to match ruby sass
Fix illegal extending accross media blocks (fixes LibSass supports cross-media @extend #712)
Improved some OutputBuffer behavior (preparation for @charset insertion produces invalid source maps #879)
Parse line comments in blocks (fixes Comments before pseudo selector interpolations ::#{$variable} cause shifts in error line number reporting #533)
Fix interpolation in media queries (fixes Interpolation of media features errors #346)
Add error for parent selectors in root blocks (fixes Should throw error when root selector has "&" #325)
Implement number prefix parsing (/[+-]+/) (fixes Error parsing properties starting with operator (-/+) #535)

Prints the exact error message as ruby sass.

Prints the exact error message as ruby sass. Implements optional flag to suppress message.

Fixes sass#533

Evaluate the actual queries against each other. We probably need to implement a better test here!

Fixes sass#346

Adds block stack for parser to do this check! Fixes sass#325

Fixes sass#535

Fixes sass#879 Should work for all prepended texts (like top-comments)

xzyfer · 2015-03-09T04:39:25Z

It's worth breaking this up into multiple PRs. Having PRs that fix multiple issues is messy IMHO and it means all these fixes can't be merged because of a single failing commit. This is also makes it difficult for me to figure out what I should be working without accidentally re-doing work you've done.

Fixes sass#923

mgreter · 2015-03-09T22:39:23Z

@xzyfer I know it's a bit messy, but it's easier this way to have a linear commit history. I can create single PRs for each issue. But it adds quite a lot of additional work, that is basically not giving any value to the final code base! So I hope we can merge this "en gros". I also assigned myself to all issues that are covered by this PR, so you should be able to get a list of issues I'm working on/have fixed!

mgreter · 2015-03-09T22:50:37Z

My spec-runner tells me that the following test will now pass:

todo-issues/issue_346
todo-issues/issue_535
todo-tests/scss-tests/126_test_media_interpolation_with_reparse

Most other fixes address issues that are either missing spec tests, source map related or about error reporting, which is not yet tested by the sassc spec runner (I have some tests in perl-libsass)!

xzyfer · 2015-03-09T23:06:41Z

Yeah sure, it's just worth keeping in mind in the future. Since we're in different timezones we're likely to step on each others toes. It's worth just merge PR with small discrete chucks rather than large multi-purpose PRs IMHO.

Nice work on this!

xzyfer · 2015-03-09T23:07:09Z

After this is merged I'll update sass/sass-spec#271 with those specs.

mgreter · 2015-03-09T23:14:21Z

OK, I'm merging this then! The code in this PR is probably not always the cleanest, but we need to get things going in a direction, so better than failing spec tests IMO. And there is a lot of existing code that could need improvement, and I basically just trust the spect tests and CI here 🚀

Fix multiple open small issues

xzyfer · 2015-03-09T23:24:01Z

Agreed.

drewwells · 2015-03-10T00:19:40Z

FYI: I'm getting runtime exceptions with these changes

mgreter · 2015-03-10T00:22:18Z

@drewwells can you tell which commit introduces your problems?
There was a breaking change in #925 for implenters!

drewwells · 2015-03-10T00:27:08Z

5757248 works fine, but this merge broke
it. I can try bisecting the merge for which commit introduced it.

On Mon, Mar 9, 2015 at 7:22 PM Marcel Greter notifications@github.com
wrote:

@drewwells https://github.com/drewwells can you tell which commit
introduces your problems?

—
Reply to this email directly or view it on GitHub
#927 (comment).

mgreter · 2015-03-10T00:30:10Z

Would be helpfull to have a specific commit to pin your issue!

drewwells · 2015-03-10T00:40:43Z

I wasn't sure how to bisect a merge, but appears you can!
b5235ec
introduced the exception.

On Mon, Mar 9, 2015 at 7:30 PM Marcel Greter notifications@github.com
wrote:

Would be helpfull to have a specific commit to pin your issue!

—
Reply to this email directly or view it on GitHub
#927 (comment).

drewwells · 2015-03-10T00:52:18Z

Sorry, I'm not easily reproducing the problem. I'll keep digging and file an issue if I can find the problem. After cleaning and rebuilding several times on latest master, I'm not reproducing this. Maybe it was just a build problem.

mgreter · 2015-03-10T07:38:37Z

Yep, you need to make clean if some header structures have changed, which was definitely the case!

drewwells · 2015-03-10T14:33:49Z

That's probably it, thanks for the info.

On Tue, Mar 10, 2015 at 2:38 AM Marcel Greter notifications@github.com
wrote:

Yep, you need to make clean if some header structures have changed, which
was definitely the case!

—
Reply to this email directly or view it on GitHub
#927 (comment).

mgreter force-pushed the bugfix/issue_317 branch 2 times, most recently from e3c15dc to f624df0 Compare March 8, 2015 05:31

mgreter added 4 commits March 8, 2015 07:05

More improvements

f0b5f22

Fix illegal extending accross media blocks

d68cc86

Prints the exact error message as ruby sass.

Fix illegal extending when no blocks are found

a61ec99

Prints the exact error message as ruby sass. Implements optional flag to suppress message.

Parse line comments in blocks

49862a6

Fixes sass#533

mgreter force-pushed the bugfix/issue_317 branch from bb78292 to 6294ec3 Compare March 8, 2015 06:06

Add another fix for media query boundaries

f65aa5f

Evaluate the actual queries against each other. We probably need to implement a better test here!

mgreter force-pushed the bugfix/issue_317 branch from 6294ec3 to f65aa5f Compare March 8, 2015 06:47

mgreter added 3 commits March 8, 2015 08:19

Improve comment parsing

b5235ec

Fix interpolation in media queries

0e52c1c

Fixes sass#346

Add error for parent selectors in root blocks

c64f228

Adds block stack for parser to do this check! Fixes sass#325

mgreter self-assigned this Mar 9, 2015

mgreter added this to the 3.2 milestone Mar 9, 2015

mgreter force-pushed the bugfix/issue_317 branch from 6f360d2 to ba1750e Compare March 9, 2015 02:13

Implement number prefix parsing (/[\+\-]+/)

5606d26

Fixes sass#535

mgreter force-pushed the bugfix/issue_317 branch from ba1750e to 5606d26 Compare March 9, 2015 02:15

mgreter changed the title ~~[WIP] Fix behavior with invalid extends~~ Fix behavior with invalid extends Mar 9, 2015

mgreter changed the title ~~Fix behavior with invalid extends~~ Fix multiple open issues Mar 9, 2015

mgreter force-pushed the bugfix/issue_317 branch from 22827bb to ff1b08d Compare March 9, 2015 03:38

Improve sourcemap output for prepended texts

21ce837

Fixes sass#879 Should work for all prepended texts (like top-comments)

mgreter force-pushed the bugfix/issue_317 branch from ff1b08d to 21ce837 Compare March 9, 2015 04:18

xzyfer mentioned this pull request Mar 9, 2015

Enable newly passing specs sass/sass-spec#271

Merged

Add local unaware atof function

14bfd99

Fixes sass#923

mgreter mentioned this pull request Mar 9, 2015

3.2 release #812

Closed

mgreter added a commit that referenced this pull request Mar 9, 2015

Merge pull request #927 from mgreter/bugfix/issue_317

51ffdcd

Fix multiple open small issues

mgreter merged commit 51ffdcd into sass:master Mar 9, 2015

mgreter deleted the bugfix/issue_317 branch March 10, 2015 00:05

mgreter restored the bugfix/issue_317 branch March 10, 2015 00:05

am11 mentioned this pull request Mar 30, 2015

Version 3.0.0-pre is no longer available in npm sass/node-sass#820

Closed

mgreter deleted the bugfix/issue_317 branch April 6, 2015 17:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix multiple open issues #927

Fix multiple open issues #927

mgreter commented Mar 8, 2015

xzyfer commented Mar 9, 2015

mgreter commented Mar 9, 2015

mgreter commented Mar 9, 2015

xzyfer commented Mar 9, 2015

xzyfer commented Mar 9, 2015

mgreter commented Mar 9, 2015

xzyfer commented Mar 9, 2015

drewwells commented Mar 10, 2015

mgreter commented Mar 10, 2015

drewwells commented Mar 10, 2015

mgreter commented Mar 10, 2015

drewwells commented Mar 10, 2015

drewwells commented Mar 10, 2015

mgreter commented Mar 10, 2015

drewwells commented Mar 10, 2015

Fix multiple open issues #927

Fix multiple open issues #927

Conversation

mgreter commented Mar 8, 2015

xzyfer commented Mar 9, 2015

mgreter commented Mar 9, 2015

mgreter commented Mar 9, 2015

xzyfer commented Mar 9, 2015

xzyfer commented Mar 9, 2015

mgreter commented Mar 9, 2015

xzyfer commented Mar 9, 2015

drewwells commented Mar 10, 2015

mgreter commented Mar 10, 2015

drewwells commented Mar 10, 2015

mgreter commented Mar 10, 2015

drewwells commented Mar 10, 2015

drewwells commented Mar 10, 2015

mgreter commented Mar 10, 2015

drewwells commented Mar 10, 2015