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

Add a --diff option to match black behavior. #4

Merged
merged 22 commits into from
Jul 3, 2020

Conversation

Carreau
Copy link
Collaborator

@Carreau Carreau commented Jun 13, 2020

This allows for example to use darker as a formatter for vscode with
minimal changes as vscode sends the --quiet and --diff flags to the
formatter.

"python.formatting.provider": "black",
"python.formatting.blackPath": "/path/to/darker"

This allows for example to use darker as a formatter for vscode with
minimal changes as vscode sends the --quiet and --diff flags to the
formatter.

    "python.formatting.provider": "black",
    "python.formatting.blackPath": "/path/to/darker"
@Carreau
Copy link
Collaborator Author

Carreau commented Jun 13, 2020

A bit ugly of an implementation, mainly to know if you would be ok with it.

I was also happy to find this project as I was really unhappy with inserting/removing format:on/off in darken, and ended up with the same diff idea as you and found you implemented it.

@Carreau
Copy link
Collaborator Author

Carreau commented Jun 13, 2020

Also just deprecated darken and point to darken now.
(https://pypi.org/project/darken/)

@akaihola
Copy link
Owner

Hey @Carreau this is a great contribution! I've started to review – please expect a couple of suggestions.

Copy link
Owner

@akaihola akaihola left a comment

Choose a reason for hiding this comment

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

There you go, just a few minor notes to keep the code style consistent and skip some unnecessary operations.

Do you think you could add a simple test case for the --diff option?

src/darker/__main__.py Outdated Show resolved Hide resolved
src/darker/__main__.py Outdated Show resolved Hide resolved
src/darker/__main__.py Outdated Show resolved Hide resolved
src/darker/__main__.py Outdated Show resolved Hide resolved
src/darker/command_line.py Outdated Show resolved Hide resolved
@Carreau
Copy link
Collaborator Author

Carreau commented Jun 21, 2020

There you go, just a few minor notes to keep the code style consistent and skip some unnecessary operations.

Do you think you could add a simple test case for the --diff option?

Yes, of course, I mainly wanted to test the waters. I'll cleanup, and I'll also see if I can make it work with isort, which I'm not sure it does now.

Copy link
Owner

@akaihola akaihola left a comment

Choose a reason for hiding this comment

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

@Carreau, I sat down and addressed my own review items. I hope you didn't have unpushed changes for those...

--diff indeed doesn't yet work together with --isort – I made that combination raise a NotIimplementedError for now. Let's implement that in a separate PR if needed.

@akaihola akaihola mentioned this pull request Jul 1, 2020
akaihola added 2 commits July 2, 2020 00:54
- `--isort` and `--diff` now work together
- diff original unmodified and user-edited versions of files in the
  Git directory using Python's difflib, not by parsing `git diff`
  output
- process each edited file individually
- only run `isort` for edited files
- write back `isort` modifications together with `black`
  modifications, and skip writing if there are errors
- remove code that became unused
- avoid extra conversions between source code as a string and a list
  of line strings
- add some tests for previously untested functions
@akaihola akaihola merged commit cd0956a into akaihola:master Jul 3, 2020
@akaihola akaihola added this to the 1.0.0 milestone Jul 11, 2020
@akaihola akaihola added the enhancement New feature or request label Jul 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants