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

Normalize output format handling (-t) to match input (-f) #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alerque
Copy link
Contributor

@alerque alerque commented Apr 12, 2019

Fixes #5.

There were a couple logic problems.

  1. The results of processing the --to argument were getting clobbered by file name guessing
  2. There were two variables being held with the target format, args.to and output_format. They were used in different places and it was possible for them to be out of sync, causing unexpected results.

This normalizes the whole mess on one variable (args.to_format) and handles setting it just like --from instead of ... differently. The only place other variables are used is in passing arguments to panflute, which apparently wants named arguments in a slightly different format.

I have not been able to test this very exhaustively with all the backends and situations that might be affected. Please proceed with caution on this one, and if something needs fixing I'd be happy to do it.

Depending on what order you merge this or #11 might need to be rebased before merging. Feel free to do ether one and I'll update the PR for the other.

@alerque alerque force-pushed the bugfix/to-format branch 3 times, most recently from 5f33e9f to 43712aa Compare April 12, 2019 13:06
@coveralls
Copy link

Pull Request Test Coverage Report for Build 42

  • 20 of 23 (86.96%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-1.9%) to 87.547%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pancritic/main.py 14 17 82.35%
Files with Coverage Reduction New Missed Lines %
pancritic/main.py 1 86.73%
Totals Coverage Status
Change from base Build 36: -1.9%
Covered Lines: 184
Relevant Lines: 205

💛 - Coveralls

@alerque
Copy link
Contributor Author

alerque commented Apr 15, 2019

I'm not familiar with testing frameworks in python and am not sure what, if anything, needs to be done about the coverage issue. Feedback welcome.

@alerque alerque force-pushed the bugfix/to-format branch from 43712aa to 0efd83a Compare May 22, 2019 07:01
@alerque
Copy link
Contributor Author

alerque commented May 22, 2019

Rebased to resolve merge conflict...

@alerque
Copy link
Contributor Author

alerque commented May 22, 2019

Travis failure appears to be related to #13 again. I think this is working as expected.

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.

To flag does not work as advertized
2 participants