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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Totto16
Copy link
Contributor

@Totto16 Totto16 commented Jan 28, 2025

  • allow the formatting provider to be set as null, 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 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

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

@Totto16 Totto16 force-pushed the better_formatter_detection branch from 2e30f7b to 9660035 Compare January 28, 2025 09:13
- allow the formatting provider to be set as null, this means, select the best available one
- add internal priority for formatting providers
@Totto16 Totto16 force-pushed the better_formatter_detection branch 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 chnage 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.

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