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

plugins option like in postcss-log-warnings to filter warnings by plugins #10

Open
gazay opened this issue May 11, 2015 · 3 comments
Open

Comments

@gazay
Copy link
Member

gazay commented May 11, 2015

Append warnings only from selected plugins if option provided

@MoOx
Copy link
Contributor

MoOx commented May 11, 2015

Might be interesting.

@ai
Copy link
Member

ai commented May 11, 2015

+1

lydell added a commit to lydell/postcss-log-warnings that referenced this issue May 29, 2015
Inspired by postcss-messages. Then why not use that plugin? Well, look at the
diff of index.js:

    -    console.log(warningLog);
    +    if (!('console' in options && !options.console)) {
    +      console.log(warningLog);
    +    }
    +
    +    if (options.browser) {
    +      result.root.append(formatBrowser(warningLog));
    +    }

The only difference is how to display the warnings! There's no need to duplicate
the rest of features between the two plugins.

This fixes postcss/postcss-browser-reporter#8, postcss/postcss-browser-reporter#9 and
postcss/postcss-browser-reporter#10.

As opposed to postcss-messages, this implementation:

- Inserts the messages at `head::before` instead of `html::before`. While
  `html::before` is unlikely to conflict with other styles, `head::before` is
  even less so! In fact, it should be _extremely_ unlikely to conflict.

- Does not allow to change the selector. Because of the above point there's no
  need to.

- Does not use `::after` if `::before` was already used in the CSS. Again,
  because of the first point there is no need to.

- Does not allow to change the styling. There's no way to change the styling
  when logging to the console either. Also, @ai said:

  > BTW, options is a lack in UX. You should better think more about default
  > options and styles, rather than add options for every case.

  postcss/postcss-browser-reporter#3 (comment)

- Uses simpler styling. KISS. It is still similar to postcss-messages.
lydell added a commit to lydell/postcss-log-warnings that referenced this issue May 30, 2015
Inspired by postcss-messages. Then why not use that plugin? Well, look at the
diff of index.js:

    -    console.log(warningLog);
    +    if (!('console' in options && !options.console)) {
    +      console.log(warningLog);
    +    }
    +
    +    if (options.browser) {
    +      result.root.append(formatBrowser(warningLog));
    +    }

The only difference is how to display the warnings! There's no need to duplicate
the rest of features between the two plugins.

This fixes postcss/postcss-browser-reporter#8, postcss/postcss-browser-reporter#9 and
postcss/postcss-browser-reporter#10.

As opposed to postcss-messages, this implementation:

- Inserts the messages at `head::before` instead of `html::before`. While
  `html::before` is unlikely to conflict with other styles, `head::before` is
  even less so! In fact, it should be _extremely_ unlikely to conflict.

- Does not allow to change the selector. Because of the above point there's no
  need to.

- Does not use `::after` if `::before` was already used in the CSS. Again,
  because of the first point there is no need to.

- Does not allow to change the styling. There's no way to change the styling
  when logging to the console either. Also, @ai said:

  > BTW, options is a lack in UX. You should better think more about default
  > options and styles, rather than add options for every case.

  postcss/postcss-browser-reporter#3 (comment)

- Uses simpler styling. KISS. It is still similar to postcss-messages.

- Properly escapes the CSS content string.
@pixeldrew
Copy link

+1

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

No branches or pull requests

4 participants