lambdalisue / vim-fern

🌿 General purpose asynchronous tree viewer written in Pure Vim script
MIT License
1.29k stars 49 forks source link

Renaming a directory in a deeply nested path while executing a rename on a file will create a new nested directory if it doesn't exist #150

Closed nickjj closed 4 years ago

nickjj commented 4 years ago

If you rename a file that's in a nested directory and you decide to rename one of the nested directories to a name that doesn't exist yet, it will create a new directory instead of renaming the existing directory and your renamed file (even if the file name didn't change) will get located there.

Reproducible steps on Vim 8.1 using the latest master commit as of this issue's date:

  1. Create a test file in a deeply nested directory:
mkdir -p /tmp/testfern/foo/bar/baz \
  && cd /tmp/testfern \
  && touch foo/bar/baz/hello
  1. Open Vim with vim .

  2. Open the hello file with :e foo/bar/baz/hello

  3. Toggle and reveal hello in fern

  4. Execute fern-action-move on hello

  5. Notice how fern is asking you to rename the hello file

  6. Rename foo to foo2 in the destination path and but the leave the file name alone

You will end up with tmp/testfern/foo2/bar/baz/hello and /tmp/testfern/foo/bar/baz will be empty. The file was moved to a new directory instead of the existing foo directory being renamed to foo2.

I'll admit, I'm not 100% on what the expected behavior should be here. Personally I feel like it should probably rename the existing directory to foo2 without creating a new one. It's worth bringing up that if both foo and foo2 directories exist beforehand it will move the file between them as expected.

bluz71 commented 4 years ago

This one could go both ways.

It may be legit for fern to state that renames/moves are bound to the tail of a path only.

Depends how complicated the internal logic is. Ideally we should want fern to have a simple internal implementation, which may sacrifice some corner edge cases. To-be-determined.

It may be worth finding out how vimfiler handles the same case, vimfiler being more similar to fern than NERDTree.

nickjj commented 4 years ago

It may be legit for fern to state that renames/moves are bound to the tail of a path only.

Perhaps but if the source and destination nested directories already exist then fern will move the file to that existing directory, which I think means it's already looking at other bits of the path beyond just the tail.

It's tricky because it operates like mv would work if both nested directories exist (which is nice), but if the destination nested directory doesn't exist, it drastically changes behavior by creating a new directory for you, when I think if you're working with a move / rename operation you would expect a change of the name in any part of the path, not a new copy -- especially since if you rename the file, it renames the file, not creates a new one.

bluz71 commented 4 years ago

Yes, I understand the confusion.

fern maybe could display a warning stating that rename/move effects only tail elements.

Note, I too prefer that intermediates be moved. But I also understand that it may complicate the internals, aka too hard. Maybe, maybe not.

Whatever vimfiler does with intermediate moves fern should do the same.

nickjj commented 4 years ago

fern maybe could display a warning stating that rename/move effects only tail elements.

I guess that's the thing. It already handles moving a file to intermediate directories. It's just the behavior is different based on the intermediate existing beforehand or not. When it exists it does a move operation, when it doesn't it creates a new directory.

bluz71 commented 4 years ago

Yes, the base issue should be resolved.....or the operation should not occur at all with a helpful informative message.

nickjj commented 4 years ago

Yep, I think as long as you can execute a rename / move on hello.txt in foo/bar/baz/hello.txt and it gets moved to foo2/bar/baz/hello.txt when both foo and foo2 already exist then we'll be in good shape.

That is super common behavior (moving a file from directory A to B) and in this case it requires manipulating an intermediate directory. This is how NT works and while I never tried other tools, I would guess they work the same. Otherwise you wouldn't be able to move files to a new directory.

Fortunately this specific issue is kind of an edge case in behavior consistency because if you wanted to rename foo to foo2 you would likely select that directory instead of a file that exists somewhere within there. Let's see what happens.

bluz71 commented 4 years ago

Yep, I think as long as you can execute a rename / move on hello.txt in foo/bar/baz/hello.txt and it gets moved to foo2/bar/baz/hello.txt when both foo and foo2 already exist then we'll be in good shape.

Agreed.

lambdalisue commented 4 years ago

This is basically the same as https://github.com/lambdalisue/fern.vim/issues/111 but this issue has more info thus I close the #111 and continue here.

I know vimfiler cannot handle it and NERDTree is out of scope (it doesn't have a feature similar to exrename/renamer right?) But this is a point why I could not believe exrename on vimfiler thus I'd like to find a way to make it stable.

I'm challenging it on https://github.com/lambdalisue/fern.vim/pull/141 👍

nickjj commented 4 years ago

When you say you're challenging it, do you mean you're working towards a solution that would rename A to B even if B doesn't exist when moving / renaming a file that exists either directly in A / B or somewhere down the directory path chain?

lambdalisue commented 4 years ago

I don't get the meaning but cases I'm trying to solve is listed on #141

nickjj commented 4 years ago

The meaning would be the original steps in this issue resulting in a rename, not creating a new directory.

bluz71 commented 4 years ago

Personally I don't believe there is an actual issue here.

A user should not execute a move operation on a leaf file but only for the intention of renaming an intermediate directory.

In the original case in the first post fern.vim works well if the move operation is done on the original foo directory, not the hello leaf file. Just move the cursor up to foo then do `move.

In that I get this message:

Move: /tmp/testfern/foo --> /tmp/testfern/foo

I then enter foo2 and all is good including the hello file now existing in a foo2 subdirectory.

This is documentation issue, move operation should only be with regards to the last component, not intermediates. If done on intermediates it becomes a copy operation; but not ideal, but not a deal-breaker.

If NERDTree behaves different allowing intermediate directory renames, that's fine, it is a very different implementation. fern.vim can rename directories, but only done directly on the directory itself.

My 2cents.

bluz71 commented 4 years ago

Personally I would just update the fern-action-move documentation noting not that intermediate directories should not be moved, such operation will result in a copy, only the tail component can actually be moved. If an intermediate directory requires a rename then move up to it and move it.

I would close then close this issue.

nickjj commented 4 years ago

Personally I would just update the fern-action-move documentation noting not that intermediate directories should not be moved

What's your definition of moved here?

I'm constantly doing things like editing foo/bar/baz.txt, revealing it in the draw and then wanting to move it to somewhere/bar/baz.txt where somewhere happens to exist already. NERDTree happily moves the file there, as does the mv command. It's the expected behavior. That's how fern works too as of today.

How would fern differentiate the intent of wanting to move baz.txt to a different directory, or that I'm renaming an intermediate directory? In both cases the file / leaf node doesn't get changed but an intermediate directory does change.

It just seems strange to me that a move operation works as a move operation in the above case, but then it changes to a half-copy + half-move operation when an intermediate directory doesn't exist beforehand. Because it makes a copy of the intermediate directory and then moves the file to it. It's very unexpected for that to happen. A move / rename operation should stick to only moving / renaming IMO.

chiefjester commented 4 years ago

Just chiming here, like @nickjj I move files a lot, what I love about Fern is using a buffer for moving files. On this buffer, I can use the full power of vim to edit the move command.

How would fern differentiate the intent of wanting to move baz.txt to a different directory, or that I'm renaming an intermediate directory? In both cases, the file/leaf node doesn't get changed but an intermediate directory does change.

I think it's more explicit if you move by folder. And like what @bluz71 is saying, I believe that's a better workflow IMHO. Because like you said, it's how mv works, and that's how most users will be accustomed to. Another reason is, depending on leaf node can produce undesire results, because there's so many if's to answer.

  1. if there are other files inside that folder, eg if /tmp/testfern/foo/bar/baz isn't empty, what happens?
  2. if it is empty, I guess you can delete it?
  3. Is this even a desired behavior?
  4. what if you just want to move the file, which is I think most people are more accustomed to?

So having a lot of if's makes the workflow more brittle because your intention and what the plugin does should be 1:1.

So what @bluz71 was suggesting seems to be the better approach. Because if you really wanted to move the folder, you would move the folder, not the leaf node, and you wouldn't care if there are existing files on that folder or not. That's just one less thing to think about.

Again this is file operations, rather than having multiple outcomes on a single command (does the user want to move the folder, does the user want to move the file only, etc.), making things more explicit is a better workflow.

It just seems strange to me that a move operation works as a move operation in the above case, but then it changes to a half-copy + half-move operation when an intermediate directory doesn't exist beforehand. Because it makes a copy of the intermediate directory and then moves the file to it. It's very unexpected for that to happen. A move/rename operation should stick to only moving/renaming IMO.

@lambdalisue I hope you correct me if I'm wrong here. But I believe under the hood, what the move command is doing is

  1. mkdir -p /newpath/
  2. mv file /newpath/

So there is no copy command being done on a move command, only a move command, and that it's more concerned to the leaf node rather than the directory path.

nickjj commented 4 years ago

I think it's more explicit if you move by folder

In the above example I'm moving a file tho, not a folder.

With mv you can do mv foo/bar/hello.txt example/bar/hello.txt when example already exists. The end result there is moving hello.txt to a different directory. In a fern context you would be operating on the file not the folder because chances are you revealed that file before issuing the move / rename command.

Are we all in agreement that this behavior should remain as it does today? Because without it, we can't ever move a file.

So there is no copy command being done on a move command

Sorry, yes I should have been more clear on that one. It doesn't copy the contents of the directory over. From an end result POV a new directory gets created. In my mind that correlated to a copy of the old directory instead of a move even though it technically isn't a copy.

chiefjester commented 4 years ago

In the above example I'm moving a file tho, not a folder.

Hi @nickjj, So just some clarifications, isn't that what Fern is doing right now? Isn't your issue is what happens if the example doesn't exist? Which, according to your request, should behave differently?

Because in that scenario, you'll bump to different if's

  1. What happens if the folder is empty?
  2. What happens if the folder is not empty?
  3. Should you just rename the folder to tmp/test/fern/foo2/bar/baz/hello? What happens if it has existing files?
  4. If you rename a folder, what happens to its subfolders, bar/baz/?

What I'm trying to get at is there shouldn't be any questions like this when you do an explicit command. The only magic that Fern is doing from native mv, is making a mkdir -p on the path if it doesn't exist.

Revisiting this:

In the above example, I'm moving a file tho, not a folder.

What is your end goal here? Do you want the original foo folder to get deleted? Because if it does, then you are moving the folder, even though your original intention is moving a file?

Anyway, that's how I was phrasing being explicit.

  1. if we want to move the folder, then we move the folder,
  2. if we want to move the file, then we only need to move the file, and not do other folder modifications.
nickjj commented 4 years ago

So just some clarifications, isn't that what Fern is doing right now? Isn't your issue is what happens if the example doesn't exist?

Yes and yes.

The only magic that Fern is doing from native mv, is making a mkdir -p on the path if it doesn't exist.

Right but this feels magical in a questionable way (IMO at least). If you perform a move operation, it's very unexpected that something new gets created.

Maybe I'm way off base here but I would expect the 2nd one to also be a move operation if any magic were to happen. AKA, the file would be moved and the directory that's being changed would move too.

What is your end goal here? Do you want the original foo folder to get deleted?

I wouldn't want the original foo folder deleted along with all of its contents, I would expect the existing directory that's being renamed to be renamed to whatever you put in as the new name. This way we have consistent behavior when both the directory does and does not exist when moving a file.

chiefjester commented 4 years ago

@nickjj , thanks for the clarification, I now get your point and where you coming from. It’s a choice between having more magic and being more explicit. I guess the question is which gives the user a better default?

Whatever we arrive at, we do need to clarify it in the documentation.

lambdalisue commented 4 years ago

First of all, I had misunderstood the issue. This issue is NOT what I'm trying to solve in #141. Sorry about that.

IMO, the behavior of current move action is simple and expectable as @bluz71 and @thisguychris said. Any intermediate directories are automatically created not only in move action but also in new-file, new-dir, copy, and rename. Thus the behavior is consistent at least in fern. So I'm not going to change it sorry.

What I'm trying to solve in #141 is the behavior of rename action which executes multiple renames in a single action and it cause some unexpectable behavior. For example, current fern cannot handle the following renames

# SRC    -> DST
alpha    -> beta
alpha/a  -> beta/A

while no alpha directory exists in filesystem after alpha -> beta rename had executed.

To solve that issue, above must be converted to the following operations

alpha   -> beta
beta/a  -> beta/A

141 try to solve this kind of renaming puzzle. It's quite challenging while Vim script is extremely slow thus I cannot use numerous algorithms. Additionally, the above operations should fail if alpha/A had already existed while users expects rename but overwrite existing file/directory in the filesystem.

lambdalisue commented 4 years ago

I'm OK to fix the documentation for move action to clarify the limitation but I have no idea how to explain this clearly in English thus PR for that is welcome.

bluz71 commented 4 years ago

The current documentation states:

*<Plug>(fern-action-move)*
    Open a prompt to ask a path and move a file/directory of the cursor
    node or marked node path(s) to the input path(s).
    Any intermediate directories of the destination will be created.
    The prompt will repeatedly open if multiple nodes has marked.

Since directory creation is already mentioned, I really don't think there is anything more to do.

chiefjester commented 4 years ago

@bluz71 how about adding this statement of yours?

fern maybe could display a warning stating that rename/move effects only tail elements.

Which makes it more explicit, that the operation is only on tail nodes, not the subdirectory or path. cc @lambdalisue

lambdalisue commented 4 years ago

I'm not sure but I feel the term "WARNING" is too strong for explaining that kind of behavior.

Well, I'm OK to fix the documentation to make it more clear while I'm not a native English speaker and I know there is a better choice of sentences or paragraphs to explain behaviors. However, I don't think it's a good idea to add notes every time when someone misunderstands the behavior. So probably I'll reject the #153 if there are no strong willings for that sentence.

lambdalisue commented 4 years ago

I also think there is anything more to do so close.