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

Improve specification of lorem module and definitions #2884

Open
ST-DDT opened this issue May 8, 2024 · 5 comments · May be fixed by #2885
Open

Improve specification of lorem module and definitions #2884

ST-DDT opened this issue May 8, 2024 · 5 comments · May be fixed by #2885
Assignees
Labels
c: docs Improvements or additions to documentation c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs m: lorem Something is referring to the lorem module p: 1-normal Nothing urgent s: needs decision Needs team/maintainer decision
Milestone

Comments

@ST-DDT
Copy link
Member

ST-DDT commented May 8, 2024

Languages that use latin characters should use the latin lorem text (e.g. copy it from en).
We will create a separate PR that will document this behavior of the lorem feature in the definitions and module.
After the change of definitions we should create another PR that actually changes the locale data to comply with the new spec and additional care should be taken regarding tree shakeability if the locale data are imported from other locales.

Especially the later part should be done by the team to ensure there are no side effects.

Originally posted by @ST-DDT in #2824 (comment)

@ST-DDT ST-DDT added c: docs Improvements or additions to documentation p: 1-normal Nothing urgent c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs m: lorem Something is referring to the lorem module labels May 8, 2024
@ST-DDT ST-DDT added this to the v9.0 milestone May 8, 2024
@ST-DDT
Copy link
Member Author

ST-DDT commented May 8, 2024

Suggestion by @matthewmayer

If it is identical to en could we not just have words.ts be like this to avoid duplication:

import words from '../../en/lorem/words';
export default words;

@xDivisionByZerox
Copy link
Member

I'll create a PR for the documentation part. Unless you are already working on it.

@xDivisionByZerox xDivisionByZerox self-assigned this May 8, 2024
@xDivisionByZerox xDivisionByZerox linked a pull request May 8, 2024 that will close this issue
@xDivisionByZerox xDivisionByZerox linked a pull request May 8, 2024 that will close this issue
@ST-DDT
Copy link
Member Author

ST-DDT commented May 19, 2024

#2907 (comment)

This raises my impression that lorem should be strictly Latin only and we should probably add gibberishSentence/Paragraph/Text methods to the word module instead.

@xDivisionByZerox
Copy link
Member

Honestly, as long as we have a clear definition I'm fine with it. It's not like we can't change it in the future, if required.

@ST-DDT ST-DDT added the s: needs decision Needs team/maintainer decision label May 19, 2024
@matthewmayer
Copy link
Contributor

If we do decide to to keep lorem non-Latin languages we should definitely allow for languages that don't have a space as a word seperator (e.g. Chinese, Thai). For example here you'd probably want to use a definition like faker.definitions.lorem.word_seperator instead of a hardcoded ' '.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: docs Improvements or additions to documentation c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs m: lorem Something is referring to the lorem module p: 1-normal Nothing urgent s: needs decision Needs team/maintainer decision
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants