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

FileTree abstraction #11

Merged
merged 20 commits into from
Aug 23, 2023
Merged

FileTree abstraction #11

merged 20 commits into from
Aug 23, 2023

Conversation

Al12rs
Copy link
Contributor

@Al12rs Al12rs commented Aug 21, 2023

This PR adds abstractions for file trees, as detailed in this issue: Nexus-Mods/NexusMods.App#553

  • The IFileTree interface defines a set of common functionalities.
  • FileTreeNode<TPath, TValue> defines a generic implementation of IFileTree, where the contained paths are Type agnostic and where each File entry contains an associated generic TValue.

To allow FileTreeNode to be path agnostic a new IPath<TConcretePath> was added, with the intent have it be be implemented by AbsolutePath, RelativePath and GamePath.

The main objective of this new interface is to define a set of common functionality for path types, while still allowing these methods to return concrete path types instead of IPath.

The problem with using IPath directly are Boxing allocations, since all Path types are implemented through structs.

To avoid any boxing/unboxing, the code had to heavily rely on generics, making sure to always reference concrete types rather than interfaces.

A limited number of common methods where added to IPath<TConcretePath>, but this quickly revealed a number of inconsistencies between AbsolutePath and RelativePath.
Most of these come from the special handling that Root folders require.

It should be noted that trying to deal with them in an abstracted way was quite frustrating. Each type has its own set of methods, only some of them implemented on both types, without a common interface to define them and with some of them having different meanings or semantics.
All the common functionality is in PathHelpers, which only deals with strings and requires an OS object for each method, making using it very unwieldy.
There is an open issue for this here: #10

@codecov-commenter
Copy link

codecov-commenter commented Aug 21, 2023

Codecov Report

Patch coverage: 30.71% and project coverage change: -3.60% ⚠️

Comparison is base (dd77c8a) 75.96% compared to head (afc0de3) 72.37%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #11      +/-   ##
==========================================
- Coverage   75.96%   72.37%   -3.60%     
==========================================
  Files          34       35       +1     
  Lines        1577     1712     +135     
  Branches      263      286      +23     
==========================================
+ Hits         1198     1239      +41     
- Misses        326      420      +94     
  Partials       53       53              
Flag Coverage Δ
Linux 71.78% <30.71%> (-3.55%) ⬇️
Windows 71.78% <30.71%> (-3.55%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
src/NexusMods.Paths/FileTree/FileTreeNode.cs 0.00% <0.00%> (ø)
src/NexusMods.Paths/AbsolutePath.cs 89.28% <90.47%> (+0.86%) ⬆️
src/NexusMods.Paths/RelativePath.cs 89.88% <100.00%> (+6.55%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Al12rs
Copy link
Contributor Author

Al12rs commented Aug 21, 2023

Needs tests

@Al12rs Al12rs changed the title FileTree abstraction FileTree abstraction WIP Aug 21, 2023
@Al12rs Al12rs changed the title FileTree abstraction WIP FileTree abstraction Aug 22, 2023
@Al12rs Al12rs self-assigned this Aug 22, 2023
@Al12rs Al12rs requested a review from a team August 22, 2023 13:46
@Al12rs
Copy link
Contributor Author

Al12rs commented Aug 22, 2023

Tests are done, should be ready for review. Let me know of any functionality you might want added or different.

@Al12rs
Copy link
Contributor Author

Al12rs commented Aug 22, 2023

Additional functionality that MO2 has on its IFileTree abstraction:

  1. node.hasSuffix(RelativePath suffixPath)
  2. node.pathFrom(ancestorNode) retrieves the relative path from this node up to the passed ancestor node
  3. clone()
  4. Insert policy feature: determines what happens when user tries to add existing entries (Fail, Replace, Merge).
  5. bool Exists(path, bool IsFile): checks whether a path to a file or a folder exists in the tree.
  6. node.pathTo(descendentNode): retrieves the path from this node to a descendent one.
  7. startNode.walk(func (entryPath, entry)): breadth first iteration of the tree with a callback for each entry, with the path from startNode to the current entry.
  8. addFile()
  9. addDirectory()
  10. insert(tree)
  11. node.merge(tree)
  12. clear, erase, copy, move
  13. node.Remove(names) removes entries with those names the node
  14. We actually have the possibility to register callabacks on BeforeRemove(), BeforeInsert(), BeforeReplace()
  15. Ability to lazily populate the tree

Any of these that we would like to add to our version?

@halgari halgari merged commit b3eda87 into main Aug 23, 2023
3 checks passed
@halgari halgari deleted the feature-filetree branch August 23, 2023 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants