Skip to content

Conversation

@kmvanbrunt
Copy link
Member

@kmvanbrunt kmvanbrunt commented May 16, 2018

Possible fix for #402

This closes #402

@kmvanbrunt kmvanbrunt requested review from kotfu and tleonhardt May 16, 2018 04:23
@codecov
Copy link

codecov bot commented May 16, 2018

Codecov Report

Merging #403 into master will increase coverage by 0.66%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #403      +/-   ##
==========================================
+ Coverage   89.48%   90.15%   +0.66%     
==========================================
  Files           8        8              
  Lines        2530     2528       -2     
==========================================
+ Hits         2264     2279      +15     
+ Misses        266      249      -17
Impacted Files Coverage Δ
cmd2/parsing.py 98.26% <100%> (-0.02%) ⬇️
cmd2/cmd2.py 91.56% <0%> (+1.02%) ⬆️
cmd2/rl_utils.py 100% <0%> (+4.34%) ⬆️

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 a9b7121...490c842. Read the comment docs.

tleonhardt
tleonhardt previously approved these changes May 16, 2018
try:
pos = tokens.index(test_terminator)
if pos < terminator_pos:
terminator_pos = len(tokens) + 1
Copy link
Member

Choose a reason for hiding this comment

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

Looks ok to me

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 don't think we are quite there yet. I just committed a bunch of unit tests that I think should pass, and many of them don't.

@tleonhardt tleonhardt added this to the 0.9.0 milestone May 17, 2018
assert statement.argv == ['has', '>', 'inside']
assert statement.terminator == ';'

@pytest.mark.parametrize('line,terminator',[
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding lots of unit tests to cover this functionality

@tleonhardt tleonhardt merged commit 8125d45 into master May 17, 2018
@tleonhardt tleonhardt deleted the terminator_run branch May 17, 2018 16:29
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.

Allow continuous run of terminators to end a multiline command

3 participants