Skip to content

Conversation

@mzabriskie
Copy link
Contributor

Closes #2862

@zpao
Copy link
Member

zpao commented Jan 27, 2015

Awesome! A few comments incoming, but overall it looks good.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can leave out the parens since we're not using that matching group anywhere. But good call on making the regex a bit more robust with the space checks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you're just copying the pattern from above, but let's make use of %s instead of doing the concats (we're going to fix the others up in #2941)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should this be handled if the message itself is > 80 max-len? Ok to concatenate actual message to avoid lint failures?

warning(
  false,
  'Style property values shouldn\'t contain a semicolon. ' +
  'Try "%s: %s" instead.',
  name,
  value.replace(badStyleValueWithSemicolonPattern, '')
);

@zpao
Copy link
Member

zpao commented Jan 27, 2015

Awesome, thanks! I think this will be helpful.

zpao added a commit that referenced this pull request Jan 27, 2015
Provide warning when using styles containing a semicolon
@zpao zpao merged commit c6d1904 into facebook:master Jan 27, 2015
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

Successfully merging this pull request may close these issues.

Style updates silently fail with trailing semicolon in style

2 participants