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

Strictly return a word with given length #721

Closed
Shinigami92 opened this issue Mar 29, 2022 · 9 comments
Closed

Strictly return a word with given length #721

Shinigami92 opened this issue Mar 29, 2022 · 9 comments
Assignees
Labels
c: feature Request for new feature p: 1-normal Nothing urgent

Comments

@Shinigami92
Copy link
Member

Clear and concise description of the problem

#714 (comment)

MO it feels a bit weird if a word with given length is not found then use random length 🤔
IMO we should change behavior for all such cases to a min/max length and search between that. If nothing found return undefined or throw an error?

Suggested solution

We need to discuss whether we should return undefined, null or an error in such a case

Alternative

No response

Additional context

No response

@Shinigami92 Shinigami92 added the s: pending triage Pending Triage label Mar 29, 2022
@pkuczynski
Copy link
Member

I think for some values it might be hard to keep the exact length (like when length > 10). So maybe we should simply call this param maxLength?

@Shinigami92
Copy link
Member Author

It's not that simple, because then users would loose current behavior to generate an exact length word
So we need a minLength and a maxLength, but I also think we should do this via an options param, so maybe { minLength, maxLength } or length: { min, max }

@pkuczynski
Copy link
Member

Well, the current behavior is actually maxLength and not length. The param name is misleading :)

@pkuczynski
Copy link
Member

pkuczynski commented Mar 29, 2022

I agree those functions could have 2 signatures:

function xyz (maxLength: number)
function xyz (options: { minLength?: number, maxLength?: number})

@Shinigami92
Copy link
Member Author

Well, the current behavior is actually maxLength and not length. The param name is misleading :)

No it's not. The current behavior is: if length match was found use exact length, if not fallback to random length.

@pkuczynski
Copy link
Member

Which means that current length actually describes maxLength, no? Or at least kind of. More like desiredLength ;)

@ejcheng ejcheng added s: needs decision Needs team/maintainer decision and removed s: pending triage Pending Triage labels Mar 29, 2022
@ST-DDT
Copy link
Member

ST-DDT commented Mar 29, 2022

As the word functions currently use desiredLength we should keep this as the default. Maybe adding other variants as an option.

I suggest this signature
word( length?: number | { min?: number, max?: number, fallbackToAnyLength: boolean = true})

That way it would be non breaking. v6.2 at the earliest.

@ST-DDT ST-DDT added c: feature Request for new feature p: 1-normal Nothing urgent labels Mar 29, 2022
@Shinigami92
Copy link
Member Author

I suggest this signature word( length?: number | { min?: number, max?: number, fallbackToAnyLength: boolean = true})

On the first looks that looks good
We will see if this can successfully apply to everything

@xDivisionByZerox
Copy link
Member

This got solved with #1407.

@xDivisionByZerox xDivisionByZerox removed the s: needs decision Needs team/maintainer decision label Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature p: 1-normal Nothing urgent
Projects
None yet
Development

No branches or pull requests

5 participants