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

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

Open
akaihola opened this issue Feb 22, 2022 · 19 comments
Open

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

akaihola opened this issue Feb 22, 2022 · 19 comments
Assignees
Labels
enhancement New feature or request many-formatters Support for multiple code formatters and skipping formatting
Milestone

Comments

@akaihola
Copy link
Owner

akaihola commented Feb 22, 2022

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:

  • Black calls itself a "formatter", so I discarded --reformat and --reformatter (also for needless verbosity)
  • Linters are chosen with --lint, not --linter, so for consistency, I'd prefer --format over --formatter

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

  • pip install 'darker[black]' and darker --black for the current behavior
  • split into two Python packages:
    • move all code from darker to a new package (package name to be decided) which
      • doesn't require black
      • doesn't run black by default (so has --format=none as the default)
    • make darker depend on the new package and black
    • in darker, wrap and command line parser of the new package and override --format by setting --format=black as default

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

  • 🟢👍 --black: can enable multiple formatters (although what about the running order?)
  • 🟢👍 --black: consistent with --isort and --flynt
  • 🟢👍 --black: shorter to type

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

  • 🟢👍 --format=: consistent with --lint=
  • 🔴👎 --format= inconsistent with --lint= since --format=black or --format=blue don't really specify a command line
  • 🔴👎 --format=: more verbose
@akaihola akaihola added the enhancement New feature or request label Feb 22, 2022
@akaihola akaihola added this to the 1.5.0 milestone Feb 22, 2022
@akaihola
Copy link
Owner Author

akaihola commented Apr 1, 2022

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

@akaihola akaihola modified the milestones: 1.5.0, 1.5.1, 1.6.0 Apr 5, 2022
@akaihola akaihola modified the milestones: 1.6.0, 1.6.1 Dec 19, 2022
@akaihola akaihola self-assigned this Dec 25, 2022
@akaihola akaihola modified the milestones: 1.6.1, 1.8.0 Dec 25, 2022
@okuuva
Copy link
Collaborator

okuuva commented Feb 8, 2023

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
Copy link
Owner Author

akaihola commented Feb 8, 2023

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

This comment was marked as resolved.

@akaihola

This comment was marked as resolved.

@okuuva
Copy link
Collaborator

okuuva commented Feb 9, 2023

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 #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.

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

This comment was marked as resolved.

@okuuva

This comment was marked as resolved.

@akaihola

This comment was marked as resolved.

@akaihola

This comment was marked as resolved.

@akaihola

This comment was marked as resolved.

@akaihola

This comment was marked as resolved.

@okuuva

This comment was marked as resolved.

@okuuva

This comment was marked as resolved.

@akaihola

This comment was marked as resolved.

@okuuva

This comment was marked as resolved.

@akaihola
Copy link
Owner Author

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

This comment was marked as resolved.

@akaihola
Copy link
Owner Author

@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.

@akaihola akaihola changed the title Run without Black, i.e. become a generic reformat/lint helper tool Run without Black, i.e. become a generic code fixer tool for touched code Mar 17, 2024
@akaihola akaihola added the many-formatters Support for multiple code formatters and skipping formatting label Mar 17, 2024
@akaihola akaihola changed the title Run without Black, i.e. become a generic code fixer tool for touched code [DESIGN] support different code formatters, allow to skip formatting Mar 17, 2024
@akaihola akaihola moved this from Todo to In Progress in Darker and Graylint development Mar 17, 2024
@akaihola akaihola modified the milestones: Darker 2.1.1, Darker 2.1.2 Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request many-formatters Support for multiple code formatters and skipping formatting
Projects
Development

No branches or pull requests

2 participants