Skip to content

Conversation

tleonhardt
Copy link
Member

@tleonhardt tleonhardt commented May 27, 2018

Deleted postparse() since it was redundant with postparsing_precmd()
- Decided to keep postparsing_precmd() because of its more self-documenting name

  • Removed reference to postparse() in the docs

This partially addresses #417

Also:
- Deleted postparse() since it was redundant with postparsing_precmd()
@codecov
Copy link

codecov bot commented May 27, 2018

Codecov Report

Merging #420 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #420      +/-   ##
==========================================
+ Coverage   88.94%   88.97%   +0.02%     
==========================================
  Files           9        9              
  Lines        2678     2676       -2     
==========================================
- Hits         2382     2381       -1     
+ Misses        296      295       -1
Impacted Files Coverage Δ
cmd2/cmd2.py 90.32% <ø> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f88f81...12cc83b. Read the comment docs.

@tleonhardt tleonhardt requested review from kotfu and kmvanbrunt May 27, 2018 22:01
@tleonhardt tleonhardt self-assigned this May 27, 2018
@tleonhardt tleonhardt added this to the 0.9.0 milestone May 27, 2018
@tleonhardt
Copy link
Member Author

As long as everyone is happy with how I handled this issue, then this is the last thing I wanted to get in before we do a 0.9.0 release.

I think we are ready for an 0.8.6 release as-is from the python2 branch.

@tleonhardt tleonhardt requested a review from anselor May 27, 2018 22:05
Copy link
Member

@kotfu kotfu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd defer the preparse() and postparse() changes until a later release. As currently implemented the preparse() call will not receive all of the arguments of a multiline command.

@tleonhardt
Copy link
Member Author

@kotfu I think I might revert the preparse behavior in this PR, but leave the deletion of postparse if that seems reasonable to you. And then you can fix preparse as part of the changes you are working on for #410.

@tleonhardt
Copy link
Member Author

@kotfu Are you happy with the postparse() deletion present in this PR for now?

@kotfu
Copy link
Member

kotfu commented May 28, 2018

Yup.

@tleonhardt tleonhardt merged commit 76c2961 into master May 28, 2018
@tleonhardt tleonhardt deleted the preparse branch May 28, 2018 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants