I m working on adding a TOML formatter via a plugin I ve mor Pants #general

I’m working on adding a TOML formatter via a plugi...

future-oxygen-10553

04/03/2023, 8:06 PM

I’m working on adding a TOML formatter via a plugin. I’ve more-or-less followed the instructions in the docs (https://www.pantsbuild.org/v2.16/docs/plugins-fmt-goal) to add a formatting goal, and I also added a new

SourceField

for TOML files. This works great when I have a

toml_source

target. Now, of course, I want to use the

python_requirements

target with a

pyproject.toml

file. Is there a way to configure the

FieldSet

to have a union for the sources, so it pulls both `TomlSourceField`s and `PythonRequirementsSourceField`s? Assume for now that I’m fine with hard crashes if the

PythonRequirementsSourceField

points to a

requirements.txt

-format file 🙂

witty-crayon-22786

04/03/2023, 8:33 PM

not currently i don’t think… but it seems like a very reasonable thing to want to do. i think that it would either be a change to

FieldSet

, or to

FmtRequest

, to match multiple

FieldSet

types?

witty-crayon-22786

04/03/2023, 8:34 PM

this change seems pretty straightforward to motivate… you might consider posting your existing formatter as a draft PR to

pantsbuild/pants

, and then another non-draft PR to make the change that you need to enable the use-case? that will make it easier to discuss in context.

broad-processor-92400

04/03/2023, 8:36 PM

For the particular task here, would it be unreasonable to have the

pyproject.toml

file “touched” by two targets? (Both the Python requirements one and the toml one) As in, avoid the toml plugin needing to think about Python at all

bitter-ability-32190

04/03/2023, 8:38 PM

Could this just be a target less formatter?

witty-crayon-22786

04/03/2023, 8:38 PM

oh. yea.

bitter-ability-32190

04/03/2023, 8:38 PM

I would assume it's relatively safe to assume toml files have a toml suffix

bitter-ability-32190

04/03/2023, 8:38 PM

I think our technological advances have surpassed our docs 😓

wide-midnight-78598

04/04/2023, 12:07 AM

What about prettier + a toml ~~formatter~~ plugin?

future-oxygen-10553

04/04/2023, 1:03 PM

Thanks all for the replies! I’m happy to contribute this formatter back upstream, if it would be helpful. @bitter-ability-32190 can you say more about the target-less formatter?

future-oxygen-10553

04/04/2023, 1:06 PM

@wide-midnight-78598 I’d prefer to use Taplo

👍 1

wide-midnight-78598

04/04/2023, 1:20 PM

Roger - that powers the VSCode plugin I think

future-oxygen-10553

04/04/2023, 1:24 PM

Yep, that’s why I want to use it 😄

bitter-ability-32190

04/04/2023, 2:11 PM

Yeah, I'm OoO this week so I can do a detailed write up next week, if that's stomachable

bitter-ability-32190

04/04/2023, 2:12 PM

Otherwise, look at the

preamble

plugin in-repo to get an idea

future-oxygen-10553

04/04/2023, 2:12 PM

Awesome thanks! I’ll take a look at the plugin, and I’ll reach out next week if needed 😄

❤️ 1

future-oxygen-10553

04/05/2023, 2:48 PM

So I got something working with the target-less formatter, but I had to essentially duplicate the “action” rule because I couldn’t download the external tool in a function outside a rule (or, at least, I don’t know how 😄). That feels suboptimal. Also, I’m not sure how the code is selecting for

pyproject.toml

files. I added a function that I thought would work but deleting that function doesn’t seem to change anything. Finally, I think it’d be beneficial to be able to turn off the formatting per-file similar to black et al. So I think I’m back to the idea that @witty-crayon-22786 shared of being able to have multiple field sets associated with a FmtTargetsRequest. Does that sound right?

bitter-ability-32190

04/05/2023, 2:55 PM

I do think a target less formatter would foot the bill for each of these, sorry we don't have more documentation on one. If you post your code I'd be happy to help you along. Alternatively, I could try in a week or two writing up a targetless tool guide

future-oxygen-10553

04/05/2023, 3:01 PM

🎉 Thanks @bitter-ability-32190! If you’re on vacation for your OOO, please don’t let this interrupt! It can wait until next week 🙂 Nonetheless, I’ll post a GH repo shortly. Thanks again!

future-oxygen-10553

04/21/2023, 10:49 PM

OK, well two weeks later, but I pushed a repo here: https://github.com/bryanwweber/pants-toml-formatter/blob/main/pants-plugins/taplo/taplo_fmt.py#L105-L204 Those lines are the ones I was less than happy with, as they’re more-or-less duplicate. But it’s the only way I could get it to work with both

toml_source

and no-target files.

bitter-ability-32190

04/26/2023, 2:47 PM

You wanna u[stream this? I can comment on PR

future-oxygen-10553

04/26/2023, 2:57 PM

Sure, I’ll try to ticket some time to upstream this

future-oxygen-10553

04/26/2023, 2:57 PM

Thanks for the offer!

bitter-ability-32190

04/26/2023, 2:57 PM

🙏

bitter-ability-32190

04/26/2023, 2:57 PM

The upside is when the Plugin API breaks (as it does very frequently. I'm to blame) the breaker fixes your plugin for you 🙂

bitter-ability-32190

04/26/2023, 2:58 PM

I promise I'll carve out some time to clone your branch and give it a thorough review when it's in PR

future-oxygen-10553

04/26/2023, 3:46 PM

😄 That’s why I upstreamed reading dependencies from

pyproject.toml

🙂

❤️ 1

Open in Slack

Previous Next