jun7 / rox-filer

ROX file manager
24 stars 6 forks source link

very long relative links #104

Closed step- closed 8 years ago

step- commented 8 years ago

Have you noticed that this rox can create very long relative links? It doesn't happen all the time. Just a minute ago I jumped into a directory, say "DIR", then dragged and dropped a file onto the up arrow toolbar icon and selected "relative link". The resulting link file should target "DIR/file" but instead it targets something like "../../../../parent-of/parent-of/..../DIR/file". So it goes way up in the file tree then it comes back down. I suspect it does this because it calculates the target path based on the canonical path of "file". It doesn't seem to use a more useful - IMO - ln -s command with the shortest possible target path. What do you think?

jun7 commented 8 years ago

I changed making of symlink because rox had make symlink using sympath -> sympath. It makes wrong link because symlink uses realpath as from. So there are two choice which realpath -> realpath or realpath -> sympath. I picked realpath -> sympath, because rox uses sympath for settings. So we can have different settings on a dir by use of symlinks. Also we can switch link by changing of symlink in pathway. Is that you say this thing?

step- commented 8 years ago

What I'm,saying is that it isn't good that this rox makes a symlink ../../../../parent-of/parent-of/..../DIR/file when it would be perfectly all right to make a symlink DIR/file. It isn't good to make a long path when a shorter path suffices, I think. The reason is that the long path is less clear and less portable than the short path. The long path involves directory nodes that you might want to rename in the future. The short path doesn't involve unnecessary directory names. It's better.

You say you changed the making of symlinks because the standard rox made wrong links. I have never seen that happening with the standard rox. I'm not saying that it couldn't happen, just saying that I haven't seen it. Can you please give me a concrete example of wrong links with the real rox, so I can try it here with Puppy Linux rox?

jun7 commented 8 years ago

1:/A/B/C 2:/symB/C 3:/symB/symA/symC <-make this `symC' using 2.

jun7 commented 8 years ago

I find adding choice of realpath in my todo list ;P But it is very low priority because I already added middle button assign what to move realpath's parent to the Up button.

jun7 commented 8 years ago

Ah, "So we can have different settings on a dir by use of symlinks" is wrong.

step- commented 8 years ago

57f77d1 is good because it seems to fix the issue of standard rox filer making wrong links. Thank you.

Below I have documented the before/after test case of standard rox files making wrong links, so that issue can be reproduced more easily in the future. However, 57f77d1 isn't addressing #104, which is the issue of making unnecessarily long target paths.

Show that standard rox creates wrong link in some cases:

# rm -fr A _B
# mkdir -p A/B/C
# ln -fsT A/B _B
# ln -fsT ../../A A/B/_A
# tree -F A _B
A
└── B/
    ├── _A -> ../../A/
    └── C/
_B
├── _A -> ../../A/
└── C/

5 directories, 0 files
#

Now open _B in standard rox, open _B/_A in standard rox, drag C from _B to _B/_A and drop to make relative link -- renamed to _C

# tree -F A _B
A
├── B/
│   ├── _A -> ../../A/
│   └── C/
└── _C -> ../C         <--- The new link points to a non-existent folder
_B
├── _A -> ../../A/
└── C/

5 directories, 1 file
# 

Show that commit 57f77d1 fixes #104:

With the same directories as before, delete _C and switch rox to jun7's commit 57f77d1. Open _B in jun7's rox, open _B/_A in jun7's rox, drag C from _B to _B/_A and drop to make relative link -- renamed to _C

# tree -F A _B
A
├── B/
│   ├── _A -> ../../A/
│   └── C/
└── _C -> ../_B/C/
_B
├── _A -> ../../A/
└── C/

6 directories, 0 files
# 

Continue testing. Open _B in jun7's rox, open A/B in jun7's rox, drag C from A/B to _B/_A and drop to make relative link -- renamed to _C2

# tree -F A _B
A
├── B/
│   ├── _A -> ../../A/
│   └── C/
├── _C -> ../_B/C/
└── _C2 -> B/C/
_B
├── _A -> ../../A/
└── C/

7 directories, 0 files
# 
jun7 commented 8 years ago

Thanks for the report.

step- commented 8 years ago

I have edited my previous comment. Please revisit #104 because it's a different issue than the one you fixed with 57f77d1. For my own use case, fixing #104 is very important. I don't think I will keep using this rox version if it makes symbolic links with unnecessarily long paths. I explained that all the extra path components become failure points when you start moving or renaming folders.

jun7 commented 8 years ago

Yes. so I added 932c3336bc437b8855f831db4f9980acc5613be3. Is it solve that?

step- commented 8 years ago

I don't know if 932c333 solves #104 and unfortunately I don't have time to build and test now, but later I will and let you know. However, in general I think a menu option isn't a good solution because It makes using rox more and more complicated. I can suggest a better solution, I think. Please think about it and perhaps you will like it.

So, the idea is that the best solution links the shortest path, and that standard rox links the shortest path when it doesn't link the wrong path. Therefore, try building the path the way you do in 57f77d1. Call that path P1 and keep it on hold. Then try building the path the way that standard rox does. Call that path P2 and keep it on hold. Now test:

if i-node(P2) is valid {
  if i-node(P2) == i-node(P1) {
    link path P2
  } else {
    error name clash /* error dialog ? or use P1 ? */
  }
} else {
  link path P1
}

What do you think?

jun7 commented 8 years ago

932c333's default is realpath -> realpath. It is enough for normal use and simple.

So don't warry about it, it behaves like standard rox without the bug.

step- commented 8 years ago

Yes, I compiled 932c333 and it's OK. Basically the top two menu entries for "link" look and work the same as standard rox but without the bug. That's good and it won't surprise current rox users. I would suggest renaming the other two entries to something that is more understandable and fully English. Here are some ideas:

However, it sounds technical and even confusing. Perhaps it's better to recognize that yours is a different method to compute the link target, and use a menu entry like:

It sounds less technical and it shouldn't confuse users because it doesn't try to tell them what the method does.