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

add the package name custom validate to take effect #276

Merged
merged 2 commits into from Dec 30, 2016
Merged

add the package name custom validate to take effect #276

merged 2 commits into from Dec 30, 2016

Conversation

likun7981
Copy link

I see the #73 ,The add the validation for package name , it is really nice , but it will be overwritten the user custom validate function , so , I fix it.

@kazupon
Copy link
Member

kazupon commented Dec 26, 2016

Thank you for your PR!
However, I could not understand overwritten the user custom validate function.
Can you tell us about the usecase please?

@likun7981
Copy link
Author

in the meta.js , if user define a validate option in the name prompt , the validate option will be overitten , it will not work。
if I config a meta.js the prompts like this:

prompts: {
    name: {
      type: 'string',
      required: true,
      label: 'Project name',
      validate: function (input) {
        return input === 'custom' ? 'can not input `custom`' : true
      }
    }
 }

#73 , Add a package name validate, like this:

function setValidateName (opts) {
  opts.prompts.name.validate = function (name) {
    var its = validateName(name)
    if (!its.validForNewPackages) {
      var errors = (its.errors || []).concat(its.warnings || [])
      return 'Sorry, ' + errors.join(' and ') + '.'
    }
    return true
  }
}

so, the validate function I defined in meta.js will be overwritten by it

@kazupon
Copy link
Member

kazupon commented Dec 26, 2016

Thanks!
However, I think this essential issue is overwritten the user.
we should be fixed, or warned it.
for example, in options function, check the whether exisit name, if exist it, ignore or warn.

@likun7981
Copy link
Author

Sometimes , we need a unified format for name ,example vue-router, vue-resource and so on. I think you should keep it

@likun7981
Copy link
Author

Maybe you can give a configuration item, it's value is a String or RegExp.

@kazupon
Copy link
Member

kazupon commented Dec 26, 2016

we need a unified format for name ,example vue-router, vue-resource and so on.

Indeed,I could now see your use-cases.
In that case, Looks good to me.

@kazupon kazupon merged commit 0d3f96d into vuejs:master Dec 30, 2016
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.

None yet

2 participants