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

Add a config option to limit the max amount of characters for a value shown in error messages #110

Merged
merged 11 commits into from
Feb 18, 2025

Conversation

nvnieuwk
Copy link
Collaborator

Fixes #80

This PR adds the validation.maxValueLength option that takes an integer (bigger than or equal to 0) as input and uses this to limit the amount of characters that a value can be in error messages.

Example:
with validation.maxValueLength = 20

* --test (abcdefghij...qrstuvwxyz): Value is [string] but should be [integer]

Where the original value is 26 characters long (abcdefghijklmnopqrstuvwxyz)

The default of this option is 150 which should prevent the loss of most data, but I'm of course open to discuss lowering or raising this default

@mirpedrol
Copy link
Collaborator

Do we have an example for how an error with 150+ characters will look like?

@nvnieuwk
Copy link
Collaborator Author

Do we have an example for how an error with 150+ characters will look like?

You can see an error with a shorter max length in the docs. The value will basically be split up with ... separating the two halves

failUnrecognisedParams = config.failUnrecognisedParams ?: false
failUnrecognisedHeaders = config.failUnrecognisedHeaders ?: false
showHiddenParams = config.showHiddenParams ?: false
maxErrValSize = config.maxErrValSize && config.maxErrValSize >= 1 ? config.maxErrValSize : 150
Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry I had a new idea 😅 What do you think if by setting maxErrValSize to 0 we don't have a maximum?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe -1 might be better for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But I like the idea!

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, -1 sounds better 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks :p It's been implemented

Copy link
Collaborator

@mirpedrol mirpedrol left a comment

Choose a reason for hiding this comment

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

👏 LGTM!

@nvnieuwk nvnieuwk merged commit 36b2938 into 2.4.0dev Feb 18, 2025
4 checks passed
@nvnieuwk nvnieuwk deleted the fix/map-params-error-value branch February 18, 2025 11:03
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