leo-arch / clifm

The shell-like, command line terminal file manager: simple, fast, extensible, and lightweight as hell.
https://github.com/leo-arch/clifm/wiki
GNU General Public License v2.0
1.29k stars 41 forks source link

cd on quit #237

Closed KenJean closed 11 months ago

KenJean commented 12 months ago

With cd_on_quit set up, quitting clifm always invokes cd_on_quit regardless of how I exit.

Steps to reproduce the behavior

  1. Source the provided cd_on_quit.sh in shell (in my case, .zshrc)
  2. Launch clifm
  3. Exiting by the Q, q or Ctrl-D commands always cd's to the last directory clifm was in.

Expected behavior cd on quit should only be invoked when exiting with 'Q' (capital q) command

System set-up

Hope I'm not misunderstanding how this feature is supposed to work.

⇒ KJ

leo-arch commented 12 months ago

Hi @KenJean. Thanks for reporting!

Yes, you're right. Should be working as expected now. Please give it a try and let me know if it works for you too.

NOTE: You might want to update your cd_on_quit.sh function (or just remove the No directory specified line from you rc file, it doesn't make much sense now).

KenJean commented 12 months ago

You da man !!!! This is fantastic !!! ... not to mention a lightening fast response.

Thanks for the tip to update my cd_on_quit.sh. I have also added an if clause to the launch command thus:

if [ -n "$CLIFM" ]; then
  echo 'clifm already running'
  exit 1
else
  clifm "--cd-on-quit" "$@"
fi

This is to avoid absent-mindedly opening a subshell within a subshell within a subshell within ...

clifm is now the file manager/subshell of my dreams. Heretofore I was using nnn of which I am still very fond. But with nnn I had to fork it and hack into the code just to customise keymappings and display parameters (I need to have file sizes reported with resolution down to single bytes.)

Am still exploring the myriad of features in clifm and trying to figure out how I may take advantage of them.

Anyway, keep up the good work. I shall certainly sponsor you now and see if I can buy you a beer.

⇒ KJ

leo-arch commented 12 months ago

Thanks for your kind words @KenJean! I'm really glad to hear the good news.

Please do not hesitate to open a new issue if something comes up, even if it's only a suggestion or a feature request. Feedback is the life and water of these kind of projects: community is the key.

Btw, consider opening a PR adding your subshell check to cd_on_quit.sh.

leo-arch commented 12 months ago

Thanks for the beer @KenJean!

KenJean commented 12 months ago

I'm working on doing a pull request to submit my cd_on_quit.sh. Since I've never done one before, I'm having trouble figuring out how to do it properly. This is something I should learn how to do, so I don't consider it a hassle.

Incidentally, as it stands now, one has to delete any previous version of ".last" before invoking CliFM, otherwise CliFM will "cd on quit" to that previous ".last "if one just exits without changing directories.

⇒ KJ

On Sun, 16 Jul 2023 at 18:37, leo-arch @.***> wrote:

Thanks for the beer @KenJean https://github.com/KenJean!

— Reply to this email directly, view it on GitHub https://github.com/leo-arch/clifm/issues/237#issuecomment-1637206578, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMPH6NKV6YIZAKE4FCDAFNDXQRUKXANCNFSM6AAAAAA2LWCW4I . You are receiving this because you were mentioned.Message ID: @.***>

leo-arch commented 12 months ago

I'm working on doing a pull request to submit my cd_on_quit.sh

Take your time, you'll figure it out. Github provides some simple instructions. Do it from the web interface, it's simpler than from the command line. Basically, you just clone the repo, make your changes, and then create a pull request.

one has to delete any previous version of ".last" before invoking CliFM

I'll take a look at it. I was just about to release 1.13. It'll has to wait I guess.

EDIT: Could you provide the exact steps to reproduce the last issue?

EDIT 2: Issue reproduced if exiting via Ctrl-d or F12 (neither Q nor q). Is that what you mean?

KenJean commented 12 months ago

I apologise for just mentioning something like this in passing without giving details.

To reproduce:

Now here is what I observe:

If I change directories during this session from within CliFM, then everything still functions as it should: i.e. Exiting with 'q' or Ctrl-D leaves me in the directory I started in. 'Q' exits in the 'new' .last directory. I assume this is because by changing directories, a new .last was written.

However, if I don't change directories from within CliFM, then a 'q' or Ctrl-D leaves me in the directory pointed to from the previous '.last', not the directory I started this session in.

The simple work-around right now is simply to delete any .last file before launching CliFM. Then everything always works as it should.

⇒ KJ

leo-arch commented 12 months ago

Still cannot reproduce the issue. Are you running the latest clifm version (1.13) for all you instances?

KenJean commented 12 months ago

Yes, 1.13.rc5

If you can't reproduce it, then maybe it's something at my end. Let me try this on different computers and see if it's only the one that's causing the problem.

⇒ KJ

leo-arch commented 12 months ago

The latest version is 1.13, not 1.13.rc5. Try cloning the current state and do a make install.

KenJean commented 12 months ago

Ah, OK then, I shall clone the proper version and test it out.

Meanwhile, just for you to know, in 1.13rc5, I can reproduce this on at least two machines,

... BUT ...

The problem only exists if I did the previous 'Q' when in a directory of a networked drive. The problem does NOT exist when only local drives are involved.

I don't know if this means anything, but there you are.

Hang on. Before I waste any more of your time, let me clone the proper version and see if this still happens.

... be right back.

⇒ KJ

KenJean commented 12 months ago

OK ... version 1.13 proper is now on my machines.

I'm afraid the problem still exists. But as I said, it only happens when I 'Q' into a networked drive.

The work-around, however, works in both 1.13 and 1.13rc5. All I do is put the erase .last command in my cd_on_quit.sh file above the actual launch command and everything works perfectly.

⇒ KJ

leo-arch commented 11 months ago

Ok. It's good to have a simple workaround, but, of course, it shouldn't be necessary in the first place. I'll keep trying to reproduce this.

Btw, by networked drives you mean samba, SSHFS, and the like, isn't it? What are you using exactly? Are you mounting the drives from outside clifm, or via the net command?

KenJean commented 11 months ago

Yes, I am with you on that. Something that needs to be fixed, should be fixed. I shall do anything I can to help aid in that cause.

"Networked drive." This was bloody vague of me, wasn't it?

To explain: My computers are hooked up together on an NFS network. I am a Linux purest so I don't have to indulge in Samba. The trick I use is autofs. I have a folder assigned to be the mount point. autofs mounts the required folder (machine) only when I access it. It will automatically dismount (umount) if there is no file activity for 30 seconds.

At first I thought this may be the culprit, but I have since tested everything while making sure the mount points remain valid all the way through. Mind you, there may still be something funky going on here, but that is beyond my competence to determine, at least for now.

⇒ KJ

leo-arch commented 11 months ago

Reproduced! It should be fixed tomorrow. In the worst scenario, I could just add the rm line to cd_on_quit.sh.

KenJean commented 11 months ago

As I said before: You Da Man !!!!!

Glad we can all go to bed tonight without this hanging over our heads.

⇒ KJ

leo-arch commented 11 months ago

Hey @KenJean, I was wrong: couldn't really reproduce the issue (I was rather using an old clifm version, which I keep for testing purposes). However, I just applied a bit of logic here:

  1. cd_on_quit.sh changes the shell working directory only if .last is found (and contains a valid directory) after running clifm.
  2. So, we just need to make sure that, after running clifm, .last only exists provided cd-on-quit is enabled and the exit command was Q (to prevent cd_on_quit.sh from changing the working directory using an old .last file).

Please try the latest version (make sure to remove the rm line from your cd_on_quit.sh) and keep me informed.

EDIT: To be really sure, you can try rewriting your cd_on_quit.sh to make it tell whether .last exists after running clifm:

c() {
    clifm --cd-on-quit "$@"

    FILE="${XDG_CONFIG_HOME:=${HOME}/.config}/clifm/.last"
    [ -f "$FILE" ] && printf "%s: File exists\n" "$FILE"

    dir="$(grep "^\*" "$FILE" 2>/dev/null | cut -d':' -f2)";
    if [ -d "$dir" ]; then
        cd -- "$dir" || return 1
    fi
}

If the File exists message is printed even when not using the Q command to quit, something is wrong.

KenJean commented 11 months ago

OK, I think you've got it. Everything works as it should. Until this version, CliFM would NOT delete the previous .last file no matter how I exited. Now this is fixed.

Thanks for the hard work.

By the way, not that this means anything to me, but I do notice that there is a duplicate .last file written to ~/.config/clifm/profiles/default. I suppose that's there for other reasons.

⇒ KJ

leo-arch commented 11 months ago

Great!

As to the duplicate file, don't worry, it's supposed to be there. ~/.config/clifm/.last is in fact a copy of ~/.config/clifm/profiles/PROFILE/.last intended to be used by the cd-on-quit function, simply because this function cannot know in advance which profile are you using (and the original .last file is stored in your profile directory, because each profile has an independent .last file).

KenJean commented 11 months ago

I thought there had to be a reason. OK then, shall we close this issue then ?

⇒ KJ

leo-arch commented 11 months ago

Yes, close it.

leo-arch commented 11 months ago

Btw, your comment about the duplicate file made me realize that I'm wasting resources: instead of creating a copy of the original file, I could create a simple symlink, which has several advantages:

  1. It makes clearer what the relation between the two files is
  2. It's less expensive: copying a whole file (no matter how small it is) requires more disk writes than creating a simple symlink
  3. There's no need to execute a shell command (in this case cp(1)): a system call, here symlinkat(2), will always be faster.
  4. There's no need to modify the current cd_on_quit.sh.

Just thinking aloud, don't worry. The point is that I will modify the current code to implement this change. I'll make a few more tests and let you know so that you can try it and make sure everything works as expected.

KenJean commented 11 months ago

Well, this is why it caught my attention in the first place. Whenever I see a duplicate file, I usually do an involuntary flinch. But this is not because I'm a good programmer. It's just my OCD neurons acting up.

Am eager to see what you come up with.

⇒ KJ

On Mon, 17 Jul 2023 at 16:29, leo-arch @.***> wrote:

Btw, your comment about the duplicate file made me realize that I'm wasting resources: instead of creating a copy of the original file, I could create a simple symlink, which has several advantages:

  1. It makes clearer what the relation between the two files is
  2. It's less expensive: copying a whole file (no matter how small it is) requires more disk writes than creating a simple symlink
  3. There's no need to execute a shell command (in this case cp(1)): a system call, here symlinkat(2), will always be faster.
  4. There no need to modify the current cd_on_quit.sh.

Just thinking aloud, don't worry. The point is that I will modify the current code to implement this change. I'll make a few more tests and let you know so that you can try it and make sure everything works as expected.

— Reply to this email directly, view it on GitHub https://github.com/leo-arch/clifm/issues/237#issuecomment-1638824556, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMPH6NM2BAINJZMKBNART7TXQWOCVANCNFSM6AAAAAA2LWCW4I . You are receiving this because you modified the open/close state.Message ID: @.***>

leo-arch commented 11 months ago

Done. Please give it a shot @KenJean.

KenJean commented 11 months ago

The highest compliment I think I can give is to say that I don't notice any difference. Everything I tried works as before - which is the what we want.

I was worried that I wasn't git cloning the proper new version, but upon checking, the .last file that caused us problems before is now indeed a symlink.

Bravo !

⇒ KJ