Skip to content

Conversation

@horriblename
Copy link
Collaborator

@horriblename horriblename commented Aug 23, 2025

Allow multiple formatters in vim.languages.<lang>.format.type, deprecates use of non list type

Also deprecates non-list values in vim.languages.*.lsp.server

Sanity Checking

  • I have updated the changelog as per my changes
  • I have tested, and self-reviewed my code
  • My changes fit guidelines found in hacking nvf
  • Style and consistency
    • I ran Alejandra to format my code (nix fmt)
    • My code conforms to the editorconfig configuration of the project
    • My changes are consistent with the rest of the codebase
  • If new changes are particularly complex:
    • My code includes comments in particularly complex areas
    • I have added a section in the manual
    • (For breaking changes) I have included a migration guide
  • Package(s) built:
    • .#nix (default package)
    • .#maximal
    • .#docs-html (manual, must build)
    • .#docs-linkcheck (optional, please build if adding links)
  • Tested on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin

Add a 👍 reaction to pull requests you find important.

@github-actions
Copy link

github-actions bot commented Aug 23, 2025

🚀 Live preview deployed from 3e48f13

View it here:

Debug Information

Triggered by: horriblename

HEAD at: python-multi-formatter

Reruns: 1495

@horriblename horriblename force-pushed the python-multi-formatter branch from 1fb7c25 to 61e7c8c Compare August 23, 2025 11:56
@NotAShelf
Copy link
Owner

I think this will be easier if you cover all at once, because we will need to cover it before 0.8 anyway.

@horriblename
Copy link
Collaborator Author

aight bet

@horriblename horriblename force-pushed the python-multi-formatter branch 3 times, most recently from 906a500 to 6697e54 Compare August 23, 2025 18:15
@horriblename horriblename changed the title language/python: allow multiple formatters languages/formatters: allow multiple formatters Aug 23, 2025
@horriblename horriblename force-pushed the v0.8 branch 2 times, most recently from 8b98f07 to ba9ce8b Compare September 13, 2025 07:23
github-actions bot pushed a commit that referenced this pull request Sep 22, 2025
@horriblename horriblename force-pushed the python-multi-formatter branch 2 times, most recently from 163eb0d to 2ca2b56 Compare September 22, 2025 20:44
@horriblename horriblename mentioned this pull request Sep 25, 2025
17 tasks
github-actions bot pushed a commit that referenced this pull request Sep 25, 2025
@horriblename horriblename force-pushed the python-multi-formatter branch from d20abc1 to 393b777 Compare October 13, 2025 16:59
github-actions bot pushed a commit that referenced this pull request Oct 13, 2025
Copy link
Owner

@NotAShelf NotAShelf left a comment

Choose a reason for hiding this comment

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

Thank you for the good work.

I'm afraid I'll have to nit this a little, because I think we're inadvertently introducing some technical debt that will end up biting us in the ass later down the line.

  1. The functions with "deprecated" in their names are bothering me a little. Are we doing to get rid of those? If so, why not do that now since 0.8 is the big bad breaking change.
  2. The removal of "format" sections is not really good practice imo. We should strive to provide predictable and consistent APIs for the users, and I think removing two options does the opposite of contributing to this goal
  3. I left a few comments about unpacked sets, statix warns you about those and I'd rather get rid of them while we're already working on it.

Comment on lines +267 to +272
(mkRemovedOptionModule ["vim" "language" "astro" "format"] ''
This option has been removed due to being broken for a long time.
'')
(mkRemovedOptionModule ["vim" "language" "svelte" "format"] ''
This option has been removed due to being broken for a long time.
'')
Copy link
Owner

Choose a reason for hiding this comment

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

I think removing format options introduces an annoying, backwards-incompatible breakage that the users will not appreciate.

We should instead find a way to minimize the surface, e.g., how much users are affected by the "broken for a long time" section. We could warn them, or we could provide a stub that doesn't format anything but simply warn the user.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm gonna add back biome but I don't consider removing prettier and prettierd a breaking change because that thing never worked, can't break user's workflow when the workflow never worked.

nevertheless, I'll look into packaging prettier-plugin-astro and -svelte, if it's not a pain in the neck I'll add them back in.

fuck all these frameworks holy fuck how hard is it to write your own fucking formatter you already have to parse the whole thing just print it back out nicely goddammit

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added back in biome and prettier, removed prettierd because trying to support it is pure insanity

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for now trying to use prettierd falls back to prettier in astro and svelte

@horriblename horriblename force-pushed the python-multi-formatter branch 2 times, most recently from 580746c to fced08b Compare October 14, 2025 21:31
@horriblename horriblename force-pushed the python-multi-formatter branch from fced08b to 0c0fe24 Compare October 14, 2025 22:02
@horriblename horriblename force-pushed the python-multi-formatter branch 3 times, most recently from 314f578 to 2429336 Compare October 14, 2025 22:21
@horriblename horriblename force-pushed the python-multi-formatter branch from 2429336 to 06aa186 Compare October 14, 2025 22:39
@horriblename
Copy link
Collaborator Author

The functions with "deprecated" in their names are bothering me a little. Are we doing to get rid of those? If so, why not do that now since 0.8 is the big bad breaking change.

yea we're getting rid of them in the future, I just put up a little warning for now if you use the old syntax and convert it for you, just cuz I can and also cuz I hate breaking changes that don't say anything useful in logs/warning

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants