copy/move: wrong directory update with the same name

[lly]

Steps to reproduce:
1) Left panel /home/ - has directory aaa1 with some files
2) Right panel /u00/ - also has directory aaa1 with some files
3) Try to copy(F5) directory aaa1 from Left panel to Right
4) Result - /u00/aaa1/aaa1 instead of updated content of /u00/aaa1

i.e. directory erroneously copies to one level deeper, move(F6) is also affected. This behavior absent in 4.7.0.6

[zaytsev]

This problem does not exist on 4.7.0.7. I think it was introduced in #1907.

[x905]

confirm, mc 4.7.3-26-g848c2ad bug still there

[andrew_b]

Yes, this bug was introduced in #1907.

The simplest possible solution: append file name only, not directory name.

[zaytsev]

Why not just check whether the directory already exists on the target panel or not and if it does, refrain from appending the directory name?

[ossi]

Replying to zaytsev:

Why not just check [...]

that sounds wrong. the ui should behave consistently, regardless of the contents of the target panel.

[zaytsev]

It will behave consistently in all cases apart from this very special case, in which I would still qualify my suggested behavior as consistent. The solution to not to append directory names at all is IMO more inconsistent than that.

An alternative solution to check at the copy time does not sound right to me. What if the user actually DID want to copy this directory inside the directory of the same name?

[ossi]

look at the posix rename api. it is absolutely unambiguous. the problem is, that it can represent only exactly one rename at a time. that's why the command line tools change their behavior depending on whether the target is an existing directory (which i don't like at all). but that still doesn't happen in the "ui".

[zaytsev]

In my understanding the consistent behavior is the one that is consistent with the command line tools everybody is used to. What other UI you are talking about with which mc should be consistent rather than with cp etc. I don't understand.

[ossi]

you know what the "ui" stands for, and the that a command line is also a ui, right?

anyway, i'm not sure any more what you were actually suggesting, as in fact you didn't say anywhere at which level you want to do the special-casing.

[x905]

please fix this bug - its bad by design
i cant use mc with that

[zaytsev]

x905, did you notice that people are discussing the way to solve the issue?

ossi, I suggested to make it work the way cp works right now. That is not to append the directory name by default whenever the target directory already exists. I am not sure what exactly you found inconsistent in this behavior.

[andrew_b]

Replying to x905:

please fix this bug - its bad by design
i cant use mc with that

If you build mc youself from git, temporary revert the 9b5a8dec333a3d9724623c83cc0e60a121b5cc76 commit.

[andrew_b]

To fix this bug and restore the status quo I propose revert the 9b5a8dec333a3d9724623c83cc0e60a121b5cc76 commit and reopen #1907.

[ossi]

zaytsev, that added zero new information. :)
the question is whether you want to have the different appending behavior already in the ui or somewhere at the vfs level (the latter would be in line with cp/mv behavior).

[zaytsev]

Hmmm... what's the point? I don't care in which ticket it will be fixed. Is it gonna be easier to backport?

All we need is to agree on a solution. Does anyone has any objections to my suggestion?

ossi: the vfs level is already in line with cp/mv behavior. It is just that #1907 introduced a new ui feature fro F5/F6. Now the name of the file or directory is automatically appended to the To: field, so in case if one wants to change it he don't have to retype, but just edit (this is what many tc guys were missing as a substitute for click and rename).

However, Ilia didn't take into account that if the target directory already exists, this feature will make mc copy the directory to a subdirectory in line with cp rules. So I suggest to not to append it automatically in this case, because its not equivalent to what this command is supposed to do.

Can you make yourself clear? What exactly you don't like and how do you want it to be fixed instead? Your comments are just confusing everybody.

[zaytsev]

To make it even MORE clear: this appending was assumed to be equivalent to the respective cp command, which turned out not to be the case for this edge case:

cp file /folder == cp file /folder/file, but
cp folder1 folder2 != cp folder1 folder2/folder1

[andrew_b]

Replying to zaytsev:

cp folder1 folder2 != cp folder1 folder2/folder1

The result of
cp -R folder1 folder2
is folder2/folder1

[andrew_b]

Replying to zaytsev:

Hmmm... what's the point?

We can fix this bug immediately. As a result we will not have the bug but we will have the unimplemented enhancement. We can discuss the correct way how to implement that in proper ticket.

[zaytsev]

As you wish, I don't care. I don't see the point anyway, because you not gonna release 4.7.4 just for this, are you? Those who are really suffering will revert the commit and rebuild from source anyway.

[x905]

yes, i see discussion ossi vs zaytsev )
i just want a solution, and i try it (brunch)
but i want old behavior back as default

[x905]

as for ticket #1907 - the solution may be a hot key to copy selected name to clipboard (is it exits ? i want it too), then in f5/f6 dialog just press "end" and paste name to edit

[slavazanko]

created branch 2276_cpmv_wrong_dest_dir:

• ae914737419e5f3221721a2554ed69deb1e47069

Review, please.

[slavazanko]

Well, this is really no right way to fix it. #1907 have broken thinks. Need to revert patch 9b5a8dec333a3d9724623c83cc0e60a121b5cc76 and reopen #1907.

[slavazanko]

New changeset for branch:

• 91b430758a98da4287c2d1d54249637efecbe13d (reverting 9b5a8dec333a3d9724623c83cc0e60a121b5cc76)

[slavazanko]

merge changeset:cb85a8b87e5150e18c838600d461f259e6a5685e