addArgument([...],{defaultValue:0}) does not work

[hpaulj]

Trying to set the Action defaultValue to a falsie via addArgument does not work.
The problem is with the if (!options.defaultValue) { test. It is supposed to detect
when this option is not defined, but instead it rejects all falsies, including 0.
A better translation of the Python block is:

  // closer Python translation
  if (_.isUndefined(options.defaultValue)) {
    var dest = options.dest;
    if (_.has(this._defaults, dest)) {
      options.defaultValue = this._defaults[dest];
    } else if (!_.isUndefined(this.argumentDefault)) {
      options.defaultValue = this.argumentDefault;
    }
  }

(options.defaultValue == null) would also work, due to how == is defined.

On a related note, the getDefault function does not work, returning null all the time
It is not called by other argparse code, but can be called by the user. Part of the problem is the test on action.defaultValue. But also the return action.defaultValue only returns from the inner function, not outer one.

// corrected version, based on a Coffeescript implementation
ActionContainer.prototype.getDefault = function (dest) {
  var result, _ref;
  // Python: self._defaults.get(dest, None)
  // coffee: result = this._defaults[dest] ? null
  result = (_ref = this._defaults[dest]) != null ? _ref : null;
  this._actions.forEach(function (action) {
    if (action.dest === dest && action.defaultValue !== null) {
      result = action.defaultValue;
    }
  });
  return result;
};

In looking for other uses of defaultValue, I found an error in ActionContainer._registryGet. It incorrectly translates the Python default=None optional argument. The defaultValue argument in this case is not connected with the previously one. The simplest correction is to simply omit the test, since the function is always called with 3 arguments.

ActionContainer.prototype._registryGet = function (registryName, value, defaultValue) {
  // defaultValue = defaultValue || null; // does not implement "default=None"
  return this._registries[registryName][value] || defaultValue;
};

[puzrin]

@hpaulj thanks a lot. Could you do it as pull requests and add tests? That will greately speedup accepting fixes.

[hpaulj]

On Sun, Dec 9, 2012 at 8:49 AM, Vitaly Puzrin notifications@github.comwrote:

@hpaulj https://github.com/hpaulj thanks a lot. Could you do it as pull
requests and add tests? That will greately speedup accepting code,

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/22#issuecomment-11172787.

I've committed a number of changes to my fork. I need to read up on pull
requests.

Paul

[puzrin]

Ah, i see now. It worth to follow simple rules:

1. Do each fix in separate branch (to make separate pull requests). If one commit will be rejected/postponed, that will not delay others.
2. Add test to each fix. We are not fans of TDD, but all fixes should be covered.

[puzrin]

Fixed in ither commit