-
Notifications
You must be signed in to change notification settings - Fork 56
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
No additional newline with --stdout
flag.
#257
No additional newline with --stdout
flag.
#257
Conversation
The I'm also noticing that we didn't really have tests for the output behavior of |
Ah, and the Actually @magnunm would you like to become a collaborator on the repository and review the fix in #258? After merging #258, you should rebase your branch on Also, by a lucky chance, the colorization test answers my question about testing TTY output ( def test_colorize(text, lexer, tty, expect):
"""``colorize()`` produces correct highlighted terminal output"""
with patch("sys.stdout.isatty", Mock(return_value=tty)):
result = with_pygments.colorize(text, lexer)
assert result in expect |
Will see what I can do about the |
e26309d
to
5ffbf0b
Compare
548ed94
to
56626f3
Compare
Could you rebase on I'll take a look at the py<3.8 test failures next. Maybe a context manager syntax compatibility issue? |
src/darker/tests/test_main.py
Outdated
with ( | ||
patch("sys.stdout.isatty", Mock(return_value=tty)), | ||
patch( | ||
"darker.__main__._import_pygments", | ||
Mock( | ||
return_value=darker.__main__._import_pygments(), | ||
side_effect=None if with_pygments else ImportError(), | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parenthesized context managers are a recent addition to Python syntax. We should support older versions using this old uglier syntax (made even worse by Black):
with ( | |
patch("sys.stdout.isatty", Mock(return_value=tty)), | |
patch( | |
"darker.__main__._import_pygments", | |
Mock( | |
return_value=darker.__main__._import_pygments(), | |
side_effect=None if with_pygments else ImportError(), | |
), | |
with patch("sys.stdout.isatty", Mock(return_value=tty)), patch( | |
"darker.__main__._import_pygments", | |
Mock( | |
return_value=darker.__main__._import_pygments(), | |
side_effect=None if with_pygments else ImportError(), |
56626f3
to
c10b39d
Compare
Because `print()` adds a newline to the end of its string argument, the output with `--stdout` contained an additional newline compared to the result of modifying the file.
Unit-test for the `print_source` function used when the `--stdout` option is applied. To test the behaviour with/without pygments the importing of pygments was extracted to a function.
c10b39d
to
66f38fb
Compare
Sourcery Code Quality Report✅ Merging this PR will increase code quality in the affected files by 0.41%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, ready to merge in my opinion!
Fixes #250
master
after Add compatibility with Black >= 22.1 #270 has been merged