Skip to content

Feature request: Add ability to undo modifications #860

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

Open
ghostbuster91 opened this issue Apr 10, 2023 · 5 comments
Open

Feature request: Add ability to undo modifications #860

ghostbuster91 opened this issue Apr 10, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@ghostbuster91
Copy link
Contributor

ghostbuster91 commented Apr 10, 2023

This would probably be hard to do, but since I haven't found any issue for this I thought I would create one just to start a discussion.

I think that since there is an option to perform some modifications in the filesystem source like:

  • renaming
  • moving
  • deleting

It would be cool to have the ability to undo those modifications.
It seems that it could apply to a wider group of sources though for some it could be harder to do than for others (for example git source)

Of course there is no regular way to restore a file after it was rm, so for this one neo-tree would have to provide integration with something like trash-cli if I am not mistaken.

Reverting renaming and moving should be just a matter of tracking those changes.

I am not sure how to approach git source, but at the same time I don't think that it is that needed.

Wdyt?

@miversen33
Copy link
Collaborator

Tagging #473 and #202 as related

@miversen33 miversen33 added the enhancement New feature or request label Apr 10, 2023
@miversen33
Copy link
Collaborator

I've actually been digging into this a bit for the netman source as I feel it would be a good feature to have. But basically as you call out, its kinda hard lol.

IMO I think we wait for @cseickel on this one as they will certainly have thoughts. My best guess is that this will be tagged as "Ya that would be cool but we don't have the time to implement it right now". If you are open to attempting it, #473 is a closed PR attempt that you could go peek at to get a starting idea, and I am certain that any of the contributors or maintainers here would be more than happy to provide help along the way.

@nhat-vo
Copy link
Collaborator

nhat-vo commented Apr 10, 2023

@ghostbuster91 Great idea! I also think that it would be quite nice to revert the changes - I quite often make a typo in renaming/moving files, and it's quite inconvenient to have to recalculate the destination.

I think it should be fairly simple to add a LIFO queue to track the changes for each source. Each supporting command (e.g rename) will require a corresponding undo command (e.g undo_rename), and will add the relevant information needed for (undo_rename). The main problems are

  • These undo commands need good error handling. If you trash a file X and generate a new file with the same name through some other means, the restore routine needs to deal with this.
  • There might be bugs with how the state are handled. If a state is deleted, then all undo information will be deleted as well.

Still, this is a good idea and worth looking into. But as @miversen33 mentioned, this might take a while to get implemented, as there are still a lot of things going on at this moment. If you have any further suggestions, @ghostbuster91, feel free to let us know.

@ghostbuster91
Copy link
Contributor Author

I am very happy to see that you are so open to that idea.

I was thinking that it might be problematic prevent certain types of bugs/unintended behaviors due to model getting out of sync with the filestystem, but with file-system watcher that neotree already has it shouldn't be a problem. We could simply clear the queue if we detect any external changes to the filesystem. Obviously that could be more smart than that, we could only invalidate these elements that were affected by such change etc, but for the first iteration clearing the queue seems enough.

@cseickel
Copy link
Contributor

I would concur that this would be great to have, but also really difficult to get right. I would not personally attempt it because I just don't have the time and wouldn't want to support this feature. I would accept a PR that added these features, with the understanding that:

  1. I am going to enforce the highest standards on this PR, it has to be an excellent design that is implemented with a lot of defensive code.
  2. A good test suite is required for all functionality.
  3. Whoever adds this, I'm going to request that you stay on and help support it for a few months while the bugs shake out.

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

No branches or pull requests

4 participants