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 some subtle option parsing problems: #265

Merged
merged 1 commit into from Nov 23, 2015

Conversation

gdetrez
Copy link
Collaborator

@gdetrez gdetrez commented Nov 21, 2015

  • If the section doesn't exist, _bool_option should return the given
    default value, not False
  • The fourth argument of conf.get() is not a fallback value but controls
    how the value is parsed ([1])
  • "False" is true-ish, which was causing legacy_matching to be true by
    default

[1] https://docs.python.org/2/library/configparser.html#ConfigParser.ConfigParser.get

In addition. the legacy_matching option behave erratically if the issue description contains pipes (|). But since it seems deprecated, I don't know if it's worth fixing.

- If the section doesn't exist, `_bool_option` should return the given
  default value, not `False`
- The fourth argument of `conf.get()` is not a fallback value but controls
  how the value is parsed ([1])
- `"False"` is true-ish, which was causing `legacy_matching` to be true by
  default

[1] https://docs.python.org/2/library/configparser.html#ConfigParser.ConfigParser.get
@ralphbean
Copy link
Sponsor Collaborator

Oh, good good good. Thank you!

FWIW, I'd love to kill off legacy_matching two minor version bumps from now.

ralphbean added a commit that referenced this pull request Nov 23, 2015
Fix some subtle option parsing problems:
@ralphbean ralphbean merged commit 73c6e09 into GothenburgBitFactory:develop Nov 23, 2015
@gdetrez gdetrez deleted the false-is-true branch May 20, 2016 14:54
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