search

akaihola / darker

Apply black reformatting to Python files only in regions changed since a given commit. For a practical usage example, see the blog post at https://dev.to/akaihola/improving-python-code-incrementally-3f7a
https://pypi.org/project/darker/
Other
628 stars 56 forks source link

[DESIGN] support different code formatters, allow to skip formatting #304

Open akaihola opened 2 years ago

akaihola commented 2 years ago

See also: many-formatters sub-project

Currently there's no way to run Darker and have it just do, say, isort and flynt but not reformat code using Black.

Also, there are open requests for replacing Black with another formatter or expanding the choices of formatters/linters:

We could expand Darker's behavior in stages:

For the option name, I've considered --format, --formatter, --reformat and --reformatter:

At some point after making --format=none the default, black should be removed as a hard dependency. I can see two alternatives:

For the --black/--no-black/--blue/--no-blue design choice, some pros and cons are:

and for --format={black|blue|none}:

akaihola commented 2 years ago

FYI @Carreau, @guettli & @jedie, based on earlier discussions, you may be interested in this feature idea for Darker.

okuuva commented 1 year ago

I don't know if you're interested in a random person's input on this (and wouldn't blame you if you didn't) but I thought I'd share my 2c anyway.

Having an ability to run just linters with darker would be awesome. There are no easy ways to run linters for just changed parts of Python code afaik. Running linters to changed files is (at least a bit) easier and less hacky but then you end up cleaning up other people's messes. Which is good for the project but sometimes you just wouldn't have time for that and you almost always have better things to do anyway.

That being said, I'd put that functionality into a separate tool and make darker depend on it. The main reason (and all the other points I can think of right now relate to it) being that the name darker doesn't make sense anymore if it doesn't run black. Let's say the name stays. It would make finding the awesome tool that lets you run linters on changed parts of the code very hard since the name wouldn't be very descriptive (right there with bob). And the pun would be lost which would be devastating ;)

The other thing is that darker is an established tool for use cases where you want to use black but really can't. Sure, there's black-machiato and pyink (and in the future, maybe even black itself), but right now darker is by far the easiest to integrate into IDEs and editors. Changing its behaviour from an executor for black to a generic tool would be confusing IMO.

I also think there could be more demand for a tool that runs formatters/linters/younameits for the changed parts of code than there are for darker which is, let's face it, a tool that fulfills a very narrow niche in the market. Don't get me wrong, I do love it. But a more generic tool could (at least in theory) be language agnostic.

So I would go with the separate package for it. Then at some point darker might devolve back into being a very thin wrapper around it that just runs black (or blue) and nothing else since all the other functionality could be included in the new tool.

akaihola commented 1 year ago

I don't know if you're interested in a random person's input

I certainly am!

There are no easy ways to run linters for just changed parts of Python code afaik.

Note that after #393, darker version 1.7.0 will always run linters for the whole codebase, and twice – a baseline run for the old version of the code (e.g. main branch) and another run for the current version (e.g. the feature branch). It will then display those linter messages falling on modified lines, but also only those introduced elsewhere by the modifications between the old and current versions. This is actually the only correct way to run linters, since typically changes in one part of the code introduce linter errors in other parts (or even in different files). For speeding up repeated runs, #443 outlines caching for the baseline run.

That being said, I'd put that functionality into a separate tool and make darker depend on it.

So effectively extracting linter support from darker into a new incremental-py-linter package and calling that one from Darker when the -L/--lint option is used?

The main reason (and all the other points I can think of right now relate to it) being that the name darker doesn't make sense anymore if it doesn't run black.

Thanks for raising that point. I've been pondering the "two separate packages" option, too, but so far ended up only considering a single code base for ease of maintenance.

One detail to consider is that after splitting incremental-py-linter from darker, both of the packages will need a number of helpers (e.g. Git related) which currenly reside in darker. Would they then be moved into a) incremental-py-linter, or b) into a third incremental-utils package upon which both of the new packages would depend?

Changing its behaviour from an executor for black to a generic tool would be confusing IMO.

In addition to linting without running black, some users actually may want to only run isort, or flynt (support coming up in 1.7.0), or maybe pyupgrade (also planned for a release in the near future) on modified code. The same name confusion would ensue, wouldn't it?

What about then moving everything from darker to a generic incremental-py-assistant package for running tools on modified parts of code? incremental-py-assistant wouldn't default to running black, so each tool it runs would be activated using an option (--black, --isort, --flynt, --pyupgrade, --lint <cmd>). darker would turn into a thin wrapper around the incremental-py-assistant, with --black being enabled by default. And if we want, we could make incremental-py-linter just another thin wrapper for running linters only.

What do you think?

But a more generic tool could (at least in theory) be language agnostic.

That's another interesting idea which I have considered before. For that, it would be good to generalize the design a bit. Currently the order of running different tools and the way (and whether) e.g. AST verification is done is custom coded for the set of tools supported.

So ultimately there could be a incremental-code-assistant package with plugin support, and the Python related tools would be plugins among other plugins for other languages. This would require some thought and design so the technical nuances could be implemented on the plugin side.

I'd love to iron out the long-term plan in collaboration with you and/or whoever is interested in the topic. I've also hosted video call meetings about darker in the past, so we could also consider reviving that practice. Or even meet IRL if you ever roam around in the southern part of our country.

akaihola commented 1 year ago

I actually now thought of some possible names for a more generic tool in darker's category (all of these are available on PyPI):

akaihola commented 1 year ago

Or, staying in the world of colors:

okuuva commented 1 year ago

I don't know if you're interested in a random person's input

I certainly am!

Glad to hear it :)

There are no easy ways to run linters for just changed parts of Python code afaik.

Note that after https://github.com/akaihola/darker/pull/393, darker version 1.7.0 will always run linters for the whole codebase, and twice – a baseline run for the old version of the code (e.g. main branch) and another run for the current version (e.g. the feature branch). It will then display those linter messages falling on modified lines, but also only those introduced elsewhere by the modifications between the old and current versions. This is actually the only correct way to run linters, since typically changes in one part of the code introduce linter errors in other parts (or even in different files). For speeding up repeated runs, https://github.com/akaihola/darker/issues/443 outlines caching for the baseline run.

Ok, that makes perfect sense.

That being said, I'd put that functionality into a separate tool and make darker depend on it.

So effectively extracting linter support from darker into a new incremental-py-linter package and calling that one from Darker when the -L/--lint option is used?

Yeah, although I think in the long run darker could drop the linter support completely if the functionality is extracted into a separate tool.

The main reason (and all the other points I can think of right now relate to it) being that the name darker doesn't make sense anymore if it doesn't run black.

Thanks for raising that point. I've been pondering the "two separate packages" option, too, but so far ended up only considering a single code base for ease of maintenance.

One detail to consider is that after splitting incremental-py-linter from darker, both of the packages will need a number of helpers (e.g. Git related) which currenly reside in darker. Would they then be moved into a) incremental-py-linter, or b) into a third incremental-utils package upon which both of the new packages would depend?

I honestly don't know. With option a) changes in the helper parts wouldn't require updating three different packages (utils, linter and darker). But then again, I'm not familiar enough with the code base to say how often those "utils" parts change. If they're stable enough already then splitting them into a separate package makes sense. If the linter logic is moving into a separate package then darker could be considered "ready" and the active development would happen in the linter package and occasionally in the utils.

Changing its behaviour from an executor for black to a generic tool would be confusing IMO.

In addition to linting without running black, some users actually may want to only run isort, or flynt (support coming up in 1.7.0), or maybe pyupgrade (also planned for a release in the near future) on modified code. The same name confusion would ensue, wouldn't it?

Yes it would.

What about then moving everything from darker to a generic incremental-py-assistant package for running tools on modified parts of code? incremental-py-assistant wouldn't default to running black, so each tool it runs would be activated using an option (--black, --isort, --flynt, --pyupgrade, --lint ). darker would turn into a thin wrapper around the incremental-py-assistant, with --black being enabled by default. And if we want, we could make incremental-py-linter just another thin wrapper for running linters only.

What do you think?

Sounds like a plan to me :)

But a more generic tool could (at least in theory) be language agnostic.

That's another interesting idea which I have considered before. For that, it would be good to generalize the design a bit. Currently the order of running different tools and the way (and whether) e.g. AST verification is done is custom coded for the set of tools supported.

So ultimately there could be a incremental-code-assistant package with plugin support, and the Python related tools would be plugins among other plugins for other languages. This would require some thought and design so the technical nuances could be implemented on the plugin side.

That would be super cool and something I think is worth pursuing. But for the time being I think the focus should be in the Python tooling and just keep it in mind and trying not to hard code language specific stuff.

I'd love to iron out the long-term plan in collaboration with you and/or whoever is interested in the topic. I've also hosted video call meetings about darker in the past, so we could also consider reviving that practice. Or even meet IRL if you ever roam around in the southern part of our country.

Sounds good to me! Telcos sound like a good idea, so does IRL meeting should we ever happen to be located in near(ish) proximity :)

I'll address the naming stuff in another comment, this one's getting long as it is.

okuuva commented 1 year ago

I actually now thought of some possible names for a more generic tool in darker's category (all of these are available on PyPI):

  • cwic
    • acronym from "Check What I Changed"
  • cwac
    • acronym from "Check What All Changed"
  • ocwic
    • "Only Consider What Is Changed"
  • cowic
    • "Consider Only What Is Changed"
  • ocwim
    • "Only Consider What Is Modified"
  • cowim
    • "Consider Only What Is Modified"

I'd personally like something that's easy to pronounce, and wouldn't sound too much like Covid :sweat-smile: Maybe something related to automatic reviewing? Like you said, it checks what changed which is what reviewing code is mostly about. It effectively checks the "boring" (i.e. repetitive and error prone) parts and lets humans focus on "why" and "how". Then again, that's the point of CI in general so ¯\_(ツ)_/¯

Or, staying in the world of colors:

  • nurie
    • Japanese for "coloring book"
    • idea: linters and reformatters "color" the code, but we're only allowing them to do it in a limited way
  • arcus
    • latin for "rainbow"
    • different linters and reformatters apply their own "colors" to the code
    • we want to do the "coloring" in an orderly fashion, like in a rainbow
    • but, taken in PyPI

I do like the idea of staying with the color theme because the origins of the name darker and the fact that the tool stems from it. I also think the "linters and reformatters are adding 'color' to code" is an apt analogy.

Both "rainbow" and "coloring book" would be fitting for the reasons you stated above. I wonder what those are in Esperanto? I think using a name derived from it would play nicely with the idea of maybe making the tool language agnostic sometime in the future.

EDIT: checked them. "rainbow" is "ĉielarko" and "coloring book" is "kolorlibro". "ĉ" as part of the name might not be a good idea but even if we'd spell it with "c" both names are available in PyPI.

okuuva commented 1 year ago

What about "palette" or "crayons"? The tool lets you arrange and use the "colors" you want. Both are unsurprisingly taken in PyPI but continuing on the Esperanto theme both names "paletro" and "krajonoj" are available. The latter doesn't really roll of your tongue though...

akaihola commented 1 year ago

In the spirit of bikeshedding, the name is the easiest and most stimulating aspect to think about... so I was thinking, while reformatters "color" the code in the sense that they do actual modifications, linters on the other hand just look at the existing colors and point out if they could be improved. Could we find inspiration from there?

"Color consultant" is one specialization in interior design. It's "kolorkonsilisto" in Esperanto, so "Kolorkon" or "Kolkon" ("Colcon" is taken)?

"Paint" is "farbo" in Esperanto (available on PyPI).

The mantis shrimp (Stomatopod, in Esperanto "manto salikoko") has better color vision than any other known species. There we would have a mascot as well... According to Wikipedia, they are called "sea locusts" by ancient Assyrians, "prawn killers" in Australia, and now sometimes referred to as "thumb splitters".

in Japan, a mantis shrimp species called the shako (available on PyPI) is used for sushi.

In Mediterranean countries, another species called the Squilla mantis (squilla available on PyPI), known as canocchia on Adriatic coasts and galera (hm, a MariaDB replication system) around the Gulf of Cádiz.

And, haha,

In the[Philippines, the mantis shrimp is known as tatampal, hipong-dapa, pitik-pitik, or alupihang-dagat,

Curiously, "the peacock mantis is especially colourful" as well.

I guess from this list the following stand out: farbo, shako, squilla and galera.

akaihola commented 1 year ago

Of course, the excellent color vision of the mantis shrimp has been debunked in an article in the Nature journal. But *shrug*, they still have 12 different color receptors as opposed to the 3 us humans have, so good enough for our purposes I guess...

akaihola commented 1 year ago

darker could drop the linter support completely if the functionality is extracted into a separate tool.

The more I think about that the more I like it. And it clearly points to the direction of three (at least) separate packages.

One crazy solution is for all of this to live in one repository, but to publish two (or more) Python packages from it, with different subsets of the code base included.

But perhaps it's just more understandable to separate repositories as well.

I would then create the linter-tool and utils packages, and start moving over code from Darker to those two with the goal of moving linter functionality over and not duplicating code between the packages.

akaihola commented 1 year ago

Or, what about greylint or graylint for the name of the linter review tool? Or maybe, considering that the tool focuses on newly introduced linter messages, newlint, neolint or nulint?

Including those, my candidate list would be farbo, shako, squilla, galera, graylint, greylint, neolint and nulint.

okuuva commented 1 year ago

Mantis shrimps are cool indeed and would make a badass mascot. From the three names for them there shako is probably my favourite.

gr[ae]ylint would also be good, depicting all the different shades of grey the different linters represent. Stuff ain't black and white. And now that I wrote it in the regex format maybe that spelling would work? Depicting the choises you have between n+1 linters.

I kinda like neolint as well but it might get associated with neovim instead of a standalone tool. And nulint brings numetal into mind which can be a very good or extremely bad depending on your standing. But maybe that's just me.

One more suggestion: prismo which is "prism" in Esperanto. Idea being that it brings all the different linters (colors) together. Then again white would work with the same logic but that would then sound like a code obfuscator as opposed to black 😅

I personally like graylint and greylint the best. And I'm also partial for the graeylint spelling because I really like the aesthetic of gr[ae]ylint. "gr[ae]ylint lets you pick from your favourite shades of gray and combine them into a new and unique shade of grey". Or something like that.

okuuva commented 1 year ago

And in case you find gr[ae]ylint spelling nice: what about gr[ae]yhound? "Hunts down linter errors from your changes at the speed of a greyhound". As an added bonus, it would have a pun in Finnish (linttikoira). Traditional spelling for it might be too generic though, not to mention taken in PyPI.

akaihola commented 1 year ago

Top candidates for linter name currently: shako, prismo, graylint, greylint, graeylint, grayhound, graeyhound

Other ones we haven't yet rejected: farbo, squilla, galera

I'm leaning towards graylint, with this spelling due to other similarly named existing tools in the industry (like Graylog). The [ae] variants graeylint and graeyhound are cool, but aren't they a bit of a nuisance for users to learn to spell? The [æ] might make a good logo though...

Could a supporting common code library for darker and graylint be called libdarkgray or darkgraylib?

I created these issues for the steps to make this idea happen:

okuuva commented 1 year ago

I'm leaning towards graylint, with this spelling due to other similarly named existing tools in the industry (like Graylog).

If we'll go with traditional spelling, this would be my choise too.

The [ae] variants graeylint and graeyhound are cool, but aren't they a bit of a nuisance for users to learn to spell?

That's true. The more I look at these without the brackets surrounding ae the more I dislike them. We could mitigate it by linking gr[ae]y(?:lint|hound) to the graey(?:lint|hound) executable but the tool would still be harder to find with a search engine than a traditional spelling and wouldn't help people launching tools with python -m graey(?:lint|hound). Oh well.

The [æ] might make a good logo though...

Hmm, hadn't thought of that but it does look nice. Would it make sense with the traditional spelling? Even if the linking trick wouldn't help when launching tools with python -m it would still highlight the number of choises between n+1 different linters out there and could be seen as a friendly jab at the "Any color you want" catchphrase of black.

Could a supporting common code library for darker and graylint be called libdarkgray or darkgraylib?

Uu, that's a great idea! darkgraylib sounds real nice but either works for me.

akaihola commented 1 year ago

Hey, thinking about possible logos led me to a feature idea! We could think of graylint as a tool that "grays out" linter messages we're not interested in. A logo could show multiple lines of text in different colors, with a lens that turns all but a few of them gray.

Well, why couldn't graylint have a mode in which, instead of completely hiding previously existing linter messages for unmodified lines, it would still show them, but in a gray color, while the relevant linter messages would be color highlighted, or at least displayed in a more prominent color?

As for the python -m problem, it's easily solved by having graylint not only install a graylint package, but also greylint and graeylint, which would both have their own __main__.py. But if we were to go that route, I would actually register all three packages with PyPI and have the two latter two install graylint as a dependency as well. Registering greylint and pointing to graylint in the README would be a friendly thing to do in any case, I think, but maybe graeylint would be too "smart" a move to do, if you know what I mean...

darkgraylib would leave darkgray up for grabs on PyPI. But I can't think of a good use for that name right now.

okuuva commented 1 year ago

Hey, thinking about possible logos led me to a feature idea! We could think of graylint as a tool that "grays out" linter messages we're not interested in. A logo could show multiple lines of text in different colors, with a lens that turns all but a few of them gray.

Sounds cool!

Well, why couldn't graylint have a mode in which, instead of completely hiding previously existing linter messages for unmodified lines, it would still show them, but in a gray color, while the relevant linter messages would be color highlighted, or at least displayed in a more prominent color?

This sounds very cool! I'd make it configurable though, so that they still could be hidden if so desired.

As for the python -m problem, it's easily solved by having graylint not only install a graylint package, but also greylint and graeylint, which would both have their own __main__.py. But if we were to go that route, I would actually register all three packages with PyPI and have the two latter two install graylint as a dependency as well.

Sounds good :+1:

Registering greylint and pointing to graylint in the README would be a friendly thing to do in any case, I think, but maybe graeylint would be too "smart" a move to do, if you know what I mean...

Yeah, I thinkgraeylint is a neat idea but it's not practical in any way. And it's more important to be practical than witty, at least in this case.

darkgraylib would leave darkgray up for grabs on PyPI. But I can't think of a good use for that name right now.

Whether it's libdarkgray or darkgraylib I think we should register darkgray as well and point it to the lib package so nobody else gets any ideas.

akaihola commented 6 months ago

@okuuva FYI the first step for reformatting and linting separation has now been done! Darker 2.0.0 now depends on Darkgraylib for common code between reformatting and linting, and on Graylint for the code for linting. Graylint has a command line interface for linting, but for now Darker's -L/--lint option remains as well.