Skip to content
This repository was archived by the owner on Feb 1, 2024. It is now read-only.

Fixes #1

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

Fixes #1

wants to merge 21 commits into from

Conversation

w8r
Copy link
Collaborator

@w8r w8r commented Mar 28, 2022

Hi @Ebernn , can you edit this PR to list the changes, please? It it the main branch where you are adding changes?

Changes

  • x
  • y

@w8r w8r requested a review from a team March 28, 2022 09:30
Copy link
Collaborator Author

@w8r w8r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the example folder contents should go to somewhere in the text/fixture

.eslintrc Outdated
@@ -26,7 +26,7 @@
"no-shadow": 0,
"no-undef": 2
},
"extends": ["eslint:recommended", "prettier"],
"extends": ["eslint:recommended", "airbnb-typescript-prettier"],
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for this stuff, you can create another .eslintrc in the folder when you want to use these rules. Otherwise the PR will never be accepted upstream

@@ -1,3 +1,5 @@
coverage
.nyc_output
/node_modules
docs/index.html
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs folder is used for the project documentation. Is there something like dist here?

@@ -0,0 +1,3 @@
{
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better not to commit in this PR

undocumentedMethod(): string;
}

export { Utils, I, Test };
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 !

traverse(ast, {
Statement(path) {
// https://stackoverflow.com/a/66294873
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment about what happens if you don't do this trick?

@@ -125,17 +130,24 @@ export default function walkExported(
bindingPath = tmp.ast;
specData = tmp.data;
} else if (exportKind === 'value') {
bindingPath = path.scope.getBinding(local).path;
const binding = path.scope.getBinding(local);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs comment

@@ -72,7 +72,8 @@ export default function inferProperties(comment) {
function inferProperties(value, prefix) {
if (
value.type === 'ObjectTypeAnnotation' ||
value.type === 'TSTypeLiteral'
value.type === 'TSTypeLiteral' ||
value.type === 'TSInterfaceBody'
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -36,7 +36,7 @@ function buildMarkdownAST(comments, config) {
comments,
namespace => {
const slugger = new GithubSlugger();
return '#' + slugger.slug(namespace);
return `#${slugger.slug(namespace)}`;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is needed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean space

}
return [t(node.key)];

case Syntax.FunctionType:
result = [t('function (')];

if (node['this']) {
if (node['new']) {
if (node.this) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep it they way they had it before, for clarity. Reserved words should be handled with care

src/parse.js Outdated
* @param {string[]} tags tags to consider
* @returns description string
*/
function retreiveDescriptionNextToTags(rawComment, tags) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

retrieve

*/
function retreiveDescriptionNextToTags(rawComment, tags) {
// regex to retreive the whole description string (new lines included)
const descLines = new RegExp(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can it be just starts with * and ends either with */ or * @ (new tag start)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are matching all the description lines from JSDoc, right?

/**
 * @tag xxx
 * <MATCHED_CONTENT>
 * @other_tag
 */

@@ -660,7 +698,7 @@ export default function parseJSDoc(comment, loc, context) {
flatteners[tag.title](result, tag, tag.title);
} else {
result.errors.push({
message: 'unknown tag @' + tag.title,
message: `unknown tag @${tag.title}`,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants