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 8 commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ node_modules/
/playwright-report/
/playwright/.cache/
$__StoryList.tid
types/generated/
29 changes: 29 additions & 0 deletions core/modules/parsers/wikiparser/rules/codeblock.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,25 @@ Wiki text rule for code blocks. For example:
```

\*/

/**
* 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.
*/

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

(function(){

/*jslint node: true, browser: true */
Expand All @@ -21,12 +40,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
31 changes: 19 additions & 12 deletions core/modules/parsers/wikiparser/wikiparser.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,20 @@ Attributes are stored as hashmaps of the following objects:
/*global $tw: false */
"use strict";

/*
type: content type of text
text: text to be parsed
options: see below:
parseAsInline: true to parse text as inline instead of block
wiki: reference to wiki to use
_canonical_uri: optional URI of content if text is missing or empty
configTrimWhiteSpace: true to trim whitespace
*/
var WikiParser = function(type,text,options) {
/**
* WikiParser class for parsing text of a specified MIME type.
*
* @class
* @constructor
* @param {string} type - The content type of the text to be parsed.
* @param {string} text - The text to be parsed.
* @param {Object} options - Options for parsing.
* @param {boolean} [options.parseAsInline=false] - If true, the text will be parsed as an inline run.
* @param {Object} options.wiki - Reference to the wiki to use.
* @param {string} [options._canonical_uri] - Optional URI of the content if the text is missing or empty.
* @param {boolean} [options.configTrimWhiteSpace=false] - If true, trims white space according to configuration.
Copy link
Member

Choose a reason for hiding this comment

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

IMO the full-stop at the end of the comment text can be removed.
IMO "The" and "the" can be removed -- most of the time. So it does not obscure the important content. eg:

 * @param {string} type - Content type of the text to be parsed
 * @param {string} text - Text to be parsed
 * @param {Object} options - Parser options
 * @param {boolean} [options.parseAsInline=false] - If true, the text will be parsed as an inline run
 * @param {Object} options.wiki - Reference to the wiki store in use
 * @param {string} [options._canonical_uri] - Optional URI of the content if text is missing or empty
 * @param {boolean} [options.configTrimWhiteSpace=false] - If true, the parser trims white space

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, this will be the updated prompt:

add jsdoc, only output jsdoc for this class and its method, no impl. full-stop at the end of the comment text can be removed. "The" and "the" can be removed -- most of the time. So it does not obscure the important content.

*/
function WikiParser(type,text,options) {
this.wiki = options.wiki;
var self = this;
// Check for an externally linked tiddler
Expand Down Expand Up @@ -99,8 +103,11 @@ var WikiParser = function(type,text,options) {
// Return the parse tree
};

/*
*/
/**
* Load a remote tiddler from a given URL.
*
* @param {string} url - The URL of the remote tiddler to load.
*/
WikiParser.prototype.loadRemoteTiddler = function(url) {
var self = this;
$tw.utils.httpRequest({
Expand Down
43 changes: 27 additions & 16 deletions core/modules/widgets/widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,30 @@ Widget base class
/*global $tw: false */
"use strict";

/*
Create a widget object for a parse tree node
parseTreeNode: reference to the parse tree node to be rendered
options: see below
Options include:
wiki: mandatory reference to wiki associated with this render tree
parentWidget: optional reference to a parent renderer node for the context chain
document: optional document object to use instead of global document
*/
var Widget = function(parseTreeNode,options) {
/**
* Widget class for creating a widget object for a parse tree node.
*
* @class
* @constructor
* @param {Object} parseTreeNode - Reference to the parse tree node to be rendered.
* @param {Object} options - Options for the widget.
* @param {Object} options.wiki - Mandatory reference to the wiki associated with this render tree.
* @param {Widget} [options.parentWidget] - Optional reference to a parent renderer node for the context chain.
* @param {Document} [options.document] - Optional document object to use instead of the global document.
*/
function Widget(parseTreeNode,options) {
this.initialise(parseTreeNode,options);
};

/*
Initialise widget properties. These steps are pulled out of the constructor so that we can reuse them in subclasses
*/
/**
* Initialise widget properties. These steps are pulled out of the constructor so that we can reuse them in subclasses.
*
* @param {Object} parseTreeNode - Reference to the parse tree node to be rendered.
* @param {Object} options - Options for the widget.
* @param {Object} options.wiki - Mandatory reference to the wiki associated with this render tree.
* @param {Widget} [options.parentWidget] - Optional reference to a parent renderer node for the context chain.
* @param {Document} [options.document] - Optional document object to use instead of the global document.
*/
Widget.prototype.initialise = function(parseTreeNode,options) {
// Bail if parseTreeNode is undefined, meaning that the widget constructor was called without any arguments so that it can be subclassed
if(parseTreeNode === undefined) {
Expand Down Expand Up @@ -64,9 +72,12 @@ Widget.prototype.initialise = function(parseTreeNode,options) {
}
};

/*
Render this widget into the DOM
*/
/**
* Render this widget into the DOM.
*
* @param {Element} parent - The parent DOM node to render into.
* @param {Element} nextSibling - The next sibling DOM node to render before.
*/
Widget.prototype.render = function(parent,nextSibling) {
this.parentDomNode = parent;
this.execute();
Expand Down
90 changes: 51 additions & 39 deletions core/modules/wiki.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,8 @@ Adds the following properties to the wiki object:
* `globalCache` is a hashmap by cache name of cache objects that are cleared whenever any tiddler change occurs

\*/
(function(){

/*jslint node: true, browser: true */
/*global $tw: false */
"use strict";

var widget = require("$:/core/modules/widgets/widget.js");
var Widget = require("$:/core/modules/widgets/widget.js").widget;

var USER_NAME_TITLE = "$:/status/UserName",
TIMESTAMP_DISABLE_TITLE = "$:/config/TimestampDisable";
Expand Down Expand Up @@ -1041,19 +1036,30 @@ exports.initParsers = function(moduleType) {
}
};

/*
Parse a block of text of a specified MIME type
type: content type of text to be parsed
text: text
options: see below
Options include:
parseAsInline: if true, the text of the tiddler will be parsed as an inline run
_canonical_uri: optional string of the canonical URI of this content
*/
/**
* @typedef {import('$:/core/modules/parsers/wikiparser/wikiparser.js')["text/vnd.tiddlywiki"]} WikiParser
*/

/**
* Parse a block of text of a specified MIME type.
*
* @param {string} type - The content type of the text to be parsed.
* @param {string} text - The text to be parsed.
* @param {Object} [options] - Options for parsing.
* @param {boolean} [options.parseAsInline=false] - If true, the text will be parsed as an inline run.
* @param {string} [options._canonical_uri] - Optional string of the canonical URI of this content.
* @param {string} [options.defaultType="text/vnd.tiddlywiki"] - The default type to use if no parser is found for the specified type.
* @param {boolean} [options.configTrimWhiteSpace=false] - If true, trims white space according to configuration.
*
* @returns {WikiParser|null} The parser instance or null if no parser is found.
*/
exports.parseText = function(type,text,options) {
text = text || "";
options = options || {};
// Select a parser
/**
* @type WikiParser
* Select a parser
*/
var Parser = $tw.Wiki.parsers[type];
if(!Parser && $tw.utils.getFileExtensionInfo(type)) {
Parser = $tw.Wiki.parsers[$tw.utils.getFileExtensionInfo(type).type];
Expand Down Expand Up @@ -1143,14 +1149,16 @@ exports.getTextReferenceParserInfo = function(title,field,index,options) {
return parserInfo;
}

/*
Parse a block of text of a specified MIME type
text: text on which to perform substitutions
widget
options: see below
Options include:
substitutions: an optional array of substitutions
*/
/**
* Parse a block of text of a specified MIME type and perform substitutions.
*
* @param {string} text - The text on which to perform substitutions.
* @param {Widget} widget - The widget context used for variable substitution.
* @param {Object} [options] - Options for substitutions.
* @param {Array<{name: string, value: string}>} [options.substitutions] - An optional array of substitutions.
*
* @returns {string} The text with substitutions applied.
*/
exports.getSubstitutedText = function(text,widget,options) {
options = options || {};
text = text || "";
Expand All @@ -1171,15 +1179,17 @@ exports.getSubstitutedText = function(text,widget,options) {
});
};

/*
Make a widget tree for a parse tree
parser: parser object
options: see below
Options include:
document: optional document to use
variables: hashmap of variables to set
parentWidget: optional parent widget for the root node
*/
/**
* Create a widget tree for a parse tree.
*
* @param {Object} parser - The parser object containing the parse tree.
* @param {Object} [options] - Options for creating the widget tree.
* @param {Document} [options.document] - Optional document to use.
* @param {Object} [options.variables] - Hashmap of variables to set.
* @param {Widget} [options.parentWidget] - Optional parent widget for the root node.
*
* @returns {Widget} The root widget of the created widget tree.
*/
exports.makeWidget = function(parser,options) {
options = options || {};
var widgetNode = {
Expand All @@ -1204,7 +1214,7 @@ exports.makeWidget = function(parser,options) {
// Add in the supplied parse tree nodes
currWidgetNode.children = parser ? parser.tree : [];
// Create the widget
return new widget.widget(widgetNode,{
return new Widget(widgetNode,{
wiki: this,
document: options.document || $tw.fakeDocument,
parentWidget: options.parentWidget
Expand Down Expand Up @@ -1487,9 +1497,13 @@ exports.search = function(text,options) {
return results;
};

/*
Trigger a load for a tiddler if it is skinny. Returns the text, or undefined if the tiddler is missing, null if the tiddler is being lazily loaded.
*/
/**
* Trigger a load for a tiddler if it is skinny. Returns the text, or undefined if the tiddler is missing, null if the tiddler is being lazily loaded.
*
* @param {string} title - The title of the tiddler.
* @param {string} [defaultText] - The default text to return if the tiddler is missing.
* @returns {string | null | undefined} - The text of the tiddler, undefined if the tiddler is missing, or null if the tiddler is being lazily loaded.
*/
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 the type text should be as short as possible, but still understandable. Your definition contains "undefined", which is not returned. So I would suggest:

/**
* Returns the tiddler text-field. If field `_is_skinny` is present it triggers a "lazyLoad" event.
* 
* @param {string} title - Tiddler title
* @param {string} [defaultText] - Returned if tiddler is missing
* @returns {string | null } - Returns the "text-field" or an empty string, "defaultText" if the tiddler is missing or null if the tiddler is being lazily loaded
*/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When add @returns, it means it always return, so can't remove undefined here. It is same in TypeScript.

exports.getTiddlerText = function(title,defaultText) {
var tiddler = this.getTiddler(title);
// Return undefined if the tiddler isn't found
Expand Down Expand Up @@ -1781,5 +1795,3 @@ exports.slugify = function(title,options) {
}
return slug;
};

})();
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"bin": {
"tiddlywiki": "./tiddlywiki.js"
},
"types": "./types/tw.d.ts",
"main": "./boot/boot.js",
"repository": {
"type": "git",
Expand Down
26 changes: 26 additions & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"compilerOptions": {
"paths": {
// Allow `import('$:/core/modules/...')` instead of `import('../../core/modules/...')`. Only works inside this project.
"$:/core/*": ["./core/*"]
},
"noImplicitAny": false,
"strict": false,

"allowJs": true,
"allowSyntheticDefaultImports": true,
"checkJs": true,
"declaration": true,
"declarationDir": "types/generated",
"declarationMap": true,
"emitDeclarationOnly": true,
"lib": ["ES2020", "DOM", "DOM.Iterable"],
"module": "Node16",
"outDir": "types/generated",
"rootDir": ".",
"skipLibCheck": true,
"target": "ESNext"
},
"include": ["./core/**/*.js", "./core/**/*.d.ts", "types/tw.d.ts"],
"exclude": ["types/generated"]
}
7 changes: 7 additions & 0 deletions types/tw.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import * as Wiki from '../core/modules/wiki';

declare global {
var $tw: {
wiki: typeof Wiki;
};
}
Loading