stevearc / oil.nvim

Neovim file explorer: edit your filesystem like a buffer
MIT License
3.83k stars 109 forks source link

feat: notify when changing the current directory #406

Closed ro0gr closed 3 months ago

ro0gr commented 4 months ago

Right now both actions.tcd and actions.cd seem to do their job silently. That made me think the ~ key is broken(https://github.com/stevearc/oil.nvim/issues/397).

We can improve UX by providing these actions with info notifications, so the keypresses provide some visual response to the user.

ro0gr commented 3 months ago

Hm.. Strange... I've just double-checked myself, and executed the tcd manually from the command line:

:lua vim.cmd({ cmd = "tcd", args = { "/Users" } })

This way my nvim just outputs the notification with the latest directory as expected:

Знімок екрана 2024-06-05 о 23 22 15

However, for some reason when exactly the same code executes from the oil codebase, it does not trigger the notification. And it does not even add an entry to the :messages 🤔

I'm not a vim/lua expert, and I still don't know why it behaves this way. But I'm pretty sure to say, that "fix" suggested in this PR is wrong. So closing this for now.

Would appreciate any ideas about why the actions.tcd is that silent.

ro0gr commented 3 months ago

OK, it seems the difference is the keymap. If I invoke cd as a part of a keymap it's being silenced. Seems like a nvim/vim thing.

Since the lack of any feedback for ~ is what I'd like to fix, I'm re-opening this.

stevearc commented 3 months ago

I had to think about this a good bit, but I don't think that we want this. The behavior of :cd to only print the pwd if the command was typed out seems to be intentional https://github.com/neovim/neovim/blob/cb6c0fda718e4503fc1bfc49a9fe92411f5f9005/src/nvim/ex_docmd.c#L5842-L5847 I dug through the git history and couldn't find any explicit reasoning; it's always been like that. If I had to guess, I would say it's because typing out :cd is treating it like a repl, whereas in a keymap users may not want the message history to be affected, and if they do it's easy enough to add a :pwd to the keymap.

For the same reason here, I think that the default action makes sense to keep minimal and not have any impact on message history. If you want to get feedback, you can easily add on to the builtins

require("oil").setup({
  keymaps = {
    ["`"] = function()
      require("oil.actions").tcd.callback()
      print(vim.fn.getcwd())
    end,
  },
})
ro0gr commented 3 months ago

@stevearc Thank you for taking this into consideration and for your response!

I agree it does probably make sense to silence the cd by default for keymaps. However, in scope of the oil, I think, it has another meaning.

Since the oil is supposed to provide just an "edit like a normal buffer" experience(that I like a lot!), I would expect the ~ key to work as it does for normal buffers.

However, it does not affect my text anymore, instead it changes my pwd and does it silently! This was completely unexpected for me as for the new user, and this looks like a spot of poor UX to me, which I still believe is worth to be improved.

ro0gr commented 3 months ago

To express my concern better, let's stop thinking about the ~ for a moment.

Let's assume any other normal mode key is overwritten by the oil silently. So now the x stops removing the char under cursor, and changes the working dir silently. Most likely the user would be confused by thinking the x just does not work, but in fact it does, but it does some other job behind the user.

Even if we ignore the newcomers possible scenario, I can still imagine people pressing the ~ by an accident(well, with qwerty it's highly unlikely but still), w/o realizing they've just changed the directory due to no visual response.

stevearc commented 3 months ago

That's a fair point. My main concern about this was that if you make the action notify, then there's no way for the end user to silence it if they don't want that notification, but there is a way for them to add the notification if they do want it. I think I have a way in mind to parameterize actions, so if I can get that into decent shape then that can take care of the problem.

I'll merge this in now and see about parameterizing it later.

ro0gr commented 3 months ago

Great! Thank you.