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

Api docs parameter default extraction removes parts of the description #794

Closed
ST-DDT opened this issue Apr 7, 2022 · 3 comments · Fixed by #836
Closed

Api docs parameter default extraction removes parts of the description #794

ST-DDT opened this issue Apr 7, 2022 · 3 comments · Fixed by #836
Assignees
Labels
c: docs Improvements or additions to documentation p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug

Comments

@ST-DDT
Copy link
Member

ST-DDT commented Apr 7, 2022

Describe the bug

Found while reviewing #792 (comment) and writing #793.

const result = /(.*)[ \n]Defaults to `([^`]+)`./.exec(text);
if (!result) {
return;
}
comment.shortText = result[1];

The code removes the description after the parameter default section.

Reproduction

faker/src/time.ts

Lines 13 to 18 in 81171c9

* @param format The format to use. Defaults to `'unix'`.
*
* - `'abbr'` Return a string with only the time. `Date.toLocaleTimeString`.
* - `'date'` Return a date instance.
* - `'wide'` Return a string with a long time. `Date.toTimeString()`.
* - `'unix'` Returns a unix timestamp.

grafik

Additional Info

Possible solutions:

  1. Require the default to be the last segment
  2. Fix the code to not erase the suffix
  3. Intentionally ignore the parts after the defaults (not recommended)
@ST-DDT ST-DDT added c: docs Improvements or additions to documentation s: needs decision Needs team/maintainer decision labels Apr 7, 2022
@ST-DDT ST-DDT added this to the v6 - Non-Breaking Changes milestone Apr 7, 2022
@ST-DDT ST-DDT moved this to Todo in Faker Roadmap Apr 7, 2022
@Shinigami92
Copy link
Member

I would love to go with 1. Require the default to be the last segment so we have a specific convention.

@ST-DDT
Copy link
Member Author

ST-DDT commented Apr 7, 2022

So the docs should fail if it's specified otherwise?

@Shinigami92
Copy link
Member

So the docs should fail if it's specified otherwise?

I assume yes, not the docs should fail, but the generation of the docs should.

@ST-DDT ST-DDT added p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug and removed s: needs decision Needs team/maintainer decision labels Apr 12, 2022
@ST-DDT ST-DDT self-assigned this Apr 12, 2022
@ST-DDT ST-DDT moved this from Todo to Awaiting Review in Faker Roadmap Apr 12, 2022
Repository owner moved this from Awaiting Review to Done in Faker Roadmap Apr 20, 2022
@ST-DDT ST-DDT removed this from Faker Roadmap Nov 4, 2022
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 p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants