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

Fixes: #61. Allows subsequent text blocks. #62

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

Conversation

ioggstream
Copy link

What does this PR do?

Allows subsequent code and text blocks.

Reimplements text and code block merging tracking position.

for i, j in text_limits]

# remove indents
list(map(self.pre_process_code_block, code_blocks))
for pos, block in code_blocks:
Copy link
Author

Choose a reason for hiding this comment

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

Using a for-loop explicitly states that we are side-effecting and we're not interested in the map return.


# Merge back text and code blocks using the
# start position.
all_blocks = sorted(text_blocks + code_blocks)
Copy link
Author

Choose a reason for hiding this comment

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

sorted() uses the first tuple() argument (pos, an integer) to sort blocks.

@ioggstream ioggstream force-pushed the 61-allow-subsequent-text-blocks branch 2 times, most recently from 2466ec8 to f781fac Compare November 13, 2017 17:06
@ioggstream
Copy link
Author

closed for recheck

@ioggstream ioggstream closed this Nov 13, 2017
@ioggstream ioggstream reopened this Nov 13, 2017
@aaren
Copy link
Owner

aaren commented Nov 16, 2017

hey @ioggstream, thanks for the PR! Could you modify the tests so that they fail before this change and pass after?

@ioggstream
Copy link
Author

ioggstream commented Nov 16, 2017

@aaren I submitted two patches in one. I will split this patch so that it does not modify the behavior of the code.

@ioggstream
Copy link
Author

@aaren prerequisite patch #63

@ioggstream ioggstream force-pushed the 61-allow-subsequent-text-blocks branch from f781fac to a63a128 Compare November 16, 2017 18:23
@ioggstream ioggstream force-pushed the 61-allow-subsequent-text-blocks branch from a63a128 to 64f34ef Compare November 16, 2017 18:25
@ioggstream
Copy link
Author

@aaren fixed this PR: now it has the same output testcase than before.

@aaren
Copy link
Owner

aaren commented Nov 16, 2017

I mean can you add something to tests.py that causes it to fail before this PR and to pass afterwards? I don't fully understand what this PR is fixing, as I think subsequent code blocks are already supported.

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.

2 participants