Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

"before" is ignored if description is present, and required flag + default value is uneffective (need to refactor regexp) #62

Closed
lamuertepeluda opened this Issue Dec 6, 2012 · 6 comments

Comments

Projects
None yet
3 participants

Hi guys prompt is a great module, but I've found a couple of minor issues:

var schema={
  properties:{
   filename: {
      before: function(value) { return 'v -> ' + value.red; },
//       description:'Enter output file name'.green,
      pattern: /\b|(^[a-zA-Z\s\-]+$)/,
      message: 'Name must be only letters, spaces, or dashes',
      default: 'filename.json',
      required: false
    },
  }
}

If description is uncommented, then the before function is ignored. Why?

pattern has to be \b|(actual pattern) to allow empty strings, as in (http://stackoverflow.com/questions/3333461/regular-expression-which-matches-a-pattern-or-is-an-empty-string). Otherwise required flag is ignored and an error message is prompted, ignoring the default value and asking for a valid string.

Using Node.js 0.8.15 on Ubuntu 12.04

Contributor

Southern commented Dec 17, 2012

I just tested this on the latest master branch, and was unable to reproduce this error.

Here's the test and results.

var prompt = require('./lib/prompt');

var schema={
  properties:{
   filename: {
      before: function(value) { return 'v -> ' + value.red; },
//       description:'Enter output file name'.green,
      pattern: /\b|(^[a-zA-Z\s\-]+$)/,
      message: 'Name must be only letters, spaces, or dashes',
      default: 'filename.json',
      required: false
    },
  }
}

prompt.start();
prompt.get(schema, function (err, result) {
  if(err) {
    console.log('cancelled error');
    return;
  }
  console.log(err, result);
});
 ~/dev/node/flatiron/prompt master ❱ node test.js
prompt: filename:  (filename.json) 
null { filename: 'v -> \u001b[31mfilename.json\u001b[39m' }
 ~/dev/node/flatiron/prompt master ❱ node test.js
prompt: filename:  (filename.json) test.json
null { filename: 'v -> \u001b[31mtest.json\u001b[39m' }

If you're currently on the latest 0.2.8 version and seeing this problem, I can bump the version and get it published.

As for the second part of your issue, it just looks like a bad regex. You can use something like /^[a-z0-9\s\-\.]*$/i instead.

Yes, I'm using version 0.2.8 and I confirm the issue. If you just uncomment description ( line 7 in the above code) attribute, before function will be ignored.

For what concerns the regex you're right: using yours makes default work as expected. 👍

Contributor

pksunkara commented Dec 18, 2012

I agree with @Southern , I didn't see any code which can cause the mentioned error.

Ok, try this then:

var prompt = require('prompt');

var schema={
  properties:{
   filename: {
      before: function(value) { return 'v -> ' + value.toUpperCase(); },
      description:'Enter output file name',
      pattern: /^[a-z0-9\s\-\.]*$/i,
      message: 'Name must be only letters, spaces, or dashes',
      default: 'filename.json',
      required: false
    },
  }
}

prompt.start();
prompt.get(schema, function (err, result) {
  if(err) {
    console.log('cancelled error');
    return;
  }
  console.log(err, result);
});

Then tell me if you see the uppercase string. You may notice I fixed the regexp as suggested by southern.
Then comment the line: description:'Enter output file name', and you should get the uppercase string.
This is the little oddity I noticed.

Contributor

Southern commented Dec 18, 2012

Ah, okay. I misunderstood. Uncommenting the description does not work properly. This isn't a prompt error, though. The schemas you use are passed through flatiron/revalidator. I'll file an issue there with this, and mention this issue.

@Southern Southern referenced this issue in flatiron/revalidator Dec 18, 2012

Closed

"before" ignored if "description" is provided. #37

@Southern Southern was assigned Dec 19, 2012

@Southern Southern closed this in 6de2432 Dec 19, 2012

Great, I confirm it works now! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment