feat(lorem): add character length parameter to lorem.text #2227

ghost · 2023-06-27T19:23:46Z

Adds a character length parameter to faker.lorem.text as described in #1546.

Changes made:

~~Added a new parameter length?: number | { min: number, max: number } such that lorem.text returns text that has a particular length if the parameter is passed~~
Added a new optional options object {length: number | { min: number, max: number }} such that lorem.text returns text that has a particular length if the parameter is passed
Added a new optional quick primitive argument such that lorem.text returns text that the length passed as argument
Added random seeded tests for the new parameter to lorem.spec.ts and updated its snapshot

Closes #1546

codecov · 2023-06-27T19:31:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.60%. Comparing base (604d52d) to head (742e794).
Report is 324 commits behind head on next.

Additional details and impacted files

@@           Coverage Diff           @@
##             next    #2227   +/-   ##
=======================================
  Coverage   99.60%   99.60%           
=======================================
  Files        2784     2784           
  Lines      252492   252537   +45     
  Branches     1082     1085    +3     
=======================================
+ Hits       251496   251543   +47     
+ Misses        969      967    -2     
  Partials       27       27

Files	Coverage Δ
src/modules/lorem/index.ts	`100.00% <100.00%> (ø)`

... and 2 files with indirect coverage changes

test/__snapshots__/lorem.spec.ts.snap

src/modules/lorem/index.ts

test/lorem.spec.ts

…g an error

src/modules/lorem/index.ts

… empty string

…and max instead of being an object with all three properties

test/lorem.spec.ts

src/modules/lorem/index.ts

…r for negative min

…arameter_to_lorem.text

outdated

CONTRIBUTING.md

src/modules/lorem/index.ts

matthewmayer

I think the description of text() in the overview could also be modified to reflect this can be used to produce text of a certain length

The generic [`text()`](https://fakerjs.dev/api/lorem.html#text) method can be used to generate some text between one sentence and multiple paragraphs

matthewmayer · 2023-07-19T14:49:39Z

The behavior of the function when the length parameter is supplied is quite different to the version without.

I'm wondering if therefore it would make sense to just make it a separate method, like:

* faker.lorem.characters(14) // 'Doloribus aut.' (14 characters)
* faker.lorem.characters({length: 14}) // 'Doloribus aut.' (14 characters)
* faker.lorem.characters({length: {min: 10, max: 15}}) // 'autem non quis.' (15 characters)
* faker.lorem.characters({length: {min: 10, max: 12}}) // 'autem non.' (10 characters)

…e js-doc for lorem.text

…_lorem.text' of github.com:inkedtree/faker into feat/lorem/issue-1546/add_character_length_parameter_to_lorem.text

src/modules/lorem/index.ts

ST-DDT

Also, should we stick with sentence() (if length is provided) to produce a more consistent text?
(We end with a dot, so it would make sense to have dots in between as well.)

src/modules/lorem/index.ts

test/modules/lorem.spec.ts

ghost · 2023-08-20T16:26:32Z

Yes, it would make sense to put dots in between as well.

src/modules/lorem/index.ts

test/modules/lorem.spec.ts

ST-DDT · 2023-08-24T16:02:57Z

@inkedtree We kind of are unsure what requirements we have for the method/the parameter. And would like to hear your requirements for the method.
This method generates vastly different types of text (some with newlines, some without).
So do we need the length parameter for the specific versions as well? Such as sentences, lines?
Or should we limit the text method to sentences?
Would it get confusing if we have lines( { length: number}) aka does the length refer to each line or to the entire string?

ghost · 2023-08-31T19:57:34Z

@ST-DDT

Sorry, for the late reply. I am not sure, if I understand all your questions correctly.

To be honest, I am not entirely sure what my requirements are after reading about all available methods in the lorem module. Originally, I wanted to have a method that generates sentences with a particular length. So, basically a paragraph, I guess. But now I am not sure, if it wouldn't be better to add the parameter to paragraph(s) for example in this case.

Sometimes when I am using faker I would like to be able to generate sentences, lines and so on with a specific length (range).

I think, it would get confusing if the length parameter does not refer to the length of the entire string, e.g. of lines.

…arameter_to_lorem.text

ejcheng · 2024-03-28T17:45:09Z

src/modules/lorem/index.ts

+            | number
+            | {
+                /**
+                 * The minimum length of text to generate.


Suggested change

* The minimum length of text to generate.

* The minimum amount of characters to generate.

ejcheng · 2024-03-28T17:45:21Z

src/modules/lorem/index.ts

+                 */
+                min: number;
+                /**
+                 * The maximum length of text to generate.


Suggested change

* The maximum length of text to generate.

* The maximum amount of characters to generate.

ejcheng · 2024-03-28T17:47:01Z

src/modules/lorem/index.ts

@@ -310,6 +310,9 @@ export class LoremModule {
  /**
   * Generates a random text based on a random lorem method.
   *
+   * @param options The options for the text to generate or options.length as quick primitive argument.
+   * @param options.length The length of text to generate as number or range.


Suggested change

* @param options.length The length of text to generate as number or range.

* @param options.length The length of text (in characters) to generate as number or range.

ST-DDT · 2024-03-28T17:50:16Z

Team Decision

We accept this feature request. Please rebase it and work on the suggestions we point out.

ST-DDT · 2024-03-28T17:56:01Z

src/modules/lorem/index.ts

+    if (options.length == null) {
+      return `${this[method]()}`;
+    }
+
+    const length = this.faker.helpers.rangeToNumber(options.length);


Please use our standard way of decstructuring options:

Suggested change

if (options.length == null) {

return `${this[method]()}`;

}

const length = this.faker.helpers.rangeToNumber(options.length);

const { length: lengthOrRange } = options;

if (lengthOrRange == null) {

return `${this[method]()}`;

}

const length = this.faker.helpers.rangeToNumber(lengthOrRange);

ST-DDT · 2024-03-28T17:58:49Z

src/modules/lorem/index.ts

+
+    let text = '';
+    do {
+      text = `${text}${this[method]()} `;


Please only use this.sentence() here and maybe randomly (e.g. faker.datatype.boolean()) add a newline to simulate paragraphs/lines.

ST-DDT · 2024-03-28T17:59:23Z

src/modules/lorem/index.ts

+      text = `${text}${this[method]()} `;
+    } while (text.length < length);
+
+    return `${text.substring(0, length).replace(/.$/, '.')}`;


Please just return the value without dot at the end.

Suggested change

return `${text.substring(0, length).replace(/.$/, '.')}`;

return text.substring(0, length);

ST-DDT · 2024-03-28T18:00:12Z

test/modules/lorem.spec.ts

+        it('should return a dot for length one', () => {
+          const actual = faker.lorem.text({ length: 1 });
+
+          expect(actual).toBeTypeOf('string');
+          expect(actual).toBe('.');
+        });


This test is no longer needed,

ST-DDT · 2024-04-25T16:52:33Z

Closed as stale. We can reopen this when you want to continue with it.

bmulholland · 2024-09-09T11:27:33Z

I just wrote a custom function for this and would love to have it baked into faker. It looks like the original submitter deleted their Github account. I'd be open to addressing the comments and submitting a fresh PR. Would that be accepted, @ST-DDT ?

xDivisionByZerox · 2024-09-09T11:40:25Z

I just wrote a custom function for this and would love to have it baked into faker. It looks like the original submitter deleted their Github account. I'd be open to addressing the comments and submitting a fresh PR. Would that be accepted, @ST-DDT ?

@bmulholland feel free to open another PR. We always love when the community gets involved. Make sure to read our contribution guide before hand and like the original issue.

add character length parameter to lorem.text

8026316

ghost self-requested a review as a code owner June 27, 2023 19:23

ST-DDT reviewed Jun 28, 2023

View reviewed changes

test/__snapshots__/lorem.spec.ts.snap Outdated Show resolved Hide resolved

matthewmayer requested changes Jun 28, 2023

View reviewed changes

src/modules/lorem/index.ts Outdated Show resolved Hide resolved

test/lorem.spec.ts Outdated Show resolved Hide resolved

test/lorem.spec.ts Outdated Show resolved Hide resolved

test/lorem.spec.ts Outdated Show resolved Hide resolved

Alexander added 5 commits June 28, 2023 23:41

resolve issues noted in the review of ST-DDT and matthewmayer

8d1ddd9

fix example in comment of lorem.text

30e73fb

return an empty string for an ill defined interval instead of throwin…

65f7d5d

…g an error

fix typo

8f5ba98

add quick primitive argument to lorem.text

ac91a62

ghost self-requested a review June 29, 2023 19:11

matthewmayer requested changes Jun 30, 2023

View reviewed changes

src/modules/lorem/index.ts Outdated Show resolved Hide resolved

Alexander added 2 commits June 30, 2023 18:50

it throws an error if min is greater than max instead of returning an…

425ad90

… empty string

the length parameter in lorem.text is a number or an object with min …

7cb71ed

…and max instead of being an object with all three properties

matthewmayer previously requested changes Jun 30, 2023

View reviewed changes

test/lorem.spec.ts Outdated Show resolved Hide resolved

test/lorem.spec.ts Outdated Show resolved Hide resolved

src/modules/lorem/index.ts Outdated Show resolved Hide resolved

Alexander and others added 2 commits June 30, 2023 20:54

corrected descriptions in lorem.spec.ts and lorem.text throws an erro…

4fad2bf

…r for negative min

Merge branch 'next' into feat/lorem/issue-1546/add_character_length_p…

a2f6bbd

…arameter_to_lorem.text

xDivisionByZerox added c: feature Request for new feature needs rebase There is a merge conflict m: lorem Something is referring to the lorem module labels Jul 17, 2023

xDivisionByZerox assigned ghost Jul 17, 2023

xDivisionByZerox added p: 1-normal Nothing urgent c: locale Permutes locale definitions labels Jul 17, 2023

xDivisionByZerox reviewed Jul 17, 2023

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

src/modules/lorem/index.ts Outdated Show resolved Hide resolved

matthewmayer reviewed Jul 17, 2023

View reviewed changes

ghost mentioned this pull request Jul 19, 2023

docs: fix typo in CONTRIBUTING.md #2264

Merged

Alexander added 2 commits July 19, 2023 20:46

remove explicit character information and repetitive examples from th…

fa121ee

…e js-doc for lorem.text

Merge branch 'feat/lorem/issue-1546/add_character_length_parameter_to…

33d0f37

…_lorem.text' of github.com:inkedtree/faker into feat/lorem/issue-1546/add_character_length_parameter_to_lorem.text

ST-DDT reviewed Aug 20, 2023

View reviewed changes

src/modules/lorem/index.ts Outdated Show resolved Hide resolved

src/modules/lorem/index.ts Show resolved Hide resolved

feat(lorem): improve jsdoc and simplify code of lorem.text()

15a8f93

ST-DDT reviewed Aug 20, 2023

View reviewed changes

src/modules/lorem/index.ts Outdated Show resolved Hide resolved

src/modules/lorem/index.ts Outdated Show resolved Hide resolved

src/modules/lorem/index.ts Outdated Show resolved Hide resolved

test/modules/lorem.spec.ts Outdated Show resolved Hide resolved

Alex added 4 commits August 20, 2023 18:12

feat(lorem): remove unnecessary checks in text()

402802d

feat(lorem): remove redundant tests

153dcfa

feat(lorem): remove tests for checks that were removed

056886a

feat(lorem): simplify code

1fbb6da

ST-DDT reviewed Aug 21, 2023

View reviewed changes

Alex and others added 6 commits September 4, 2023 20:11

chore(lorem): simplify code and remove unnecessary tests

05fc62b

Merge branch 'next' into feat/lorem/issue-1546/add_character_length_p…

1774fc5

…arameter_to_lorem.text

Merge branch 'next' into feat/lorem/issue-1546/add_character_length_p…

0046096

…arameter_to_lorem.text

feat(lorem): fix bug in text method

c3f19ad

feat(lorem): improve text method

9cac43d

Merge branch 'next' into feat/lorem/issue-1546/add_character_length_p…

742e794

…arameter_to_lorem.text

Shinigami92 modified the milestones: v8 - None milestone specific tasks, vAnytime Sep 13, 2023

ejcheng requested changes Mar 28, 2024

View reviewed changes

ST-DDT added s: accepted Accepted feature / Confirmed bug and removed s: needs decision Needs team/maintainer decision c: locale Permutes locale definitions labels Mar 28, 2024

ST-DDT requested changes Mar 28, 2024

View reviewed changes

ST-DDT marked this pull request as draft March 28, 2024 18:04

ST-DDT closed this Apr 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(lorem): add character length parameter to lorem.text #2227

feat(lorem): add character length parameter to lorem.text #2227

ghost commented Jun 27, 2023 •

edited by ghost

Loading

codecov bot commented Jun 27, 2023 •

edited

Loading

matthewmayer left a comment

matthewmayer commented Jul 19, 2023

ST-DDT left a comment •

edited

Loading

ghost commented Aug 20, 2023

ST-DDT commented Aug 24, 2023

ghost commented Aug 31, 2023

ejcheng Mar 28, 2024

ejcheng Mar 28, 2024

ejcheng Mar 28, 2024

ST-DDT commented Mar 28, 2024 •

edited

Loading

ST-DDT Mar 28, 2024

ST-DDT Mar 28, 2024 •

edited

Loading

ST-DDT Mar 28, 2024

ST-DDT Mar 28, 2024

ST-DDT commented Apr 25, 2024

bmulholland commented Sep 9, 2024

xDivisionByZerox commented Sep 9, 2024

	* The minimum length of text to generate.
	* The minimum amount of characters to generate.

	* The maximum length of text to generate.
	* The maximum amount of characters to generate.

	* @param options.length The length of text to generate as number or range.
	* @param options.length The length of text (in characters) to generate as number or range.

	return `${text.substring(0, length).replace(/.$/, '.')}`;
	return text.substring(0, length);

feat(lorem): add character length parameter to lorem.text #2227

feat(lorem): add character length parameter to lorem.text #2227

Conversation

ghost commented Jun 27, 2023 • edited by ghost Loading

codecov bot commented Jun 27, 2023 • edited Loading

Codecov Report

matthewmayer left a comment

Choose a reason for hiding this comment

matthewmayer commented Jul 19, 2023

ST-DDT left a comment • edited Loading

Choose a reason for hiding this comment

ghost commented Aug 20, 2023

ST-DDT commented Aug 24, 2023

ghost commented Aug 31, 2023

ejcheng Mar 28, 2024

Choose a reason for hiding this comment

ejcheng Mar 28, 2024

Choose a reason for hiding this comment

ejcheng Mar 28, 2024

Choose a reason for hiding this comment

ST-DDT commented Mar 28, 2024 • edited Loading

ST-DDT Mar 28, 2024

Choose a reason for hiding this comment

ST-DDT Mar 28, 2024 • edited Loading

Choose a reason for hiding this comment

ST-DDT Mar 28, 2024

Choose a reason for hiding this comment

ST-DDT Mar 28, 2024

Choose a reason for hiding this comment

ST-DDT commented Apr 25, 2024

bmulholland commented Sep 9, 2024

xDivisionByZerox commented Sep 9, 2024

ghost commented Jun 27, 2023 •

edited by ghost

Loading

codecov bot commented Jun 27, 2023 •

edited

Loading

ST-DDT left a comment •

edited

Loading

ST-DDT commented Mar 28, 2024 •

edited

Loading

ST-DDT Mar 28, 2024 •

edited

Loading