elves / elvish

Powerful scripting language & versatile interactive shell
https://elv.sh/
BSD 2-Clause "Simplified" License
5.65k stars 299 forks source link

Directory Elvish is launched from doesn't seem to be added to location history #1809

Open ukd1 opened 4 months ago

ukd1 commented 4 months ago

What happened, and what did you expect to happen?

Fresh install, just trying it out - trying the location history:

a> brew install elvish
a> elvish
a> cd ~/b
b> <cmd L> a

expect it to find a. Didn't for me.

Output of "elvish -version"

0.20.1

Code of Conduct

xiaq commented 4 months ago

Indeed feels like a bug to me, should be easy to fix.

ukd1 commented 4 months ago

Related, auto complete via tab (vs history cmd-L) needs extra key presses:

trying to get to ~/Sites/obsidian - I got:

Screenshot 2024-05-17 at 9 04 19 AM ...great

then "o" expecting to get the next directory inside Sites image

now I'm stuck - enter, tab, etc don't work. I have to delete, hit enter, and then o...

ukd1 commented 4 months ago

Indeed feels like a bug to me, should be easy to fix.

thanks, lmk if you would like any help!

krader1961 commented 4 months ago

It won't bother me too much if the behavior is changed but I do not expect starting an interactive Elvish process to update location history with the CWD when that process began. I only expect it to update location history when Elvish explicitly changes location; i.e., the CWD. If you do something like this do you really want the location history to reflect that the CWD was updated a hundred times in the location history even though the location wasn't changed by either shell?

elvish> range 100 | each {|_| elvish < $os:dev-tty > $os:dev-tty 2> $os:dev-tty }

Note that I'm assuming the user immediately exits the sub-shell by pressing Ctrl-D or typing exit.

ukd1 commented 4 months ago

@krader1961 - I'm just missing being able to do cd - and was told to checkout cmd+L, which fails for me with this use case.

krader1961 commented 4 months ago

So the thing about location history is it only includes directories you have actively made the CWD. The CWD when you start an interactive Elvish shell does not count as a deliberate change to a new directory. At least not unless the project owner decides to change the current, long standing, behavior. You should be able to get the behavior you expect by typing cd . or adding that to your rc.elv. You can also get a longer list of recent directories in location mode by adding a function like the following to your rc.elv:

# Support the equivalent of `cd -` in location mode (Ctrl-L) for the four most recently visited directories.
set before-chdir = (conj $before-chdir ^
  {|_| var prev = [(take 4 [(
            each {|dir| if (not (eq $pwd $dir)) { put $dir } } ^
                 $edit:location:pinned )] )]
    set edit:location:pinned = [$pwd $@prev]
  })
ukd1 commented 4 months ago

Thanks for the code @krader1961. I understand what you're saying, but fyi cd . does not work, that (as expected?) changes to the current directory.

Searching a little finds someone else asking for this in #434 and #1043, and a little in #1414. Closing, as this looks like a dead end of discussion as well as duplicative.

krader1961 commented 4 months ago

I understand what you're saying, but fyi cd . does not work, that (as expected?) changes to the current directory.

FWIW, My cd . trick works if you use the before-chdirhook customization in my previous comment. You are correct that the default behavior recognizes that the directory wasn't actually changed and therefore doesn't show the CWD as the most recent directory in location mode. Whether it should do so is an open question. It might even make sense to do so without requiring the user include cd . in their rc.elv but am ambivalent. Primarily because it's not clear to me that it is preferable location mode show the CWD when the shell was started as the "previous" location even though no location change has occurred.

What shouldn't happen is the equivalent of store:add-dir $pwd every time a new interactive Elvish shell is started. I may have misunderstood what change to the behavior you were proposing when you opened this issue.

xiaq commented 4 months ago

Huh, I hope my comment was clear that I think the behavior should be changed.

ukd1 commented 3 months ago

Huh, I hope my comment was clear that I think the behavior should be changed.

🙇🏼‍♂️