-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Enable pretty by default #19510
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
Open
null-dreams
wants to merge
14
commits into
python:master
Choose a base branch
from
null-dreams:enable-pretty-by-default
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Enable pretty by default #19510
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
f94fc8f
feat: Enable --pretty by default.
null-dreams 54a0196
fix(tests): Adapt TypeCheckSuite to default to non-pretty output
null-dreams e0d968e
fix(tests): Adapt CmdlineSuite to default to non-pretty output
null-dreams 201f436
fix(tests): Adapt pythoneval suite to default to non-pretty output
null-dreams 9d498a8
fix(tests): Adapt errorstream suite to default to non-pretty output
null-dreams 0a9e326
fix(tests): Adapt DaemonSuite to handle new pretty default
null-dreams f07217d
fix(tests): Adapt PEP561Suite to default to non-pretty output
null-dreams d4de5a7
fix(tests): Update Stubtest unit tests to expect pretty output
null-dreams ce8cc29
docs: Update docs for pretty-by-default behavior
null-dreams 7b79b5a
style: Fix ruff linting error in testdaemon.py
null-dreams d67c367
chore: Re-trigger CI
null-dreams afc6585
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 23e5980
fix: Remove redundant `flag_list` in `parse_options`
null-dreams 66121c9
refactor(cli): Simplify flag implementation using inverted `add_inver…
null-dreams File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Okay, I must be missing something - why won't just checking for
--pretty
flag do? Yes, it will deviate from normal behavior, but be consistent with all other tests - essentially sticking to old default unless requested otherwise.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.
Hello @sterliakov , this is still my main approach and it is also partly working in the this test-runner to maintain the consistent behavior with all other test-runners.However, I discovered a critical technical constraint: the
dmypy
client itself does not accept the--pretty
or--no-pretty flags
.The formatting flags must be directly passed to the server, which only worked in these two ways for me:
- During startup:
$ dmypy start -- --no-pretty
- During one-shot run:
$ dmypy run -- --no-pretty
Because the testcases contains a mix of commands where the flag will and won't work, I had to make
#NO-MODIFY
as the magic comment to recognize where to not modify the command.For demonstration:
In this case, my first approach was to insert the
--pretty
flag in all of them. Butdmypy recheck
threw an unrecognized argument error. So I realized I cannot insert any flag in the commands followingdmypy start...
Therefore, I preferred to adding
--pretty
flag during startup. But then the test-runner logic detecting so such--pretty
flag in the other commands automatically injects the--no-pretty
flag which also throws an unrecognized argument error. This led to the realization, that I also need to mark these commands someway, that the test-runner doesn't inject--no-pretty
in them. The best I could think of was a magic comment#NO-MODIFY
which I can use to detect these lines where the test-runner shouldn't be interfering.I understand that this is a very bare-bones workaround, and I am open to any suggestions to make it cleaner.