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

Rule idea: declaration-nested-properties #44

Closed
s10wen opened this issue Jun 6, 2016 · 11 comments
Closed

Rule idea: declaration-nested-properties #44

s10wen opened this issue Jun 6, 2016 · 11 comments
Assignees
Labels
New rule 👌 Ready to be created as a new rule
Milestone

Comments

@s10wen
Copy link

s10wen commented Jun 6, 2016

From stylelint/stylelint#1386 (comment) @jeddy3 suggested I post this here.

@kristerkari
Copy link
Collaborator

Good idea 👍

@kristerkari kristerkari added this to the 1.3.0 milestone Jun 24, 2016
@dryoma
Copy link
Collaborator

dryoma commented Jul 26, 2016

Getting down to this at last.

And a question. Do we really want the "always" option here? That would mean this rule will have to enforce nesting properties that... which ones exactly?

  1. Definitely not those with a - in a name, 'cause text-align and stuff. Or no?
text: {
  align: center;
  decoration: underline;
  indent: 2em;
}
  1. Or should we probably create a list of rules that are not allowed in non-nested form? E.g. [ /^margin-/, /^flex-/, /^background-/, etc.], and if the parser meets more than one of them it yields something like Expected "margin-left" to be in a form of a nested rule.

UPD: if the parser meets more than one of them...

  1. Or just declaration-no-nested-properties?

@s10wen
Copy link
Author

s10wen commented Jul 26, 2016

@dryoma interesting, regarding 2) are you thinking of setting defaults, or have the user set them? Also wondering what would be the best approach, blacklist / whitelist, or give the user the option to set them, then in the docs show a suggestion.

@kristerkari
Copy link
Collaborator

  1. would be the easiest to implement, but yeah 2) could also work as set of whitelisted properties

@dryoma
Copy link
Collaborator

dryoma commented Jul 26, 2016

If we go with whitelist/blacklist, than no defaults are possible by the nature of stylelint. And whenn suggesting the 2nd option I was hoping that someone could point at the list of properties that are usually/preferably use nesting in Sass, as Sass itself (according to Sassmeister) does'n seem to give two moos about what is nested:

a {
  may: {
    the: {
      force: be with you;
    }
  }
}
a {
  may-the-force: be with you;
}

@kristerkari
Copy link
Collaborator

I guess if Sass does not care about what you nest, maybe we should not either -- and just skip option 2.

So we could do 3), or 1) that would have "always" option to enforce nesting for any property that has - in the name.

@dryoma
Copy link
Collaborator

dryoma commented Jul 26, 2016

If we go with 1), then is this:

a {
  display: block;
  margin: {
    left: 10px;
  }
}

acceptable? I.e. nesting just one prop. Maybe this rule must be triggered only if two or more props with the same prefix (margin-) are met?

Or maybe create an option that allows/prohibits that (enforcin nesting of just one rule)?

@kristerkari
Copy link
Collaborator

or maybe require everything to be nested by default and add an option except: first?

@dryoma dryoma modified the milestones: 1.3.0, 1.4.0 Jul 26, 2016
@dryoma
Copy link
Collaborator

dryoma commented Jul 26, 2016

So, with "always":

a {
  margin-left: 10px; // a warning
  animation-name: weeee; // a warning
  animation-delay: 0.1s; // a warning
  text: {
    align: justify; // ok
  }
} 

with [ "always", {except: "only-of-namespace"} ]

a {
  margin-left: 10px; // get off, I'm the only child!
  animation-name: weeee; // a warning
  animation-delay: 0.1s; // a warning
  text: {
    align: justify; // warning, as "except" options reverse corresponding main options
  }
} 

Like so?

Also, how should we name that except option? It is "the only property with certain prefix/of certain namespace". "only-of-namespace"?

p.s.

.funky {
  font: 20px/24px fantasy {
    weight: bold;
  }
}

makes me wanna go nuts rignt now :)

@dryoma dryoma self-assigned this Jul 26, 2016
@dryoma dryoma added the New rule 👌 Ready to be created as a new rule label Jul 26, 2016
dryoma added a commit that referenced this issue Jul 27, 2016
dryoma added a commit that referenced this issue Aug 31, 2016
@dryoma dryoma closed this as completed Nov 24, 2016
@dryoma
Copy link
Collaborator

dryoma commented Dec 12, 2016

@s10wen the rules are available in 1.4.1 (at last)

@s10wen
Copy link
Author

s10wen commented Dec 13, 2016

@dryoma niiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiiice and <3 the example at #44 (comment) the force must be strong with you 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New rule 👌 Ready to be created as a new rule
Development

No branches or pull requests

3 participants