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.

Sign up for GitHub

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

Jump to bottom

feat: add new move, trash, and delete event kinds to DDS #880

Merged
merged 13 commits into from
Apr 10, 2024
Merged

feat: add new move, trash, and delete event kinds to DDS #880

merged 13 commits into from
Apr 10, 2024

Conversation

mikavilpas
Copy link
Contributor

Previously, only the rename command sent rename events. Now a cut and paste operation also sends events.

Demo:

yazi-rename.mov

@sxyazi
Copy link
Owner

sxyazi commented Apr 6, 2024

Thanks for the PR!

Could you create a new event type move for it and add the code to that event?

@mikavilpas
Copy link
Contributor Author

Sure!

@mikavilpas mikavilpas force-pushed the cut-and-paste-sends-rename-events branch from 42ecaec to edb2840 Compare April 6, 2024 17:56
@mikavilpas
Copy link
Contributor Author

Okay, there is now a new move event instead.

image

I don't know how to test the lua side of things yet. I copied the code from the rename events so I guess it might work without any changes 🙂

sxyazi
sxyazi reviewed Apr 6, 2024
View reviewed changes
yazi-core/src/manager/commands/paste.rs Outdated
@@ -21,6 +22,12 @@ impl Manager {
if self.yanked.cut {
tasks.file_cut(&src, dest, opt.force);

src.iter().for_each(|source_file| {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be moved to the scheduler,

yazi/yazi-scheduler/src/file/file.rs

Line 28 in 357f87a

FileOp::Paste(mut task) => {
, as moving is not immediately completed (for example, when it involves moving across mount points). Some files may fail to move due to permissions or because the mount point is full, and I think such cases should not be reported as events.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea. I tried implementing it but ran into some issues and could use some help.

Issue 1

In the scheduler I don't seem to have access to the current tab though. The tab is present in the rename event, and I think it would need some refactoring to get access to the current tab.

Issue 2

Publishing events from the scheduler doesn't cause the events to be visible for me. I pushed the commit 410bf4a as a work-in-progress commit to show what I tried.

I could not figure this out. Any ideas on what could be done?

sxyazi
sxyazi reviewed Apr 6, 2024
View reviewed changes
yazi-dds/src/body/mod.rs Outdated Show resolved Hide resolved
sxyazi
sxyazi reviewed Apr 6, 2024
View reviewed changes
yazi-dds/src/payload.rs Outdated Show resolved Hide resolved
@sxyazi
Copy link
Owner

sxyazi commented Apr 7, 2024

Thanks for the work you've done so far; I'll finish the remaining part.

@mikavilpas
Copy link
Contributor Author

Thanks, I appreciate that!

I also tried to add support for a new deleted event which I could use to detect files that were deleted in yazi in neovim.

I kinda ran into the same issue - it seemed like it's possible to request the scheduler to delete files, but I couldn't see if it's possible to know if the request has been successfully completed or not.

Maybe this is a feature that's not implemented yet? It could also be that I missed it.

mikavilpas and others added 4 commits April 9, 2024 01:00
@mikavilpas @sxyazi
feat: cut and paste (x -> p) sends new "move" events
0bc5529
@mikavilpas @sxyazi
refactor: reorder payload types
adeaf0d 
Co-authored-by: 三咲雅 · Misaki Masa <sxyazi@gmail.com>
@mikavilpas @sxyazi
refactor: rename move_event.rs to move_.rs
872a6a4
@mikavilpas @sxyazi
WIP: refactor: send the move events after moving is successful
63b20ba 
NOTE: this removes the "tab" key from the event, as the tab is not
currently passed to the scheduler and it seemed like a bigger change to
do right now.
@sxyazi sxyazi force-pushed the cut-and-paste-sends-rename-events branch from 410bf4a to 63b20ba Compare April 8, 2024 17:00
@sxyazi
Copy link
Owner

sxyazi commented Apr 10, 2024

Hi, I think I've completed the required work. I tested it by using yazi --local-events=move,delete and got the expected output:

CleanShot 2024-04-10 at 08 57 44

@sxyazi sxyazi changed the title feat: cut and paste (x -> p) sends rename events feat: add new move and delete event kinds to DDS Apr 10, 2024
@mikavilpas
Copy link
Contributor Author

Yeah it seems to work! As a suggestion, I also added sending a delete event when a file is trashed as that seemed to not emit anything for me.

sxyazi
sxyazi reviewed Apr 10, 2024
View reviewed changes
yazi-scheduler/src/scheduler.rs Outdated
@@ -174,7 +174,8 @@ impl Scheduler {
let file = self.file.clone();
_ = self.micro.try_send(
async move {
file.trash(FileOpTrash { id, target, length: 0 }).await.ok();
file.trash(FileOpTrash { id, target: target.clone(), length: 0 }).await.ok();
Pump::push_delete(target);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like adding trash to delete because they're two different concepts, I think it deserves a new trash event kind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll add a new event kind. It should be much simpler after the latest changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok it's done now and seems to work
image

@sxyazi
Copy link
Owner

sxyazi commented Apr 10, 2024

Thanks, let me do some minor refactors

@sxyazi sxyazi changed the title feat: add new move and delete event kinds to DDS feat: add new move, trash, and delete event kinds to DDS Apr 10, 2024
@sxyazi
Copy link
Owner

sxyazi commented Apr 10, 2024

All looks good to me now, let me merge it now.

Thank you so much for contributing to this feature!

@sxyazi sxyazi merged commit 3881341 into sxyazi:main Apr 10, 2024
5 checks passed
@mikavilpas mikavilpas deleted the cut-and-paste-sends-rename-events branch April 10, 2024 17:09
mikavilpas added a commit to mikavilpas/yazi.nvim that referenced this pull request Apr 11, 2024
@mikavilpas
feat: when files are moved in yazi, they stay in sync in nvim
d5fb7f7 
sxyazi/yazi#880
@mikavilpas mikavilpas mentioned this pull request Apr 11, 2024
Merged
mikavilpas added a commit to mikavilpas/yazi.nvim that referenced this pull request Apr 11, 2024
@mikavilpas
feat: when files are moved in yazi, they stay in sync in nvim
0904cdd 
sxyazi/yazi#880
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sxyazi sxyazi sxyazi left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mikavilpas @sxyazi