Added functionality to get treeview as JSON object for issue #8#27
Added functionality to get treeview as JSON object for issue #8#27vedpawar2254 merged 6 commits intovedpawar2254:mainfrom
Conversation
Summary of ChangesHello @yashpss01, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature for outputting the directory structure as a JSON object. My review focuses on ensuring the output is valid, parseable JSON and on improving the performance and clarity of the new implementation. I've identified a critical issue where the output is not valid JSON, and I've provided suggestions to correct this. Additionally, there are opportunities to refactor the recursive function for better performance and to update the README with accurate information and examples. With these adjustments, this feature will be a robust and useful addition to the tool.
| if (asJson){ | ||
| console.log(startPath, ":",printTreeAsJson(startPath, ignoreConfig, dirsOnly)) | ||
| } |
There was a problem hiding this comment.
The current implementation for the --as-json flag does not produce a valid JSON string. console.log() with multiple arguments will print a space-separated string representation of those arguments. To generate a valid JSON output that can be parsed by other tools, you should use JSON.stringify(). The root path should also be part of the JSON object itself for a valid structure.
if (asJson){
const tree = { [startPath]: printTreeAsJson(startPath, ignoreConfig, dirsOnly) };
console.log(JSON.stringify(tree, null, 2));
}| function printTreeAsJson( | ||
| dirPath, | ||
| ignoreConfig = { exactMatches: [], globPatterns: [] }, | ||
| dirsOnly = false, | ||
| jsonTree = {} | ||
|
|
||
| ) { | ||
| let items = fs | ||
| .readdirSync(dirPath) | ||
| .filter(item => !shouldIgnoreItem(item, ignoreConfig)); | ||
| // If dirsOnly, filter out files | ||
| if (dirsOnly) { | ||
| items = items.filter(item => { | ||
| const fullPath = path.join(dirPath, item); | ||
| return fs.statSync(fullPath).isDirectory(); | ||
| }); | ||
| } | ||
|
|
||
| items.forEach((item, index) => { | ||
| const fullPath = path.join(dirPath, item); | ||
| if (fs.statSync(fullPath).isDirectory()) { | ||
| jsonTree = {...jsonTree, [item]:printTreeAsJson(fullPath, ignoreConfig, dirsOnly,{})}; | ||
| }else{ | ||
| jsonTree = { ...jsonTree , [item] :"FILE"} | ||
| } | ||
| }); | ||
| return jsonTree; | ||
| } |
There was a problem hiding this comment.
The printTreeAsJson function can be inefficient for large directory structures for a few reasons:
- It uses the spread syntax (
...) inside aforEachloop to create a newjsonTreeobject on every iteration. This creates many intermediate objects, which can harm performance and increase memory usage. - The
jsonTree = {}parameter is used as an accumulator, but the recursive callprintTreeAsJson(..., {})passes a new empty object, which makes the parameter's purpose a bit confusing. - When
dirsOnlyis true,fs.statSync()is called twice for each directory: once in the initial filter and again inside theforEachloop.
A more efficient and clearer approach is to create the object once, mutate it in the loop, and avoid redundant statSync calls.
function printTreeAsJson(
dirPath,
ignoreConfig = { exactMatches: [], globPatterns: [] },
dirsOnly = false
) {
const jsonTree = {};
const items = fs
.readdirSync(dirPath)
.filter(item => !shouldIgnoreItem(item, ignoreConfig));
for (const item of items) {
const fullPath = path.join(dirPath, item);
const stats = fs.statSync(fullPath);
if (stats.isDirectory()) {
jsonTree[item] = printTreeAsJson(fullPath, ignoreConfig, dirsOnly);
} else if (!dirsOnly) {
jsonTree[item] = "FILE";
}
}
return jsonTree;
}Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Format JSON output with indentation for better readability.
|
@vedpawar2254 @yashpss01 the output does not seem to be complete. It does not show me files. Ideally, the output should allow us to distinguish between folders and files and allow for extensibility. I think it would be better if we have the output like so: {
"path": "/Users/krushn/project",
"name": "project",
"type": "directory",
"children": [
{
"path": "/Users/krushn/project/dir1",
"name": "dir1",
"type": "directory",
"children": [
{
"path": "/Users/krushn/project/dir1/file1.txt",
"name": "file1.txt",
"type": "file"
}
]
},
{
"path": "/Users/krushn/project/file2.txt",
"name": "file2.txt",
"type": "file"
}
]
}This should allow us to extend the model in the future. Also, we should have a flag for outputting this stucture as a JSON file. |
|
@yashpss01 will you work on it, or should I take it up |
|
@vedpawar2254 I will work upon this |
|
Raised PR #29 for this. Please review and provide feedback. @krushndayshmookh @vedpawar2254 . |
Description
This PR adds the functionality to use the flag
--as-jsonto get the treeview of the folder structure in nested JSON object.Related Issue
Fixes issue #8
Testing
I have tested it on my local with different folder structures and its working fine.
Checklist
Please review this and suggest if something not behaving as expected , or else merge it under hacktoberfest.