Rename TypeScript/JavaScript API to follow language conventions#91
Conversation
- Rename classes from snake_case to PascalCase (string_t -> StringT, etc.) - Rename function from snake_case to camelCase (buffer_size -> bufferSize) - Update all test files - Update example file - Update README.md - Update all documentation files Co-authored-by: hzhangxyz <11623447+hzhangxyz@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
- Update docs/concepts/search.md - Update docs/concepts/rules.md - Update docs/getting-started/installation.md - Update docs/index.md Co-authored-by: hzhangxyz <11623447+hzhangxyz@users.noreply.github.com>
hzhangxyz
left a comment
hzhangxyz
left a comment
There was a problem hiding this comment.
xx_t是c的习惯，rule_t应该变成Rule，而不是RuleT，其他类型同理。
There was a problem hiding this comment.
Pull request overview
This PR renames the TypeScript/JavaScript API from C++ snake_case conventions to JavaScript-standard naming conventions. The changes align the exported API with idiomatic JavaScript/TypeScript practices while maintaining the underlying C++ implementation's snake_case naming.
Key Changes:
- Renamed all exported classes from snake_case to PascalCase (e.g.,
string_t→StringT,search_t→SearchT) - Renamed exported functions from snake_case to camelCase (e.g.,
buffer_size()→bufferSize()) - Updated all tests, examples, and documentation to use the new naming conventions
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| atsds/index.mts | Renamed all exported classes and functions to follow JavaScript conventions; updated internal references |
| tests/test_variable.mjs | Updated imports and class instantiations to use new PascalCase class names and camelCase function names |
| tests/test_term.mjs | Updated imports and class instantiations to use new PascalCase class names and camelCase function names |
| tests/test_string.mjs | Updated imports and class instantiations to use new PascalCase class names and camelCase function names |
| tests/test_search.mjs | Updated imports and class instantiations to use new PascalCase class names |
| tests/test_rule.mjs | Updated imports and class instantiations to use new PascalCase class names and camelCase function names |
| tests/test_list.mjs | Updated imports and class instantiations to use new PascalCase class names and camelCase function names |
| tests/test_item.mjs | Updated imports and class instantiations to use new PascalCase class names and camelCase function names |
| examples/main.mjs | Updated example code to demonstrate new API naming conventions |
| docs/index.md | Updated TypeScript code examples to use new naming conventions |
| docs/getting-started/quickstart.md | Updated TypeScript code examples and API references to use new naming conventions |
| docs/getting-started/installation.md | Updated TypeScript code examples to use new naming conventions |
| docs/examples/basic.md | Updated TypeScript example code to use new naming conventions |
| docs/concepts/terms.md | Updated TypeScript code examples and explanations to use new naming conventions |
| docs/concepts/search.md | Updated TypeScript code examples and API references to use new naming conventions |
| docs/concepts/rules.md | Updated TypeScript code examples to use new naming conventions |
| docs/api/typescript.md | Comprehensively updated API reference documentation with new naming conventions |
| README.md | Updated TypeScript examples and API overview to use new naming conventions; C++ examples correctly retain snake_case |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| */ | ||
| rename(prefix_and_suffix: rule_t): rule_t | null { | ||
| const capacity = buffer_size(); | ||
| rename(prefix_and_suffix: RuleT): RuleT | null { |
There was a problem hiding this comment.
The parameter name prefix_and_suffix uses snake_case, which is inconsistent with JavaScript naming conventions. It should be renamed to prefixAndSuffix (camelCase) to follow JavaScript conventions.
There was a problem hiding this comment.
Fixed to prefixAndSuffix (commit 262761d)
| expect(v.toString()).toBe("`variable"); | ||
|
|
||
| const old_buffer_size = buffer_size(4); | ||
| const old_buffer_size = bufferSize(4); |
There was a problem hiding this comment.
The local variable old_buffer_size uses snake_case, which is inconsistent with JavaScript naming conventions. It should be renamed to oldBufferSize (camelCase) to follow JavaScript conventions.
| expect(v.toString()).toBe("(a b c)"); | ||
|
|
||
| const old_buffer_size = buffer_size(4); | ||
| const old_buffer_size = bufferSize(4); |
There was a problem hiding this comment.
The local variable old_buffer_size uses snake_case, which is inconsistent with JavaScript naming conventions. It should be renamed to oldBufferSize (camelCase) to follow JavaScript conventions.
| expect(v.toString()).toBe("item"); | ||
|
|
||
| const old_buffer_size = buffer_size(4); | ||
| const old_buffer_size = bufferSize(4); |
There was a problem hiding this comment.
The local variable old_buffer_size uses snake_case, which is inconsistent with JavaScript naming conventions. It should be renamed to oldBufferSize (camelCase) to follow JavaScript conventions.
| expect(v.toString()).toBe("(a b c)"); | ||
|
|
||
| const old_buffer_size = buffer_size(4); | ||
| const old_buffer_size = bufferSize(4); |
There was a problem hiding this comment.
The local variable old_buffer_size uses snake_case, which is inconsistent with JavaScript naming conventions. It should be renamed to oldBufferSize (camelCase) to follow JavaScript conventions.
| expect(v.toString()).toBe("----\n(a b c)\n"); | ||
|
|
||
| const old_buffer_size = buffer_size(4); | ||
| const old_buffer_size = bufferSize(4); |
There was a problem hiding this comment.
The local variable old_buffer_size uses snake_case, which is inconsistent with JavaScript naming conventions. It should be renamed to oldBufferSize (camelCase) to follow JavaScript conventions.
| export function buffer_size(size: number = 0): number { | ||
| const old_size = _buffer_size; | ||
| export function bufferSize(size: number = 0): number { | ||
| const old_size = _bufferSize; |
There was a problem hiding this comment.
The local variable old_size uses snake_case, which is inconsistent with JavaScript naming conventions. It should be renamed to oldSize (camelCase) to match the function name bufferSize and follow JavaScript conventions.
| */ | ||
| term(): variable_t | item_t | list_t { | ||
| term(): VariableT | ItemT | ListT { | ||
| const term_type: dst.TermType = this.value.get_type(); |
There was a problem hiding this comment.
The local variable term_type uses snake_case, which is inconsistent with JavaScript naming conventions. It should be renamed to termType (camelCase) to follow JavaScript conventions.
| */ | ||
| rename(prefix_and_suffix: term_t): term_t | null { | ||
| const capacity = buffer_size(); | ||
| rename(prefix_and_suffix: TermT): TermT | null { |
There was a problem hiding this comment.
The parameter name prefix_and_suffix uses snake_case, which is inconsistent with JavaScript naming conventions. It should be renamed to prefixAndSuffix (camelCase) to follow JavaScript conventions.
There was a problem hiding this comment.
Fixed to prefixAndSuffix (commit 262761d)
| expect(mp.match(pq).toString()).toBe("(! (! `x))\n----------\n`x\n"); | ||
|
|
||
| fail = new rule_t("(`q <- `p)"); | ||
| fail = new RuleT("(`q <- `p)"); |
There was a problem hiding this comment.
Variable fail is used like a local variable, but is missing a declaration.
| fail = new RuleT("(`q <- `p)"); | |
| const fail = new RuleT("(`q <- `p)"); |
There was a problem hiding this comment.
Fixed - added const declaration (commit 262761d)
Changed class names to match standard conventions: - StringT → String - VariableT → Variable - ItemT → Item - ListT → List - TermT → Term - RuleT → Rule - SearchT → Search - _CommonT → _Common Updated all source files, tests, examples, and documentation. Co-authored-by: hzhangxyz <11623447+hzhangxyz@users.noreply.github.com>
Done! Removed the "T" suffix from all class names. Classes are now: String, Variable, Item, List, Term, Rule, Search. (commit a9a445a) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * ``` | ||
| */ | ||
| export class string_t extends _common_t<dst.String> { | ||
| export class String extends _Common<dst.String> { |
There was a problem hiding this comment.
The class name String collides with JavaScript's built-in String object. This will cause issues when users try to use both in the same scope. Consider renaming to DSString, ATSString, or similar to avoid this conflict.
For example, this would fail:
import { String } from "atsds";
const str = new String("hello"); // Works
const builtinStr = String("hello"); // Error: String is not a constructorThere was a problem hiding this comment.
The biome linter also flags this as an error (noShadowRestrictedNames). Should I rename to Str, DSString, or another name to avoid the collision? The maintainer requested removing the _t suffix, but the collision is a real issue.
| const ds: dst.EmbindModule = await create_ds(); | ||
|
|
||
| let _buffer_size: number = 1024; | ||
| let _bufferSize: number = 1024; |
There was a problem hiding this comment.
函数需要变成驼峰, 内部的变量不用了, 这个还是用 原来的 _buffer_size 吧, 下面那个函数还是用bufferSize啊, 别修改函数名字
There was a problem hiding this comment.
Fixed. Internal variable is now _buffer_size (snake_case) while the exported function remains bufferSize (camelCase). (commit 262761d)
- Keep internal variable as _buffer_size (not _bufferSize) per maintainer feedback - Use camelCase for local variables: oldBufferSize, termType - Use camelCase for parameters: prefixAndSuffix - Add missing const declaration for fail variable in test Co-authored-by: hzhangxyz <11623447+hzhangxyz@users.noreply.github.com>
- Prefix unused parameters with underscore in test_search.mjs - Use template literal instead of string concatenation in bnf module Co-authored-by: hzhangxyz <11623447+hzhangxyz@users.noreply.github.com>
Summary
Updated TypeScript/JavaScript code to follow proper naming conventions and pass linter checks.
Completed Changes:
Classes (without T suffix):
string_t→Stringvariable_t→Variableitem_t→Itemlist_t→Listterm_t→Termrule_t→Rulesearch_t→Search_common_t→_Common(internal)Functions (camelCase):
buffer_size()→bufferSize()Internal variables:
_buffer_size(kept as snake_case per maintainer request)Local variables and parameters (camelCase):
old_buffer_size→oldBufferSizeterm_type→termTypeprefix_and_suffix→prefixAndSuffixLinter fixes:
constdeclaration forfailvariableKnown Issue:
Stringclass name collides with JavaScript's built-inStringobject. Awaiting maintainer guidance on preferred alternative name (e.g.,Str,DSString,ATSString).Files Updated: 19 files
atsds/index.mtstests/examples/main.mjsdocs/andREADME.mdbnf/atsds_bnf/index.mjsMigration Example
Breaking change for existing TypeScript/JavaScript users.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.