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

Activate prettier-plugin-toml for pre-commit hooks #2053

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Horstage
Copy link

In the pre-commit hook, the plugin prettier-plugin-toml is installed, but not used.
Plugins must be loaded either via CLI command --plugin or in the configuration file.
As there is a configuration file in this project anyways, I added the plugin there.

For more information, see prettier docs.

Contributor checklist
  • Included tests for the changes.
  • PR title is short, clear, and ready to be included in the user-facing changelog.
Maintainer checklist
  • Verified one of these labels is present: backwards incompatible, feature, enhancement, deprecation, bug, dependency, docs or skip-changelog as they determine changelog listing.
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

@webknjaz
Copy link
Member

Honestly, I think that multiline entries are better. Could you reconfigure it with that in mind?

@Horstage
Copy link
Author

I think so too. But sadly prettier-plugin-toml is "still in alpha state".
There are no configuration options, nor the possibilty to add an inline ignore...

Maybe we should open up an issue?

@webknjaz
Copy link
Member

Maybe so. Otherwise, we could also drop it.

Possible replacements and/or additions to evaluate:

These seem closer to the Python ecosystem, anyway.

requires = [
"setuptools==68.1.2",
"wheel==0.41.1",
"fake_static_build_dep"
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if having a trailing comma after the last item here would make the formatter to the right thing and keep the list. Worth checking...

Copy link
Author

Choose a reason for hiding this comment

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

It does not, but that would be lovely.

Copy link
Author

Choose a reason for hiding this comment

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

I added a Feature Request in the plugin project.

@Horstage
Copy link
Author

I created an issue for ignoring lines.

@Horstage
Copy link
Author

I used the workaround described in the issue to at least ignore the one instance, where multiline would be nicer.

@chrysle
Copy link
Contributor

chrysle commented Feb 23, 2024

@Horstage Could you rebase and investigate the pre-commit failure?

@chrysle chrysle added needs rebase Need to rebase or merge awaiting response Awaiting response from a contributor labels Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response Awaiting response from a contributor needs rebase Need to rebase or merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants