Conversation
…oc-style @Lion tags Co-authored-by: neongreen <1523306+neongreen@users.noreply.github.com>
Co-authored-by: neongreen <1523306+neongreen@users.noreply.github.com>
|
@copilot You can assume that tsc and node js are already available in PATH but everything else should be handled transparently I should be able to just run line and it works and there's absolutely no extra steps from my side no running of any helpers, etc.. It's must be handled by lion. Also must support combined language projects -- for example I could give you a mono pill that contains some folders with go and some folders with TS files and it should still produce a unified result |
…with tsc at runtime Co-authored-by: neongreen <1523306+neongreen@users.noreply.github.com>
Fixed in e8c9fd8. TypeScript support now works transparently with no setup required - just run |
Co-authored-by: neongreen <1523306+neongreen@users.noreply.github.com>
|
@copilot You must have unit test that verify that they extraction for type script is on par in terms of functionality where the extraction from go. Any differences have to be documented. Any documentation that existed for the lion project has to be fully actualized. Unit test must check different kinds of JSdoc comments. Unit tests must demonstrate mixing type script and go in one project works and results in a unified generated doc folder. |
…rences Co-authored-by: neongreen <1523306+neongreen@users.noreply.github.com>
Added in 8f482b1: Tests added:
Documentation updated:
Documented differences:
|
Co-authored-by: neongreen <1523306+neongreen@users.noreply.github.com>
Added in f0ff1cb. TypeScript now supports inline comments inside function bodies using The limitation was that the TypeScript compiler API's Example: function processData() {
// @lion implementation Initialize connection
connect();
//lion:implementation Process items
items.forEach(process);
}Both syntaxes work: |
🧪 CI InsightsHere's what we observed from your CI run for f0ff1cb. 🟢 All jobs passed!But CI Insights is watching 👀 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| err := filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if !d.IsDir() && (strings.HasSuffix(path, ".ts") || strings.HasSuffix(path, ".tsx")) { |
There was a problem hiding this comment.
The preflight scan in ExtractTypeScript walks every path and flags the directory as containing TypeScript as soon as it encounters any .ts/.tsx file (lines 356-360), but it doesn’t skip node_modules even though the extractor itself later ignores that folder. In a Go-only project that happens to vendor dependencies with TypeScript sources under node_modules, this check will still set hasTS=true and then try to run npm/tsc, failing with “npm/tsc not found” even though there are no TypeScript docs to parse. The detection should exclude node_modules (and other skipped folders) to avoid introducing a hard Node/npm dependency in those environments.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive TypeScript support to the lion documentation extraction tool, achieving feature parity with the existing Go extraction functionality.
Key changes:
- Implements TypeScript extraction using an embedded extractor script that uses the TypeScript compiler API
- Adds support for JSDoc-style
@liontags with metadata (title, section) - Implements inline comment support for both languages (
// @lionor//lion:inside function bodies)
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
lion/main.go |
Demonstrates inline comment placement by moving the //lion: tag after the comment block |
lion/internal/extractor/typescript.go |
New TypeScript extraction implementation with embedded extractor script compiled at runtime using tsc |
lion/internal/extractor/typescript_test.go |
Comprehensive test suite with parity tests, JSDoc format tests, inline comment tests, and mixed Go/TypeScript project tests |
lion/internal/extractor/extractor.go |
Integrates TypeScript extraction into main Extract function with doc comment updates |
lion/README.md |
Documents TypeScript support with examples, usage instructions, and language differences table |
lion/AGENTS.md |
Updates technical documentation with TypeScript implementation details and feature parity table |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let currentTopic: string | null = null; | ||
| let currentMeta: MetaInfo = {}; | ||
| let currentContent: string[] = []; | ||
| let topicLine = line; |
There was a problem hiding this comment.
The topicLine variable is set to line at the beginning of the function but is never updated when a new @lion tag is encountered within the same JSDoc comment. This means all entries extracted from a multi-tag JSDoc block will report the same line number (the start of the JSDoc), rather than the line where each specific @lion tag appears.
Consider updating topicLine when a new lion tag is found:
if (lionMatch) {
if (currentTopic) {
// ... push entry
}
currentTopic = lionMatch.topic;
currentMeta = lionMatch.meta;
currentContent = lionMatch.content ? [lionMatch.content] : [];
topicLine = line + lines.indexOf(rawLine); // Update to current line
continue;
}| if err := os.MkdirAll(nodeModulesDir, 0755); err != nil { | ||
| return nil, fmt.Errorf("failed to create node_modules directory: %w", err) | ||
| } | ||
| if err := os.Symlink(tsModulePath, filepath.Join(nodeModulesDir, "typescript")); err != nil { |
There was a problem hiding this comment.
Missing error handling for os.Symlink. On Windows, creating symlinks requires administrator privileges, which could cause this operation to fail. Consider adding a fallback mechanism or better error messaging for Windows users.
Suggestion: Add a platform-specific check or provide alternative solutions:
if err := os.Symlink(tsModulePath, filepath.Join(nodeModulesDir, "typescript")); err != nil {
// Try copying as fallback on Windows
if runtime.GOOS == "windows" {
// Implement copy fallback or provide clearer guidance
}
return nil, fmt.Errorf("failed to symlink typescript module (on Windows, run as administrator or install typescript locally): %w", err)
}| function findContainingFunction(pos: number): { name: string; node: ts.Node } | null { | ||
| let result: { name: string; node: ts.Node } | null = null; | ||
| function visit(node: ts.Node) { | ||
| if ((ts.isFunctionDeclaration(node) || ts.isMethodDeclaration(node) || ts.isArrowFunction(node) || | ||
| ts.isFunctionExpression(node)) && node.body) { | ||
| const body = node.body; | ||
| if (pos >= body.getStart() && pos <= body.getEnd()) { | ||
| let name = ""; | ||
| if (ts.isFunctionDeclaration(node) && node.name) name = node.name.text; | ||
| else if (ts.isMethodDeclaration(node) && ts.isIdentifier(node.name)) name = node.name.text; | ||
| else if (ts.isArrowFunction(node) || ts.isFunctionExpression(node)) { | ||
| // Try to get name from parent variable declaration | ||
| const parent = node.parent; | ||
| if (ts.isVariableDeclaration(parent) && ts.isIdentifier(parent.name)) name = parent.name.text; | ||
| } | ||
| result = { name, node }; | ||
| } | ||
| } | ||
| ts.forEachChild(node, visit); | ||
| } | ||
| visit(sourceFile); | ||
| return result; |
There was a problem hiding this comment.
The findContainingFunction algorithm uses linear traversal of the entire AST for each inline comment, which could be inefficient for large files with many comments. When a function contains nested functions, the algorithm doesn't properly handle the case where the innermost function should be selected.
The current logic:
- Finds any function containing the position (not necessarily the innermost)
- Overwrites
resultmultiple times if nested functions exist - Doesn't guarantee selecting the most specific (innermost) function
Consider tracking depth or ensuring the innermost function is selected:
function findContainingFunction(pos: number): { name: string; node: ts.Node } | null {
let result: { name: string; node: ts.Node } | null = null;
let smallestRange = Infinity;
function visit(node: ts.Node) {
if ((ts.isFunctionDeclaration(node) || ts.isMethodDeclaration(node) ||
ts.isArrowFunction(node) || ts.isFunctionExpression(node)) && node.body) {
const body = node.body;
if (pos >= body.getStart() && pos <= body.getEnd()) {
const range = body.getEnd() - body.getStart();
if (range < smallestRange) {
// ... extract name and update result
smallestRange = range;
}
}
}
ts.forEachChild(node, visit);
}
visit(sourceFile);
return result;
}
tscto compile embedded script at runtimetypescriptmodule for compilation@liontag parsing in JSDoc comments// @lionand//lion:syntax)Inline commentsNOW SUPPORTEDOriginal 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.