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

feat: add JSDoc example #8533

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion core/modules/parsers/wikiparser/rules/codeblock.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/*\
// @ts-check
/**
Copy link
Member

Choose a reason for hiding this comment

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

What does this do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It enables VSCode and ts cli regard this file as .ts file. Maybe this enable different parse mode of VSCode.

Copy link
Member

Choose a reason for hiding this comment

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

The docs seem to imply that one can also use a separate "jsconfig.json" file. Would that be possible here?

Copy link
Contributor Author

@linonetwo linonetwo Aug 21, 2024

Choose a reason for hiding this comment

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

I try again and find VScode works with or without this. What make it work is open both file as tab...

I need to do more experiment.

title: $:/core/modules/parsers/wikiparser/rules/codeblock.js
Copy link
Member

Choose a reason for hiding this comment

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

Do we have to change the existing /*\ pattern?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I find @module $:/core/modules/parsers/wikiparser/rules/codeblock.js is useless for us, so the comment here don't need to be JSDoc anymore.

Copy link
Contributor Author

@linonetwo linonetwo Sep 10, 2024

Choose a reason for hiding this comment

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

But the comment for type needs to be valid jsdoc

Without JSDoc:

截屏2024-09-10 15 04 18

With JSDoc:

截屏2024-09-10 15 04 26

type: application/javascript
module-type: wikirule
Expand All @@ -11,7 +12,28 @@ Wiki text rule for code blocks. For example:
```
```

@module $:/core/modules/parsers/wikiparser/rules/codeblock.js

linonetwo marked this conversation as resolved.
Show resolved Hide resolved
\*/

/**
* Represents the `codeblock` rule.
*
* @typedef {Object} CodeblockNode
* @property {string} type - The type of the widget, which is "codeblock".
* @property {Object} attributes - The attributes of the codeblock.
* @property {Object} attributes.code - The code attribute object.
* @property {string} attributes.code.type - The type of the code attribute, which is "string".
* @property {string} attributes.code.value - The actual code content within the code block.
* @property {number} attributes.code.start - The start position of the code in the source text.
* @property {number} attributes.code.end - The end position of the code in the source text.
* @property {Object} attributes.language - The language attribute object.
* @property {string} attributes.language.type - The type of the language attribute, which is "string".
* @property {string} attributes.language.value - The language specified after the triple backticks, if any.
* @property {number} attributes.language.start - The start position of the language string in the source text.
* @property {number} attributes.language.end - The end position of the language string in the source text.
*/

(function(){
Copy link
Member

@pmario pmario Sep 10, 2024

Choose a reason for hiding this comment

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

IMO having @property {string} type and then - The type of the widget, which is "codeblock". is the same thing. Both elements describe: "What it is". -- I think the second part should describe: "What it does", otherwise we can skip it.

  • @Property does describe -- What it is
  • Text describes -- What it does

eg:

/**
 * Represents the parser `codeblock` rule.
 * 
 * @typedef {Object} CodeblockNode
 * @property {string} rule - Always "codeblock"
 * @property {string} type - Always "codeblock"
 * @property {number} start - Rule start marker in source text
 * @property {number} end - Rule end marker in source text
 * @property {Object} attributes - Contains "code" and "language" objects
 * @property {Object} attributes.code - The code attribute object
 * @property {string} attributes.code.type - Always "string"
 * @property {string} attributes.code.value - Actual code
 * @property {number} attributes.code.start - Start position of code in source text
 * @property {number} attributes.code.end - End position of code in source text
 * @property {Object} attributes.language - The language attribute object
 * @property {string} attributes.language.type - Always "string"
 * @property {string} attributes.language.value - Language specified after the triple backticks, if any
 * @property {number} attributes.language.start - Start position of the string in source text
 * @property {number} attributes.language.end - End position of the string in source text
 */

Should be as short as possible and still valid. Especially see type, rule, start, end which are properties of "CodeblockNode"

I did remove the "full stops" from all @ elements. They are not needed
The first intro sentence has a full stop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'd copy that.

I have to admit, all jsdoc are generated by github copilot. I don't have time for this detail, I will leave it for future refinement. I'm still debugging the type when I'm using it in my plugin project.

Copy link
Member

Choose a reason for hiding this comment

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

I have to admit, all jsdoc are generated by github copilot. I don't have time for this detail, I will leave it for future refinement. I'm still debugging the type when I'm using it in my plugin project.

The new type information will significantly increase the TW code-size. So if there is redundant information it should be removed.

And even more important it has to be valid. So if the co-pilot only adds comments in a more verbose form than the parameters are. There should not be any comments at all -- They have no value.

So if there are no comments, we actually know, that we have to add them manually. IMO for the tooltips it would be OK to start with the type info.

Copy link
Contributor Author

@linonetwo linonetwo Sep 12, 2024

Choose a reason for hiding this comment

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

will significantly increase the TW code-size

we must remove comments before publishing HTML wiki. I think these JSDoc will basically double the size.

it has to be valid

This won't be a big problem, because 1. I use the method body as input too. And 2. we can auto merge "comment" type of PR based on #7542 , so people can update comment quickly. I find this is quite frequent when maintaining https://github.com/tiddly-gittly/TW5-Typed


/*jslint node: true, browser: true */
Expand All @@ -21,12 +43,22 @@ Wiki text rule for code blocks. For example:
exports.name = "codeblock";
exports.types = {block: true};

/**
* Initializes the codeblock rule with the given parser.
*
* @param {Object} parser - The parser object that manages the state of the parsing process.
*/
exports.init = function(parser) {
this.parser = parser;
// Regexp to match and get language if defined
this.matchRegExp = /```([\w-]*)\r?\n/mg;
};

/**
* Parses the code block and returns an array of `codeblock` widgets.
*
* @returns {CodeblockNode[]} An array containing a single codeblock widget object.
*/
exports.parse = function() {
var reEnd = /(\r?\n```$)/mg;
var languageStart = this.parser.pos + 3,
Expand Down
15 changes: 15 additions & 0 deletions core/modules/parsers/wikiparser/try.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// @ts-check
/**
* @typedef {import('./rules/codeblock').CodeblockNode} CodeblockNode
*/

/**
* A function that processes a code block.
*
* @param {CodeblockNode[]} codeblocks - An array of codeblock rules.
*/
function processCodeblocks(codeblocks) {
codeblocks.forEach(function(cb) {
console.log(cb.attributes.code.value);
});
}
Loading