Ticket #3093 (closed enhancement: fixed)

Opened 3 years ago

Last modified 3 years ago

Keep symlinks in cwd at startup (based on $PWD)

Reported by: egmont Owned by: andrew_b
Priority: major Milestone: 4.8.12
Component: mc-core Version: 4.8.10
Keywords: Cc:
Blocked By: Blocking:
Branch state: merged Votes for changeset: committed-master

Description

If you navigate in your shell to a directory containing symlinks and then start mc, mc will show the canonical path instead. It would be nice to make it show the directory with the symlinks.

Example: in your shell execute these:

user:~$ mkdir -p /tmp/a/b /tmp/x ; ln -s /tmp/a/b /tmp/x/y
user:~$ cd /tmp/x/y
user:/tmp/x/y$ mc

In mc you'll find yourself in /tmp/a/b, though it'd be nicer to see /tmp/x/y at the top, and correspondingly navigating to the parent would take you to /tmp/x.

If you start bash or zsh from /tmp/x/y, the new instance will start displaying the working directory as such. They do this via the PWD env variable. On one hand, they set and maintain PWD to point to the current directory, using the path as specified by the user (possibly containing symbolic links). On the other hand, they check its value at startup. If $PWD points to the same physical directory as the actual working directory then they use this value. If $PWD points somewhere else then it's simply ignored (so it's a hint only as to which symlinks to use to get to the working directory, but never alters the actual cwd).

It would be nice if mc also did the same at startup. Relative directories specified in the command line should be applied after possibly replacing the canonical cwd with $PWD. This way for example

user:/tmp/x/y$ mc . ..

would open the two panels in /tmp/x/y and /tmp/x instead of /tmp/a/b and /tmp/a (whereas /tmp/x is actually a different directory than /tmp/a).

When mc-wrapper.sh is used, this already works when leaving mc: the shell will change to mc's last directory, possibly containing symlinks. This feature would similary fix the "other end" of the story so you'd never get the symlinks automatically resolved while starting and exiting mc.

Attachments

mc-4.8.10-initial-dir-from-PWD.patch (3.3 KB) - added by egmont 3 years ago.
Patch that implements this feature
mc-initial-dir-from-PWD-part2.patch (2.0 KB) - added by egmont 3 years ago.
fix segfault

Change History

comment:1 Changed 3 years ago by egmont

Of course the behavior should be subject to the "Cd follows links" config option.

comment:2 Changed 3 years ago by egmont

Patch attached. Unfortunately it segfaults if your config doesn't have an "other_dir" option.

Looks like we have a circular dependency. Setting up the vfs working directory requires the setup to be loaded, because the behavior depends on the "cd follows links" option, as per comment 1. But loading the setup requires the vfs working directory to be initialized, in case other_dir is a relative one which needs to be resolved. Haha...

Anyway, applying the changes to vfs.c only should work stable, although the behavior is undesired with "cd follows links" turned off.

Changed 3 years ago by egmont

Patch that implements this feature

comment:3 Changed 3 years ago by egmont

I've replaced the broken patch with a fixed one.

setup.c contained a bit of code that was IMO way beyond the responsibility of that file (reading-writing the config), it was doing further heavy-weight processing on one of the values. I've moved that to main.c to resolve the dependency loop.

Please test it, and consider applying. Thanks!

comment:4 Changed 3 years ago by egmont

Side note: get_current_dir_name() (http://www.gnu.org/software/libc/manual/html_node/Working-Directory.html) does the same with the PWD, which implies that for apps similar to MC this is indeed the desired behavior. Unfortunately this method is glibc-specific, and surprisingly Glib doesn't have an equivalent method.

comment:5 Changed 3 years ago by egmont

Friendly ping :)

I've been using MC with this patch ever since I filed the report, and it works nicely. It's not the feature that will save the world, but it's about paying attention to a detail that makes starting mc more convenient.

Could you please consider applying it? Thanks in advance!

comment:6 Changed 3 years ago by andrew_b

We're going to release 4.8.11 at this week. This patch will be applied after release. Thanks!

comment:7 Changed 3 years ago by andrew_b

  • Status changed from new to accepted
  • Owner set to andrew_b
  • Votes for changeset set to andrew_b
  • Branch state changed from no branch to on review
  • Milestone changed from Future Releases to 4.8.12

Branch: 3093_keep_symlink_cwd_startup
changeset:feae3e18def4a7beb321e1bfec5afc8c7f1d96f5

comment:8 Changed 3 years ago by slavazanko

  • Votes for changeset changed from andrew_b to andrew_b slavazanko

comment:9 Changed 3 years ago by slavazanko

  • Branch state changed from on review to approved

comment:10 Changed 3 years ago by andrew_b

  • Status changed from accepted to testing
  • Votes for changeset changed from andrew_b slavazanko to committed-master
  • Resolution set to fixed
  • Branch state changed from approved to merged

Merged to master: [18dcddf16b799351181d4a68b619a945e68461a1].

git log --pretty=oneline 56b76ce..18dcddf

comment:11 Changed 3 years ago by andrew_b

  • Status changed from testing to closed

comment:12 Changed 3 years ago by egmont

  • Status changed from closed to reopened
  • Resolution fixed deleted

Houston, we have a problem. "mcedit relative_path" segfaults.

Changed 3 years ago by egmont

fix segfault

comment:13 Changed 3 years ago by andrew_b

  • Votes for changeset changed from committed-master to andrew_b
  • Branch state changed from merged to on review

Patch applied with some addition: free resources (allocated in load setup()) if mc_setup_by_args() returns FALSE.

Branch: 3093_keep_symlink_cwd_startup_fix
changeset:56bff66b139ee3ac9fefc63a4708748eb9ccec34

comment:14 Changed 3 years ago by egmont

Thanks a lot! :)

comment:15 Changed 3 years ago by slavazanko

  • Votes for changeset changed from andrew_b to andrew_b slavazanko
  • Branch state changed from on review to approved

comment:16 Changed 3 years ago by andrew_b

  • Status changed from reopened to closed
  • Votes for changeset changed from andrew_b slavazanko to committed-master
  • Resolution set to fixed
  • Branch state changed from approved to merged
Note: See TracTickets for help on using tickets.