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

Protection from copying a directory into itself #894

Closed
1 task
DanViTrinh opened this issue Apr 10, 2024 · 4 comments · Fixed by #925
Closed
1 task

Protection from copying a directory into itself #894

DanViTrinh opened this issue Apr 10, 2024 · 4 comments · Fixed by #925
Labels
feature New feature request good first issue Good for newcomers

Comments

@DanViTrinh
Copy link

Please describe the problem you're trying to solve

240410_16h23m30s_screenshot

cp has protection against copying a directory into itself.

240410_16h23m56s_screenshot

This is not present in yazi resulting in an operation running forever.
Of course you could just quit yazi, so it is not an urgent feature, just a nice addition to yazi.

Would you be willing to contribute this feature?

  • Yes, I'll give it a shot

Describe the solution you'd like

A warning message when you are about to copy a directory into itself.

Additional context

No response

@DanViTrinh DanViTrinh added the feature New feature request label Apr 10, 2024
@sxyazi
Copy link
Owner

sxyazi commented Apr 10, 2024

Hey, you can open the task manager by pressing the w key, then press x to cancel the failed task, keep the failed tasks in the task manager and display a red progress bar at the bottom to indicate to the user that it's an intentional design.

I don't think notifications are suitable for this scenario. For example, if a user copies 100 files and 30 of them fail, constantly popping up 30 notifications would be annoying. So I decided to just keep them in the task manager and use the red progress bar to quietly inform the user.


Regarding adding protection checks, it would be nice to have and this would make nested copying/moving more secure.

Anyone want to give it a try? I think file_cut() and file_copy() are the only two places that need to be changed:

pub fn file_cut(&self, from: Url, mut to: Url, force: bool) {

pub fn file_copy(&self, from: Url, mut to: Url, force: bool, follow: bool) {

@sxyazi sxyazi added the good first issue Good for newcomers label Apr 10, 2024
@alexandruoprinca
Copy link

@sxyazi There is already some similar code preventing the file to be copied/cut onto itself:

debug!("file_copy: same file, skipping {:?}", to);

Shouldn't the protection check be added there as well, so in that case it doesn't even reach the scheduler?
Assuming you still want it to be in the scheduler, do you think that just checking and exiting earlier is enough?

@sxyazi
Copy link
Owner

sxyazi commented Apr 11, 2024

Hey @alexandruoprinca, this is different. When the user moves/copies A to A, i.e., A -> A, Yazi assumes that the operation has been successful because nothing (and should not) change, so there is no need to give the user an error message.

However, the second image provided by OP is a process of A -> A/A, which leads to an infinite loop of A -> A/A/A/....

Copy link

I'm going to lock this issue because it has been closed for 30 days. ⏳
This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New feature request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants