feat: add ability to define number of generated extensions in system.filePath #395

pkuczynski · 2022-02-01T21:50:30Z

Includes #300 which should be merged first

ST-DDT · 2022-03-16T12:53:56Z

src/system.ts

+    switch (true) {
+      case ext === 0:
+        return path.slice(0, path.lastIndexOf('.'));
+
+      case ext === 1:
+        return path;
+
+      default: {
+        const extensions = new Array(ext - 1)
+          .fill('')
+          .map(() => this.fileExt())
+          .join('.');
+
+        return `${path}.${extensions}`;
+      }
+    }


Maybe move this logic into fileName()?

Good idea, let me do that...

Actually, Mayne I should do that in a separate PR not to mix concerts @ST-DDT ? It would be also better to spot on the changelog?

This PR is about changing the number of generated extensions,
I think that its acceptable to do so in two/three related methods within a single PR.

feat: add ability to define the number of generated file extensions

Maybe we just need to adjust the PR title a little bit 🤷
But I also think it's okay to provide this in one PR

Shinigami92 · 2022-03-29T15:41:49Z

src/system.ts

-    return this.faker.fake(
-      '{{system.directoryPath}}/{{system.fileName}}.{{system.fileExt}}'
-    );
+  filePath(ext = 1): string {


Should we make it also possible to set the nested dir depth?
So we would need to forward an arg to directoryPath
So IMO we should use an option here {extCount = 1, dirCount: 1-3?}

Sure, why not! :) I can do that once #300 is merged, ok?

Sounds good, for now stick to a fixed number for each of them.
In a later feature we can introduce number | { min, max } (that might be handy for multiple locations)
Words, Lorem, ...

directoryPath does not take any params. I would propose to extend it in a separate PR not to mix the concerns.

directory path in separate PR: OK

ST-DDT · 2022-03-29T16:17:15Z

#300 has been merged

Shinigami92 · 2022-04-05T14:25:11Z

@pkuczynski please rebase and resolve any conflicts 🙂

…filePath

codecov · 2022-04-05T23:05:17Z

Codecov Report

Merging #395 (3651bb1) into main (5beac4b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 3651bb1 differs from pull request most recent head 1cddd1b. Consider uploading reports for the commit 1cddd1b to get more accurate results

@@           Coverage Diff           @@
##             main     #395   +/-   ##
=======================================
  Coverage   99.34%   99.34%           
=======================================
  Files        1921     1921           
  Lines      176656   176676   +20     
  Branches      905      909    +4     
=======================================
+ Hits       175503   175523   +20     
  Misses       1097     1097           
  Partials       56       56

Impacted Files	Coverage Δ
src/system.ts	`96.78% <100.00%> (+0.32%)`	⬆️

ST-DDT

Please implement the requested features above.

pkuczynski · 2022-04-06T07:43:47Z

Please implement the requested features above.

Yes yes, give me a moment :)

ejcheng

Other than these two minor changes, this LGTM!

ejcheng · 2022-04-21T18:36:45Z

src/system.ts

+
+    switch (true) {
+      case ext < 0:
+        throw new FakerError('Options.ext shall not have negative value');


Suggested change

throw new FakerError('Options.ext shall not have negative value');

throw new FakerError('Options.ext should not be negative');

I think this phrasing would be more natural

ejcheng · 2022-04-21T18:37:56Z

test/system.spec.ts

+
+        it('should throw error ext < 0', () => {
+          expect(() => faker.system.filePath({ ext: -1 })).toThrow(
+            new FakerError('Options.ext shall not have negative value')


Suggested change

new FakerError('Options.ext shall not have negative value')

new FakerError('Options.ext should not be negative')

xDivisionByZerox · 2022-06-13T15:45:57Z

@pkuczynski are you still working on this? Otherwise, I would continue the development process for you.

xDivisionByZerox · 2022-07-04T17:19:40Z

Will be continued in #1101.

pkuczynski requested a review from a team as a code owner February 1, 2022 21:50

pkuczynski added the c: feature Request for new feature label Feb 1, 2022

pkuczynski added this to the v6.2 - New small features milestone Feb 1, 2022

pkuczynski mentioned this pull request Feb 1, 2022

fix: remove doubled extension in system.filePath #300

Merged

pkuczynski changed the title ~~fix: remove duplicated extension in system.filePath~~ feat: add ability to define number of generated extensions in system filePath Feb 1, 2022

pkuczynski changed the title ~~feat: add ability to define number of generated extensions in system filePath~~ feat: add ability to define number of generated extensions in system.filePath Feb 1, 2022

Shinigami92 added the p: 1-normal Nothing urgent label Mar 15, 2022

ST-DDT reviewed Mar 16, 2022

View reviewed changes

Shinigami92 reviewed Mar 29, 2022

View reviewed changes

pkuczynski self-assigned this Mar 29, 2022

pkuczynski added the s: on hold Blocked by something or frozen to avoid conflicts label Mar 29, 2022

ST-DDT added the needs rebase There is a merge conflict label Mar 29, 2022

ST-DDT removed the s: on hold Blocked by something or frozen to avoid conflicts label Mar 29, 2022

pkuczynski added 3 commits April 6, 2022 00:54

fix: remove duplicated extension in system.filePath

dc569ba

chore: fix tests

d706f18

feat: add ability to define number of generated extensions in system.…

a0f7905

…filePath

pkuczynski requested a review from a team April 5, 2022 23:00

pkuczynski force-pushed the system-filepath-ext branch from 3651bb1 to a0f7905 Compare April 5, 2022 23:10

ST-DDT removed the needs rebase There is a merge conflict label Apr 6, 2022

ST-DDT requested changes Apr 6, 2022

View reviewed changes

pkuczynski added 2 commits April 6, 2022 13:54

chore: ext as option

4edadf2

chore: tests

1cddd1b

ejcheng added the s: accepted Accepted feature / Confirmed bug label Apr 9, 2022

ejcheng reviewed Apr 21, 2022

View reviewed changes

Shinigami92 modified the milestones: v6.2 - New small features, v6.3 - Next Minor Apr 21, 2022

Shinigami92 modified the milestones: v6.3 - Next Minor, v7.0 - Next Minor, v7 - Current Major May 2, 2022

xDivisionByZerox mentioned this pull request Jun 12, 2022

Source code todo list v2 #1044

Closed

xDivisionByZerox added the needs rebase There is a merge conflict label Jun 13, 2022

xDivisionByZerox mentioned this pull request Jun 22, 2022

feat(system.fileName): file extension count #1101

Merged

xDivisionByZerox closed this Jul 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add ability to define number of generated extensions in system.filePath #395

feat: add ability to define number of generated extensions in system.filePath #395

pkuczynski commented Feb 1, 2022

ST-DDT Mar 16, 2022

pkuczynski Apr 6, 2022

pkuczynski Apr 6, 2022

ST-DDT Apr 6, 2022 •

edited

Loading

Shinigami92 Apr 6, 2022

Shinigami92 Mar 29, 2022

pkuczynski Mar 29, 2022

Shinigami92 Mar 29, 2022

ST-DDT Mar 29, 2022

pkuczynski Apr 6, 2022

ST-DDT Apr 6, 2022

ST-DDT commented Mar 29, 2022

Shinigami92 commented Apr 5, 2022

codecov bot commented Apr 5, 2022 •

edited

Loading

ST-DDT left a comment •

edited

Loading

pkuczynski commented Apr 6, 2022

ejcheng left a comment

ejcheng Apr 21, 2022

ejcheng Apr 21, 2022

xDivisionByZerox commented Jun 13, 2022

xDivisionByZerox commented Jul 4, 2022

	throw new FakerError('Options.ext shall not have negative value');
	throw new FakerError('Options.ext should not be negative');

	new FakerError('Options.ext shall not have negative value')
	new FakerError('Options.ext should not be negative')

feat: add ability to define number of generated extensions in system.filePath #395

feat: add ability to define number of generated extensions in system.filePath #395

Conversation

pkuczynski commented Feb 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ST-DDT Apr 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ST-DDT commented Mar 29, 2022

Shinigami92 commented Apr 5, 2022

codecov bot commented Apr 5, 2022 • edited Loading

Codecov Report

ST-DDT left a comment • edited Loading

Choose a reason for hiding this comment

pkuczynski commented Apr 6, 2022

ejcheng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xDivisionByZerox commented Jun 13, 2022

xDivisionByZerox commented Jul 4, 2022

ST-DDT Apr 6, 2022 •

edited

Loading

codecov bot commented Apr 5, 2022 •

edited

Loading

ST-DDT left a comment •

edited

Loading