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

Ampersand SCSS parse error #114

Closed
bgriffith opened this issue Oct 18, 2015 · 10 comments
Closed

Ampersand SCSS parse error #114

bgriffith opened this issue Oct 18, 2015 · 10 comments

Comments

@bgriffith
Copy link
Collaborator

In SCSS/Sass the & be used as a prefix to quickly form BEM style selectors.

Using the ampersand like this was ok in 3.0.0-31 but in 3.0.3 it's causing a parse issue.

.block {
  content: 'this is a block';

  &__element {
    content: 'foo';

    &--modifier {
      content: 'bar';
    }
  }
}

Results in 3 selectors; .block, .block__element and .block__element--modifier.

@jwilsson
Copy link

I'm seeing the same error for Less.

@tonyganch
Copy link
Owner

Again, consulting about tree structure.
Since the part that goes after & can be absolutely anything, I can't match it to any existing node type.
So we have a few options here.

(1) Name it all ident:

// String
&__elem

// Parse tree
 -> selector
 | -> parentSelector
 |    "&"
 | -> ident
 |    "__elem"
// String
&2-panda

// Parse tree
 -> selector
 | -> parentSelector
 |    "&"
 | -> ident
 |    "2-panda"

The problem of this solution is that 2-panda is not an ident per spec, because it starts with a number.

(2) Parse smaller parts:

// String
&__elem

// Parse tree
 -> selector
 | -> parentSelector
 |    "&"
 | -> ident
 |    "__elem"
// String
&2-panda

// Parse tree
 -> selector
 | -> parentSelector
 |    "&"
 | -> number
 |    "2"
 | -> ident
 |    "-panda"

The problem of this solution is that selector node becomes cluttered.

(3) Add a new node type:

// String
&__elem

// Parse tree
 -> selector
 | -> parentSelector
 |    "&"
 | -> parentSelectorExtension
 |    "__elem"
// String
&2-panda

// Parse tree
 -> selector
 | -> parentSelector
 |    "&"
 | -> parentSelectorExtension
 |    "2-panda"

I like this one the most. Its problem is, well, a good node type name.

I've searched through c++ sources of libsass but found nothing to help myself.

@bgriffith
Copy link
Collaborator Author

I'm actually leaning on option 2. As you may want to lint against having numbers in selectors or something similar.
I guess the other option is:

// String
&2-panda

// Parse tree
 -> selector
 | -> parentSelector
 |    "&"
 | -> parentSelectorExtension
 | | -> number
 | |   "2"
 | | -> ident
 | |   "-panda"

@DanPurdy
Copy link
Collaborator

I think option 2 is ok, but @bgriffith solution seems the most flexible for me, yes it's becoming cluttered but it offers the most flexibility. and leaves the assumptions down to whoever is using the parser.

Hope that helps.

Also @tonyganch I'll get some time this evening to go through those other issues hopefully in sass-lint and report back if they're ok now in gonzales-pe.

Thanks for the updates!

@Snugug
Copy link

Snugug commented Oct 19, 2015

I like Number 3 or the breakdown that @bgriffith provides. These are partial selectors, not full selectors, so it's a… partial ident? The full ident is the parent selector plus the partial ident presuming (in Sass, anyway) that there is not space between the parentSelector and the partial ident

tonyganch added a commit that referenced this issue Oct 19, 2015
Since the part that goes after `&` can be absolutely anything and we
can't match it to any existing node type, create a new node type.

See #114
tonyganch added a commit that referenced this issue Oct 19, 2015
Since the part that goes after `&` can be absolutely anything and we
can't match it to any existing node type, create a new node type.

See #114
@tonyganch
Copy link
Owner

Implemented @bgriffith's idea in @3.2.0.

// String
&2-panda

// Parse tree
 -> selector
 | -> parentSelector
 |    "&"
 | -> parentSelectorExtension
 | | -> number
 | |   "2"
 | | -> ident
 | |   "-panda"

@gitnik
Copy link

gitnik commented Nov 10, 2015

This still breaks for me? And I'm on 3.2.1

@gitnik
Copy link

gitnik commented Nov 10, 2015

.inputContainer {
    border: 1px solid #000;

    & input {
        border: 0;
        box-shadow: none;
    }

    @at-root #{&}__headerRow {
        width: 100%;
        height: 20px;
        padding: 3px 5px 0 12px;
        color: #b1b7ba;
        font-size: 12px;
    }
}

@tonyganch tonyganch reopened this Feb 7, 2016
@bgriffith
Copy link
Collaborator Author

So this is failing due to #150

@bgriffith
Copy link
Collaborator Author

We can close this since the outstanding issue was due to #150 which has been fixed in 3.3.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants