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 links regarding Spotless integration for gradle/maven users #1622

Merged
merged 8 commits into from
Aug 26, 2020

Conversation

nedtwigg
Copy link
Contributor

@nedtwigg nedtwigg commented Aug 25, 2020

If there's a better place to suggest this edit, happy to move it around. I know Gradle is a niche tool in the Python world, happy to be somewhere quieter than the main README, but it seems that the README gets sliced up into smaller pages somehow? aha, your CI helped me understand. I think it's in the right place now :)

Spotless is a code formatting plugin. It's main value adds are:

Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this PR! I feel like this could be more concise though.

README.md Outdated
@@ -406,6 +406,22 @@ jobs:
- uses: psf/black@stable
```

## Spotless
Copy link
Collaborator

Choose a reason for hiding this comment

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

A separate section for a single tool that's fairly obscure to the Python community seems like overkill. Maybe we could mention this in the "editor integrations" section?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll move it there :). I was looking for an "integrations" or "build tools" section. We don't quite fit "editor integrations", but we're pretty close.

README.md Outdated
formatting so that only files which have changed since `origin/main` get formatted
- slurp copyright info from
[git history](https://github.com/diffplug/spotless/tree/main/plugin-gradle#license-header)
- seamlessly integrate with
Copy link
Collaborator

Choose a reason for hiding this comment

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

Integrations with formatters for other languages don't feel relevant to Black's documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I see it, someone who comes to your website is saying "I want to stop caring about formatting", not "I want to stop caring about formatting just my python, but keep caring about my html templates, c code, and markdown documentation". So I think it's relevant that if you use Spotless as your integration layer, you only have one interface to worry about, versus "I'll use project.toml to format my python, precommit for my docs, and a shell-script to clang-format for my c-code."

The fact that this project relies on prettier for formatting its docs is a good example that the people who bother to use one autoformatter usually want others, and it's nice to be able to configure them all from one place, which is the whole point of Spotless.

One argument for keeping Spotless as a peer to VCS is that it offers unique features related to slurping info from git history, which can be valuable for projects with strict legal requirements, especially polyglot projects. Regardless, I'll move Spotless to editor integration, and you can pick whichever commit you prefer :)

@nedtwigg
Copy link
Contributor Author

Got the precommit hooks installed, which fixed my format violations (ironic!). The experience I just had, as a new prospective contributor, was:

  • CI error message indicates pre-commit is the cause
  • read CONTRIBUTING.md more carefully
  • install and fiddle with pre-commit

I love the QC, and it's very cool how pre-commit orchestrates tools from multiple ecosystems (e.g. prettier). This workflow is exactly what Spotless does for Gradle projects, it just does it inline with the user experience that Gradle devs are already used to, where the user experience would have been this:

bot@ci repo% ./gradlew build
:spotlessJavaCheck FAILED
  The following files had format violations:
  src\main\java\com\diffplug\gradle\spotless\FormatExtension.java
    -\t\t····if·(targets.length·==·0)·{
    +\t\tif·(targets.length·==·0)·{
  Run './gradlew spotlessApply' to fix these violations.
user@machine repo % ./gradlew spotlessApply
:spotlessApply
BUILD SUCCESSFUL
bot@ci repo % ./gradlew build
BUILD SUCCESSFUL

Convergent evolution from a different ecosystem :)

@nedtwigg
Copy link
Contributor Author

All tests passed except Primer / build (3.8, macOS-latest) which looks to be flaky. I'll go dormant to stop creating noise, happy to push an empty commit to trigger a rerun or respond to any other requests.

@ambv
Copy link
Collaborator

ambv commented Aug 26, 2020

Got the precommit hooks installed, which fixed my format violations (ironic!)

Yeah, admittedly this flow is not super great when editing on the Web, as I just found out myself 😅

Add missing newline
@ambv ambv merged commit 4a065a4 into psf:master Aug 26, 2020
@ambv
Copy link
Collaborator

ambv commented Aug 26, 2020

Thanks! ✨ 🍰 ✨

@nedtwigg
Copy link
Contributor Author

Thank you!

anderson-dan-w pushed a commit to sigopt/black that referenced this pull request Aug 31, 2020
* Find project root correctly (psf#1518)

Ensure root dir is a common parent of all inputs
Fixes psf#1493

* Add pip install from GitHub command to README.md (psf#1529)

* Add pip install from GitHub command to README.md
* Make it prettier ...

* Mozilla uses black too (psf#1531)

* add Quora to orgs that use Black (psf#1532)

* ISSUE 1533: Fix --config argument description (psf#1534)

Change --config argument description to "Read configuration from FILE."
The "--config FILE                   Read configuration from FILE path"

* Spelling fix in CONTRIBUTING.md (psf#1547)

* Ensure path for finding root is absolute (psf#1550)

As Path.resolve() is buggy on windows (see https://bugs.python.org/issue38671)
an absolute path is ensured by prepending the Path.cwd()

* Update curl command to use stable branch (psf#1543)

* pre-commit: show diff on failure on CI (psf#1552)

* pre-commit: --show-diff-on-failure

* pre-commit: --show-diff-on-failure

* docs: Improve pre-commit use (psf#1551)

Stable tag wasn't available and crashed when attempting to set initial
pre-commit. Also the python version needs to be installed so it would
be better to use the generic "python3" command.

* Update to accomodate isort 5 release changes. (psf#1559)

Isort 5 introduced profiles and ensure_newline_before_comments options. Either needs to be added to work correctly with black.

Co-authored-by: Richard Si <[email protected]>

* fix spelling (psf#1567)

Co-authored-by: Hugo van Kemenade <[email protected]>

* Remove slow assertion (psf#1592)

Partial fix for psf#1581

This assertion produces behavior quadratic in the number of leaves in a line, which is making Black extremely slow on files with very long expressions. On my benchmark file this change makes Black 10x faster.

* Add the direnv base directory to the default excludes (psf#1564)

Co-authored-by: Chris Rose <[email protected]>

* Make --exclude only apply to recursively found files (psf#1591)

Ever since --force-exclude was added, --exclude started to touch files
that were given to Black through the CLI too. This is not documented
behaviour and neither expected as --exclude and --force-exclude now
behave the same!

Before this commit, get_sources() when encountering a file that was passed
explicitly through the CLI would pass a single Path object list to
gen_python_files(). This causes bad behaviour since that function
doesn't treat the exclude and force_exclude regexes differently. Which
is fine for recursively found files, but *not* for files given through
the CLI.

Now when get_sources() iterates through srcs and encounters
a file, it checks if the force_exclude regex matches, if not, then the
file will be added to the computed sources set.

A new function had to be created since before you can do regex matching,
the path must be normalized. The full process of normalizing the path is
somewhat long as there is special error handling. I didn't want to
duplicate this logic in get_sources() and gen_python_files() so that's
why there is a new helper function.

* in verbose mode, print stack trace (psf#1594)

Make Black failures easier to debug

* fix some docstring crashes (psf#1593)

Allow removing some trailing whitespace

* don't strip brackets before lsqb (psf#1575) (psf#1590)

if the string contains a PERCENT, it's not safe to remove brackets that
follow and operator with the same or higher precedence than PERCENT

* fix unary op detection (psf#1600)

* Fix inline code style in README (psf#1608)

Refer to `pyproject.toml` in HTML section of README with HTML code tags

* Update all dependencies to latest versions

* Disable string splitting/merging by default (psf#1609)

* put experimental string stuff behind a flag
* update tests
* don't need an output section if it's the same as the input
* Primer: Expect no formatting changes in attrs, hypothesis and poetry with --experimental-string-processing off

Co-authored-by: Hugo van Kemenade <[email protected]>

* Upgrade docs to Sphinx 3+ and add doc build test (psf#1613)

* Upgrade docs to Sphinx 3+
* Fix all the warnings...

- Fixed bad docstrings
- Fixed bad fenced code blocks in documentation
- Blocklisted some sections from being generated from the README
- Added missing documentation to index.rst
- Fixed an invalid autofunction directive in reference/reference_functions.rst
- Pin another documentation dependency

* Add documentation build test

* Reset trailing comma handling

* Re-implement magic trailing comma handling:

- when a trailing comma is specified in any bracket pair, that signals to Black
  that this bracket pair needs to be always exploded, e.g. presented as "one
  item per line";

- this causes some changes to previously formatted code that erroneously left
  trailing commas embedded into single-line expressions;

- internally, Black needs to be able to identify trailing commas that it put
  itself compared to pre-existing trailing commas. We do this by using/abusing
  lib2to3's `was_checked` attribute.  It's True for internally generated
  trailing commas and False for pre-existing ones (in fact, for all
  pre-existing leaves and nodes).

Fixes psf#1288

* Reformat docs/conf.py according to the new style

* Open file explicitly with UTF-8 so it works on Windows, too

* Mark Primer projects that will change formatting

* Require Sphinx 3

* Use properly renamed function name in docs

* Fix dealing with generated files in docs

* Property-based fuzz test

* Update the changelog

* Make doc generation a little smarter, update doc sections

* Add more trailing comma test variants

* Run trailing comma tests with TargetVersion.PY38

* Address pre-existing trailing commas when not in the rightmost bracket pair

This required some hackery.  Long story short, we need to reuse the ability to
omit rightmost bracket pairs (which glues them together and splits on something
else instead), for use with pre-existing trailing commas.

This form of user-controlled formatting is brittle so we have to be careful not
to cause a scenario where Black first formats code without trailing commas in
one way, and then looks at the same file with pre-existing trailing commas
(that it itself put on the previous run) and decides to format the code again.

One particular ugly edge case here is handling of optional parentheses.  In
particular, the long-standing `line_length=1` hack got in the way of
pre-existing trailing commas and had to be removed.  Instead, a more
intelligent but costly solution was put in place: a "second opinion" if the
formatting that omits optional parentheses ended up causing lines to be too
long.  Again, for efficiency purposes, Black reuses Leaf objects from blib2to3
and modifies them in place, which was invalid for having two separate
formattings.  Line cloning was used to mitigate this.

Fixes psf#1619

* Improve docstring re-indentation handling

This addresses a few crashers, namely:

* producing non-equivalent code due to mangling escaped newlines,

* invalid hugging quote characters in the docstring body to the docstring outer
  triple quotes (causing a quadruple quote which is a syntax error),

* lack of handling for docstrings that start on the same line as the `def`, and

* invalid stripping of outer triple quotes when the docstring contained
  a string prefix.

As a bonus, tests now also run when string normalization is disabled.

* Add links regarding Spotless integration for gradle/maven users (psf#1622)

Co-authored-by: Łukasz Langa <[email protected]>

* Primer update config - enable pytest (psf#1626)

Reformatted projects I have acceess to:
- aioexabgp
- bandersnatch
- flake8-bugbear

```
-- primer results 📊 --

13 / 16 succeeded (81.25%) ✅
0 / 16 FAILED (0.0%) 💩
 - 3 projects disabled by config
 - 0 projects skipped due to Python version
 - 0 skipped due to long checkout
```

* Also re-enable pytest

```
-- primer results 📊 --

14 / 16 succeeded (87.5%) ✅
0 / 16 FAILED (0.0%) 💩
 - 2 projects disabled by config
 - 0 projects skipped due to Python version
 - 0 skipped due to long checkout

real	2m26.207s
user	17m55.404s
sys	0m43.061s
```

* v20.8b0

* Treat all trailing commas as pre-existing, as they effectively are

On a second pass of Black on the same file, inserted trailing commas are now
pre-existing.  Doesn't make sense to differentiate between the passes then.

* Include mode information for unstable formattings

* Add expected failure tests with the unstable formattings

* Make dependency on Click 7.0, regex 2020.1.8, and toml 0.10.1 explicit

* v20.8b1

Co-authored-by: Lihu Ben-Ezri-Ravin <[email protected]>
Co-authored-by: Cooper Lees <[email protected]>
Co-authored-by: Sylvestre Ledru <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Olexiy <[email protected]>
Co-authored-by: jtpavlock <[email protected]>
Co-authored-by: dhaug-op <[email protected]>
Co-authored-by: Steven Maude <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Vinicius Gubiani Ferreira <[email protected]>
Co-authored-by: Maximilian Cosmo Sitter <[email protected]>
Co-authored-by: Richard Si <[email protected]>
Co-authored-by: Kaligule <[email protected]>
Co-authored-by: Chris Rose <[email protected]>
Co-authored-by: Chris Rose <[email protected]>
Co-authored-by: David Szotten <[email protected]>
Co-authored-by: Daanyaal Syed <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
Co-authored-by: Zac-HD <[email protected]>
Co-authored-by: Ned Twigg <[email protected]>
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.

3 participants