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

Implement property specific validation messages #26

Open
ben-eb opened this issue Aug 31, 2016 · 3 comments
Open

Implement property specific validation messages #26

ben-eb opened this issue Aug 31, 2016 · 3 comments

Comments

@ben-eb
Copy link
Owner

ben-eb commented Aug 31, 2016

Now that 0.1.0 is out, I'd like to implement more specific validation messages. These should offer more insight into why the validator might yield a warning; here are some examples:

  • isColor: rgb(255, 255, 255, 1) - too many arguments, expected 3.
  • isColor: rgb(255, 255) - too few arguments, expected 3.
  • isColor: rgb(255, 255, 255,) - unexpected trailing comma.
  • isColor: rgb(255 255 255) - expected arguments to be comma-separated.

Essentially what we have to do now is to break down the return conditions of the custom validators. Take isLength as an example. The return condition should be extracted into individual conditions that yield a message if the condition is failing. So, for example, the function checks that the value does not end with . - instead of a boolean test, we can return a message here instead:

if (!endsWith(int.number, '.')) {
  return {
    type: 'invalid',
    message: 'At least one number should follow the decimal point.',
  };
}

Suggestions for types of messages to return would be very welcome, as well as pull requests that implement these new messages. 😄

@jeddy3
Copy link

jeddy3 commented Sep 9, 2016

Suggestions for types of messages to return would be very welcome

The only convention we have in stylelint is that messages either start with "Expected" or "Unexpected". Your examples are currently very close. Would it be possible to adopt this convention? Saving us from having to adapt them at the stylelint end.

  • isColor: rgb(255, 255, 255, 1) - Expected 3 arguments, currently too many.
  • isColor: rgb(255, 255) - Expected 3 arguments, currently too few.
  • isColor: rgb(255, 255, 255,) - Unexpected trailing comma.
  • isColor: rgb(255 255 255) - Expected arguments to be comma-separated.

As an aside, it's very exciting to see css-values come together. This depth of linting is stunning.

@ben-eb
Copy link
Owner Author

ben-eb commented Sep 9, 2016

Would it be possible to adopt this convention?

Certainly! The idea is that no adaptation will be necessary. 😄

As an aside, it's very exciting to see css-values come together. This depth of linting is stunning.

Thanks, this means a lot. 😀

I noticed that currently the generator doesn't handle validation messages properly yet, so I've been working on this - ad5b7ca implements this checking for single values, just multiple values currently outstanding. Once multiple values are implemented I'll try and work on an example conversion and link it here. I'd like to tackle the rest of the more complex properties myself instead of writing validation messages, which I would like to delegate to anyone interested.

@ben-eb
Copy link
Owner Author

ben-eb commented Sep 12, 2016

So, I've extended support for validation messages in multiple values, and have done an example conversion which you can check out at 056a394.

Would be really great to have some help to convert the rest of the validators to return appropriate validation messages. The general format of this is that each validator might need to pass a value through (returning false) to another validator, or if it looks like a mistake then it should return a message for it.

For example, a validator that validated a hypothetical foo function would return false if the function's name was not foo (such that it could be passed to another validator), but would return a message instead if any of foo's arguments were incorrect. You can use the shouldReturnResult utility to check that either the validation failed and a message was returned, or that the value passed validation. If false, you are expected to pass the value on to the next validator, which most likely will be isVariable.

for(10px); // returns false, passes through to the next validator
foo(10px); // valid, returns true
foo(bar);  // invalid, returns a message "Expected foo to be called with a length value."

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

2 participants