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

Allow specifying multiple values for line_length_guide in config #280

Merged

Conversation

christoph-heiss
Copy link
Contributor

Hi,

long time no see :^)

Anyway, had this branch still laying around from #210, so I thought I'd quickly rebase it before it completely rots away.

Preferences now returns a Vec instead of an Option. As for the configuration itself, it still allows a single value as before, but also an array now. The only change from the previous PR is a change required by the new justify code, but that's pretty trivial.

@jmacdonald
Copy link
Owner

Hey @christoph-heiss! Thanks for submitting this PR. 🍻

The code changes here look great, but can I bother you to update the associated documentation as part of this change, too?

Signed-off-by: Christoph Heiss <christoph@c8h4.io>
Signed-off-by: Christoph Heiss <christoph@c8h4.io>
Signed-off-by: Christoph Heiss <christoph@c8h4.io>
Copy link
Owner

@jmacdonald jmacdonald left a comment

Choose a reason for hiding this comment

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

LGTM! I introduced clippy checks after doing a repo-wide sweep, but lints change over time and ideally those would only be run against the changes in the PR; I'll need to get that sorted. Thanks for making an effort to address those, even if they were adjacent to this work!

@jmacdonald jmacdonald merged commit 9c8ff07 into jmacdonald:main Apr 4, 2024
1 of 2 checks passed
@christoph-heiss christoph-heiss deleted the multiple-line-length-guides branch April 4, 2024 12:58
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.

2 participants