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

feat: add more sophisticated formatter selection #275

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

Totto16
Copy link
Contributor

@Totto16 Totto16 commented Jan 28, 2025

  • allow the formatting provider to be set as auto, this means, select the best available one
  • add internal priority for formatting providers

This adds an algorithm, that searches for the best available formatter. This results in the following behavior:

  • if formatting.provider is set to auto we try to find a formatter in the following order ["meson", "muon"], if one of them is found in this order, that one is used

  • if formatting.provider is set to a specific formatter, it only tries that one and fails, if it didn't find that one

fixes #274

Let me know, me if this solution is acceptable. The problem with the previous code is, that it wasn't really designed to handle multiple formatting providers that well.

@Totto16 Totto16 force-pushed the better_formatter_detection branch 2 times, most recently from 9660035 to a698a77 Compare January 28, 2025 09:16
@tristan957
Copy link
Contributor

  • allow the formatting provider to be set as null, this means, select the best available one

    • add internal priority for formatting providers

Instead of null, can we use "auto"?

This adds an algorithm, that searches for the best available formatter. This results in the following behavior:

* if `formatting.provider` is set to `null` we try to find a formatter in the following order `["meson", "muon"`], if one of them is found in this order, that one is used

* if `formatting.provider` is set to `"meson"` we try to find a formatter in the following order `["meson", "muon"`], even if we set the provider to be `"meson"` and meson is not found, muon is used

This behavior doesn't make much sense to me. If the user requests meson, they should get meson if available, else don't format.

* The same for muon, it is a priority if the user selected it, but meson might be used, if muon is not found

The only problem I can see, if you want to disable the formatter, there is no way of doing that, but that was previously also not the case.

fixes #274

Let me know, me if this solution is acceptable. The problem with the previous code is, that it wasn't really designed to handle multiple formatting providers that well.

Let me know your thoughts!

@Totto16
Copy link
Contributor Author

Totto16 commented Jan 29, 2025

  • allow the formatting provider to be set as null, this means, select the best available one

    • add internal priority for formatting providers

Instead of null, can we use "auto"?

Yes that is also a good idea 👍🏼

This adds an algorithm, that searches for the best available formatter. This results in the following behavior:

* if `formatting.provider` is set to `null` we try to find a formatter in the following order `["meson", "muon"`], if one of them is found in this order, that one is used

* if `formatting.provider` is set to `"meson"` we try to find a formatter in the following order `["meson", "muon"`], even if we set the provider to be `"meson"` and meson is not found, muon is used

This behavior doesn't make much sense to me. If the user requests meson, they should get meson if available, else don't format.

Ok, So I'll change it, to just do that on the "auto" value 👍🏼

* The same for muon, it is a priority if the user selected it, but meson might be used, if muon is not found

The only problem I can see, if you want to disable the formatter, there is no way of doing that, but that was previously also not the case.
fixes #274
Let me know, me if this solution is acceptable. The problem with the previous code is, that it wasn't really designed to handle multiple formatting providers that well.

Let me know your thoughts!

@tristan957
Copy link
Contributor

Thanks for your contributions to the extension. I'll try and a get a release out after this PR merges.

@Totto16 Totto16 force-pushed the better_formatter_detection branch from a698a77 to 3b762c8 Compare January 31, 2025 19:45
@Totto16
Copy link
Contributor Author

Totto16 commented Jan 31, 2025

I implemented the suggested changes 😄

package.json Outdated Show resolved Hide resolved
src/formatters.ts Outdated Show resolved Hide resolved
@Totto16 Totto16 force-pushed the better_formatter_detection branch from af974c1 to 32ea852 Compare February 1, 2025 17:42
@tristan957
Copy link
Contributor

If you squash your commits, I'll go ahead and merge this.

- allow the formatting provider to be set as auto, this means, select the best available one
- add internal priority for formatting providers
@Totto16 Totto16 force-pushed the better_formatter_detection branch from 32ea852 to fbdbb97 Compare February 4, 2025 09:49
@tristan957 tristan957 added this pull request to the merge queue Feb 5, 2025
Merged via the queue into mesonbuild:main with commit fbdbb97 Feb 5, 2025
6 checks passed
@Totto16 Totto16 deleted the better_formatter_detection branch February 5, 2025 22:12
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.

Change the default formatter to the official one
2 participants