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.

Sign up for GitHub

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

Jump to bottom

fix: Update scope to default to static when memberof #1151

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: Update scope to default to static when memberof #1151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 36 additions & 0 deletions __tests__/__snapshots__/bin-readme.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,42 @@ A second function with docs
"
`;

exports[`readme command --diff-only: changes needed 1`] = `
"# A title

# API

<!-- Generated by documentation.js. Update this documentation by updating the source code. -->

### Table of Contents

- [foo](#foo)
- [Parameters](#parameters)
- [bar](#bar)
- [Parameters](#parameters-1)

## foo

A function with documentation.

### Parameters

- \`a\` {string} blah

Returns **[number](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Number)** answer

## bar

A second function with docs

### Parameters

- \`b\`

# Another section
"
`;

exports[`readme command --readme-file 1`] = `
"# A title

Expand Down
9 changes: 6 additions & 3 deletions __tests__/__snapshots__/test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22680,7 +22680,7 @@ Array [
"static": Array [],
},
"name": "getFoo",
"namespace": "MyClass#getFoo",
"namespace": ".MyClass#getFoo",
"params": Array [
Object {
"description": Object {
Expand Down Expand Up @@ -22748,6 +22748,7 @@ Array [
Object {
"kind": "class",
"name": "MyClass",
"scope": "static",
},
Object {
"kind": "function",
Expand Down Expand Up @@ -22933,12 +22934,13 @@ Array [
"static": Array [],
},
"name": "getUndefined",
"namespace": "MyClass.getUndefined",
"namespace": ".MyClass.getUndefined",
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this is a regression? Namespaces shouldn't start with ..

Copy link
Author

Choose a reason for hiding this comment

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

This is a strange case. In the tests MyClass is a member of com.Test. However com.Test is never defined. Should the name space be com.Test.MyClass.getUndefined? or should it remain MyClass.getUndefined?

Copy link
Author

Choose a reason for hiding this comment

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

@tmcw

Copy link
Member

Choose a reason for hiding this comment

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

This should probably remain as MyClass.getUndefined, and the way we format namespaces should avoid prefixing static scopes with .

Copy link
Author

Choose a reason for hiding this comment

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

@tmcw there's currently a namespace on master in the test snapshots that starts with a ..

This is the same scenario as above except in this case the static is implied from the syntax tree instead of being assumed based on the @memberof tag.

Adding memo && here would remove the leading . in all cases including cases currently in master. Let me know how I should proceed.

"params": Array [],
"path": Array [
Object {
"kind": "class",
"name": "MyClass",
"scope": "static",
},
Object {
"kind": "function",
Expand Down Expand Up @@ -23025,12 +23027,13 @@ Array [
],
},
"name": "MyClass",
"namespace": "MyClass",
"namespace": ".MyClass",
"params": Array [],
"path": Array [
Object {
"kind": "class",
"name": "MyClass",
"scope": "static",
},
],
"properties": Array [],
Expand Down
16 changes: 16 additions & 0 deletions __tests__/lib/hierarchy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,3 +184,19 @@ test('hierarchy - object prototype member names', function() {
'otherMethod'
]);
});

test('hierarchy - member namespace defaults to static', function() {
const comments = evaluate(function() {
/**
* @namespace fooNamespace
*/
/**
* @class BarClass
* @memberof fooNamespace
*/
});

expect(comments[0].members.static[0].namespace).toEqual(
'fooNamespace.BarClass'
);
});
2 changes: 2 additions & 0 deletions src/hierarchy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ function pick(comment) {

if (comment.scope) {
item.scope = comment.scope;
} else if (comment.memberof) {
item.scope = 'static';
}

return item;
Expand Down