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

Improve example in README #69

Merged
merged 2 commits into from
Sep 13, 2020
Merged

Improve example in README #69

merged 2 commits into from
Sep 13, 2020

Conversation

akaihola
Copy link
Owner

@akaihola akaihola commented Sep 2, 2020

Also point out in a more clear way that the path arguments can be both files and directories.

@akaihola akaihola added the documentation Improvements or additions to documentation label Sep 2, 2020
@akaihola akaihola added this to the 1.2.0 milestone Sep 2, 2020
@akaihola akaihola self-assigned this Sep 2, 2020
@akaihola akaihola added this to In progress in Akaihola's Open source work via automation Sep 2, 2020
Copy link

@markddavidoff markddavidoff left a comment

Choose a reason for hiding this comment

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

sure, adding the file(s) helps clarify the current behavior, but i believe darker should also extend black's behavior by letting you pass multiple file paths.

README.rst Outdated Show resolved Hide resolved
README.rst Outdated
@@ -106,22 +106,36 @@ Example:

$ mkdir test && cd test && git init

Choose a reason for hiding this comment

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

Suggested change
$ mkdir test && cd test && git init

Don't really need this, as it is a given that we will have python files and be in a git directory. instead consider an example showing an error thrown when we are not in a git directory, with a comment re-statating that you need to be in a git directory. Those are often easier to find when people scan the docs while troubleshooting

Copy link
Owner Author

Choose a reason for hiding this comment

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

What I intended to do in this example is give the reader a complete set of command lines they can copy-paste on the terminal, and have the exact thing described happen on their own disk.

But you have a good point on troubleshooting. I'll try adding that.

README.rst Outdated

$ echo "if True: print('hi')\nprint()\nif False: print('there')" | tee test.py

Choose a reason for hiding this comment

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

For readability, i don't think you should include the code used to generate the examples as they themselves distract away from the actual example. I can't nest backticks in the suggestion block so i'll just make this a comment instead:

our_file.py at commit hash XXXXXXXX

if True: print('hi')

if False: print('there')

An uncommitted change to the our_file.py:

if True: print('changed')

if False: print('there')

Show changes to the file(s) darker would make

Note, the if False: print('there') is omitted as it was not modified!

$ darker --diff ./our_file.py -r XXXXXXXX
> --- test.py
> +++ test.py
> @@ -1,3 +1,4 @@
> -if True: print('changed')
> +if True:
> +    print("changed")

You can also specify all files in the current directory:
all files under the current directory

$ darker --diff . -r XXXXXXXX

Black the file(s) with darker!

$ darker ./our_file.py  -r XXXXXXXX

or

$ darker .  -r XXXXXXXX

Output our blacked file:
Note, the if False: print('there') is not blacked as it was not been modified!

$ cat ./our_file.py
> if True: 
>     print('changed')
> if False: print('there')

Choose a reason for hiding this comment

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

@akaihola hopefully you find these suggestions useful, if you'd like i can submit them in another pr

Copy link
Owner Author

@akaihola akaihola Sep 2, 2020

Choose a reason for hiding this comment

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

Yes, this is very useful! I actually wanted to combine your suggestions immediately wtih some of my own ideas – what do you think of the example now?

Copy link
Owner Author

Choose a reason for hiding this comment

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

One modification to your suggestion was that I didn't want to introduce the -r switch in the very first example – for local interactive use I expect the default comparison to HEAD to be the sane default most often. It's in the CI environment where --revision becomes especially handy.

@akaihola
Copy link
Owner Author

akaihola commented Sep 2, 2020

sure, adding the file(s) helps clarify the current behavior, but i believe darker should also extend black's behavior by letting you pass multiple file paths.

It does!

You can run

$ darker first.py second.py subdirectory/deep/third.py

@akaihola
Copy link
Owner Author

akaihola commented Sep 2, 2020

I've been actually dreaming about a screencast. It would be fantastic to show an actual coding session in which some parts of a heavily non-black-formatted repository are edited and interactively reformatted in PyCharm or VS Code using Darker.

Update: See #73.

@akaihola
Copy link
Owner Author

akaihola commented Sep 2, 2020

By the way @markddavidoff if you're interested to contribute more to Darker, I can invite you as a contributor. You may also be interested in #60 – let us know your time zone and schedules there in case you'd like to meet other contributors on-line.

@akaihola akaihola modified the milestones: 1.2.0, 1.3.0 Sep 2, 2020
Also point out in a more clear way that the path arguments can be both
files and directories.
@markddavidoff
Copy link

@akaihola unfortunately, although interested, I can't honestly say I'd be able to make time for any meaningful contributions. Please feel free to adopt my suggestions as you wish!

@akaihola
Copy link
Owner Author

@markddavidoff wrote:

Please feel free to adopt my suggestions as you wish!

Ok thanks for all your feedback Mark!

I added other contributors as additional reviewers – I hope someone check how I modified the example in README.rst to give a second opinion and to make sure I didn't miss something.

Copy link
Collaborator

@CorreyL CorreyL left a comment

Choose a reason for hiding this comment

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

Definitely makes what the example was originally trying to get across much easier to read!

Akaihola's Open source work automation moved this from In progress to Reviewer approved Sep 13, 2020
@akaihola akaihola merged commit 98401cb into master Sep 13, 2020
Akaihola's Open source work automation moved this from Reviewer approved to Done Sep 13, 2020
@akaihola akaihola deleted the directory-argument-doc branch September 13, 2020 19:21
@akaihola akaihola mentioned this pull request Nov 30, 2020
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants