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

WIP common tests #62

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

Conversation

hroncok
Copy link
Member

@hroncok hroncok commented Dec 20, 2022

Fixes #60

Opening as a draft. I can continue this way with more tests, but i wanted to know from @frenzymadness if this approach makes sense or is too cryptic.

Copy link
Member

@frenzymadness frenzymadness left a comment

Choose a reason for hiding this comment

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

In general, I like it.


def expand_tox(text):
"""Replace [[TOX4:...]] and [[TOX3:...]] expressions,
empty liens are stripped if created."""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
empty liens are stripped if created."""
empty lines are stripped if created."""

empty liens are stripped if created."""
source_lines = text.splitlines()
out_lines = []
for line in source_lines:
Copy link
Member

Choose a reason for hiding this comment

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

Only this loop is kinda cryptic. I'd add comments saying that the first if not line is there to preserve empty lines in the original output and the second if line is there to avoid adding more empty lines if one of the inline expressions creates one.

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.

Examine the tox4 plugin tests and make sure all tox3 tests are covered
2 participants