Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: notify when changing the current directory #406

Merged
merged 2 commits into from
Jun 9, 2024

Conversation

ro0gr
Copy link
Contributor

@ro0gr ro0gr commented Jun 5, 2024

Right now both actions.tcd and actions.cd seem to do their job silently. That made me think the ~ key is broken(#397).

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

@github-actions github-actions bot requested a review from stevearc June 5, 2024 10:02
@ro0gr
Copy link
Contributor Author

ro0gr commented Jun 5, 2024

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 ro0gr closed this Jun 5, 2024
@ro0gr
Copy link
Contributor Author

ro0gr commented Jun 5, 2024

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.

@ro0gr ro0gr reopened this Jun 5, 2024
@stevearc
Copy link
Owner

stevearc commented Jun 6, 2024

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,
  },
})

@stevearc stevearc closed this Jun 6, 2024
@ro0gr
Copy link
Contributor Author

ro0gr commented Jun 6, 2024

@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
Copy link
Contributor Author

ro0gr commented Jun 6, 2024

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
Copy link
Owner

stevearc commented Jun 9, 2024

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.

@stevearc stevearc reopened this Jun 9, 2024
@stevearc stevearc merged commit 18272ab into stevearc:master Jun 9, 2024
9 checks passed
@ro0gr
Copy link
Contributor Author

ro0gr commented Jun 9, 2024

Great! Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants