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: nested selection conflict detection #689

Merged
merged 15 commits into from
Feb 18, 2024
Merged

feat: nested selection conflict detection #689

merged 15 commits into from
Feb 18, 2024

Conversation

evpeople
Copy link
Contributor

@evpeople evpeople commented Feb 17, 2024

I implemented conflict path detection using HashMap. Using a prefix tree might also be a worthwhile option to consider.

Closes #688

@sxyazi
Copy link
Owner

sxyazi commented Feb 17, 2024

Thank you for the PR!

Could you also add an insert_many(&mut self, urls: &[&Url], parent: Url) method? This is very useful for bulk selecting (selecting all) files - all of which have the same parent path, so it only needs to be detected once to improve performance.

Then insert() can simply be a wrapper for insert_many() like this:

	pub fn insert(&mut self, url: &Url) -> bool {
            self.insert_many(&[url], url.parent_url())
        }

@evpeople
Copy link
Contributor Author

Ok, I will try it

@evpeople
Copy link
Contributor Author

Considering that the paths corresponding to insert_many all have the same parent directory,
I implemented insert_many by modifying the logic in the original insert method that checks the object being inserted to instead check the first object in insert_many.

I didn't use the parent variable. I tried some methods that involved the parent variable, but they were not successful in passing the tests.

@evpeople
Copy link
Contributor Author

To make the compilation pass, I added contains and get_inner methods. I am unsure if using the BTree inside inner within yazi-fm might lead to inconsistencies between HashMap and BTree.

@evpeople
Copy link
Contributor Author

The usage of Deref here has taught me a lot.

@evpeople
Copy link
Contributor Author

I have considered the issue of data overflow. Later, I realized that if the add operation is correct, then the count in the parent can safely be decremented to 0. If overflow occurs, it indicates the possibility of some concurrency-related modifications. At this point, the program should panic.

@sxyazi
Copy link
Owner

sxyazi commented Feb 18, 2024

Hi, I made some little changes, and I believe it's all good now.

Thank you very much for contributing to this feature, let me merge it!

@sxyazi sxyazi changed the title feat: Implemented conflict path detection using HashMap.#688 feat: nested selection conflict detection Feb 18, 2024
@sxyazi sxyazi merged commit c7bfdc5 into sxyazi:main Feb 18, 2024
5 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nested selection conflict detection
2 participants