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: imrpovements to file tree #126

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

32teeth
Copy link
Member

@32teeth 32teeth commented Oct 9, 2024

Problem

The original file tree conversion logic used in the system was inefficient and had some issues handling both modified and deleted file paths. There were multiple cases where folders were incorrectly marked as deleted if only some children were deleted. Additionally, there was a lack of deduplication in handling file paths, which caused unnecessary redundancy. There was also no way to associate actions or details with the nodes (files or folders) in the file tree.

Solution

  1. Deduplication:

    • We introduced deduplication for both modified and deleted file paths. This ensures that no redundant file nodes or folder nodes are added to the tree.
  2. Folder Deletion Logic:

    • A recursive function (markFoldersAsDeletedIfApplicable) was introduced. This function walks through the folder nodes and checks if all their children are deleted. If all children are deleted, the folder is also marked as deleted. This prevents folders from being incorrectly marked as deleted when some files are still present.
  3. Handling actions and details:

    • We added the ability to associate actions and details with each file node, allowing users to perform actions like rejecting changes or showing file-specific metadata.
  4. Efficient Tree Construction:

    • We refactored the tree construction logic to be more efficient. We now use helper functions like findOrCreateFolderNode and addFileNode to handle files and folders dynamically during the tree creation process.
  5. File Path Handling:

    • The function now properly handles file paths that start with ./, ensuring that root-level files are correctly placed without unnecessary prefixing in their file paths.

Files Changed:

  1. fileListToTree function:

    • Refactored to implement better tree handling and deduplication.
    • Mark folders as deleted if all children are deleted.
  2. exampleFileListChatItem:

    • A sample structure demonstrating how actions and file paths are associated with specific files, and how deleted files are marked accordingly.

Testing:

  • Added a comprehensive test suite to ensure all edge cases are handled correctly:
    • Deduplication of file paths.
    • Correct marking of folders as deleted based on their children.
    • Correct handling of modified and deleted files, ensuring the final tree structure is accurate.
Screenshot 2024-10-09 at 1 31 55 PM

Warning

npm run dev && husky break things for module import

(node:66958) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
(Use `node --trace-warnings ...` to show where the warning was created)
/../../example/dev.js:1
import open from 'open';
^^^^^^

SyntaxError: Cannot use import statement outside a module
    at internalCompileFunction (node:internal/vm:76:18)
    at wrapSafe (node:internal/modules/cjs/loader:1283:20)
    at Module._compile (node:internal/modules/cjs/loader:1328:27)
    at Module._extensions..js (node:internal/modules/cjs/loader:1422:10)
    at Module.load (node:internal/modules/cjs/loader:1203:32)
    at Module._load (node:internal/modules/cjs/loader:1019:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:128:12)
    at node:internal/main/run_main_module:28:49

Note

Add dynamic import, easy clean up

// Dynamic import handling
const openFile = async () => {
    try {
        // Use dynamic import to load the ES module
        const open = (await import('open')).default;
        await open('./dist/index.html');
    } catch (err) {
        console.error('Error opening file:', err);
    }
};

openFile();

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@32teeth 32teeth requested a review from a team as a code owner October 9, 2024 17:26
@32teeth
Copy link
Member Author

32teeth commented Oct 22, 2024

Stale, has merge conflicts. Will review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants