Why don't we use an auto formatter such as Black?

[m4rc1e]

This isn't issue, I'm just curious why we don't use something like Black?

I find manually formatting files tedious. I just want to sling code and let an auto formatter do the grunt work.

If there's a way to automate fb code formatting so it passes pylint, I'm all ears.

[felipesanches]

No tool currently auto-formats in the style that we use here. I have considered adapting some auto formatter to enforce our code-style, but I still haven't done so.

This is an example of our code-style:

There are a few code-style decisions here that are not what auto-formatters would have done, but which I consider good for code legibility. I'm aware of how biased this conversation can get. I'm just not willing to change the code-style at this point. It would be much better for me to spend some time automating the existing tools to match this style instead.

But let's not get distracted by this. I think the important point in your initial question is that you seem to be having trouble with pylint. Can you show me some examples of pylint complaining about your code? Is this related to your ongoing work on this pull request? (PR #3194)

[m4rc1e]

I'm just not willing to change the code-style at this point.

That's fair enough and totally understandable.

Can you show me some examples of pylint complaining about your code?

I get a few nits every pr but it's nothing serious. I'm just lazy and would prefer to not even think about formatting tbh. My biggest gripe is vertical alignment e.g

yield FAIL, \
      Message("out-of-bounds",
              "Point is out by ...")

Black would instead do

yield FAIL, Message(
    "out-of-bounds",
    "Point is out by ..."
)

I get that our current version is nicer, it's just a pita when you need to refactor.

[felipesanches]

This specific case won't result in pylint warnings. So we're good here.

And I've been always completely happy making fine-tuning adjustments to the PRs I receive so that we don't have to change the code-style.

(By the way, I see that your current pylint warnings/errors are related to actual problems on PR #3194: some variables defined but not used.)

[anthrotype]

that your "current version is nicer" is only a matter of taste. Auto-enforcing code formatting spares humans from arguing about subjective style and let them focus on more meaningful stuff instead. And IMO it also lowers the bar for new contributors. But I get that for established projects (vs new ones that start from scratch) can be trickly initially to do the transition for various reasons.

[miguelsousa]

@felipesanches Can you please reconsider using Black?
I'm getting setup to contribute a few checks to the fvar and universal profiles, and had to mute 19 lint errors (list below). Now I feel like I will be coding half blind.
If you want to welcome external contributions this project must adopt a mainstream code style. There's really no better way around it.

"pep8_ignore":[
    "E111", // indentation is not a multiple of four
    "E114", // indentation is not a multiple of four (comment)
    "E116", // unexpected indentation (comment)
    "E117", // over-indented (comment)
    "E122", // continuation line missing indentation or outdented
    "E126", // continuation line over-indented for hanging indent
    "E127", // continuation line over-indented for visual indent
    "E128", // continuation line under-indented for visual indent
    "E129", // visually indented line with same indent as next logical line
    "E203", // whitespace before ‘:’
    "E231", // missing whitespace after ‘,’, ‘;’, or ‘:’
    "E251", // unexpected spaces around keyword / parameter equals
    "E261", // at least two spaces before inline comment
    "E303", // too many blank lines (3)
    "E302", // expected 2 blank lines, found 0
    "E306", // expected blank line before a nested definition
    "E501", // line too long (> 88 characters)
    "W293", // blank line contains whitespace
    "W503", // line break occurred before a binary operator
],

[miguelsousa]

@felipesanches Any progress on this? Can I submit Black-formatted changes which you'll then reformat to your style?

[felipesanches]

yes, you can submit PRs and I will adapt them if I feel the need to.

Regarding auto-formatters, I am trying to find a solution that's acceptable but I still haven't got a final decision.

[felipesanches]

Regarding the pep8_ignore list you mentioned, we have something similar to that for pylint and flake8 at https://github.com/googlefonts/fontbakery/blob/9a5535ccff0f44ba71cedae1e487be6a94166e56/tox.ini so it is setup for our continuous integration linting.

[miguelsousa]

Good to know. Seeing the amount of items that have to be ignored confirms that FB's style is very custom.

[felipesanches]

well... some of those lints were simply silenced because we chose to not address them years ago, while we also wanted to benefit from the remaining lints provided by flake8 and pylint. And I never felt any pressure to prioritizing this.

Other ones, yes, are indeed things I'd like to not touch.

[m4rc1e]

+1,000,000

I've been playing a lot with both Go and Rust and their autoformatters just make life a breeze.

Life is too short for us to be caring whether a block of code could look nicer. I'd rather use that time for making cocktails, walking on the beach with my partner or smoking cigars.

[m4rc1e]

Curiosity got the better of me so I had to do it, #3697.

[madig]

FWIW, I also support any decision to unconditionally move to black without any modifications. Hook it up to your editor's Ctrl+S and life is easy.

[miguelsousa]

It can also be run as one of the CI steps on every push or PR. If the submitted changes aren't Black-compliant, the CI fails 😁
https://black.readthedocs.io/en/stable/integrations/github_actions.html

[felipesanches]

Lots of people seem to be frustrated by fontbakery not using Black. I feel guilty of being stubborn on this matter. Let's give this another chance. I'd like to ask here what should I do to experiment with this. How can I see how it would look like? How is it setup? Can someone please, perhaps @m4rc1e, open a PR in which black is used?

[davelab6]

@felipesanches thanks for being open about your self reflection and giving this another go

@madig @yanone would love to hear your thoughts on this

[yanone]

How to set up Black in VSCode:
https://dev.to/adamlombard/how-to-use-the-black-python-code-formatter-in-vscode-3lo0

Additionally, you can configure certain formatting options per repo which VSCode will then respect automatically, the most prominent being line length: https://github.com/typeworld/typeworld/blob/main/pyproject.toml

Sadly you have to set the line length twice so that flake8 can also check for it:
https://github.com/typeworld/typeworld/blob/main/tox.ini#L3

There may be other ways to set all of this up. This has worked for me.

[madig]

You can do pip install black && black . in your venv to see what it would look like. You'd ideally hook it into your editor's Ctrl+S. In various projects I maintain, running black --check . is part of the lint job, see e.g. https://github.com/fonttools/ufoLib2/blob/master/tox.ini.

[felipesanches]

We're currently using Black auto formatter.