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

fix Issue #285 - getOption returns false when option is set on command l... #331

Merged
merged 1 commit into from
Oct 3, 2013

Conversation

MartinNowak
Copy link
Contributor

...ine

  • It only returned true when an option from a config file
    was overwritten.
  • Workaround the missing return value of std.getopt.getopt by comparing
    the lengths of the passed args.

…command line

- It only returned true when an option from a config file
  was overwritten.

- Workaround the missing return value of std.getopt.getopt by comparing
  the lengths of the passed args.
@s-ludwig
Copy link
Member

s-ludwig commented Oct 3, 2013

Thanks! Looking at the code, my initial reaction also was that the command line should override the config setting and not vice versa. @CyberShadow, was the order here intentional?

@CyberShadow
Copy link
Contributor

Looking at the code, my initial reaction also was that the command line should override the config setting and not vice versa.

Yes, absolutely. The general convention is: command line > environment > user config file > global config file > hardcoded defaults. If the order in the code was different, that's a bug.

@s-ludwig
Copy link
Member

s-ludwig commented Oct 3, 2013

Okay thanks, I'll pull and make the change then.

s-ludwig added a commit that referenced this pull request Oct 3, 2013
fix Issue #285 - getOption returns false when option is set on command l...
@s-ludwig s-ludwig merged commit 5b661d5 into vibe-d:master Oct 3, 2013
@MartinNowak MartinNowak deleted the fix285 branch April 19, 2014 10:33
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.

3 participants