[vim/vim] Implement EVENT_DIRCHANGED and EVENT_DIRCHANGEDLOCAL and ... (#888)

166 views
Skip to first unread message

allen haim

unread,
Jun 27, 2016, 7:00:35 PM6/27/16
to vim/vim

... fire autocmds on :cd and :lcd family of commands.

This is intended to fix #868.

Note that it only responds to user-initiated changes (:cd, :lcd, and synonyms); are there other ways the working directory can change that we should look out for?

You can view, comment on, or merge this pull request online at:

  https://github.com/vim/vim/pull/888

Commit Summary

  • Implement EVENT_DIRCHANGED and EVENT_DIRCHANGEDLOCAL and fire autocmds on :cd and :lcd family of commands

File Changes

Patch Links:

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

toothpik

unread,
Jun 27, 2016, 8:10:58 PM6/27/16
to vim...@googlegroups.com
On Mon, Jun 27, 2016 at 04:00:20PM -0700, allen haim wrote:
> ... fire autocmds on :cd and :lcd family of commands.

> This is intended to fix #868.

> Note that it only responds to user-initiated changes (:cd, :lcd, and
> synonyms); are there other ways the working directory can change that
> we should look out for?

have you covered the case where a user has 'autochdir' set and edits a
file on a different path?

Allen Haim

unread,
Jun 28, 2016, 4:12:43 PM6/28/16
to vim_dev
Thanks, good point. I've updated it to fire on :cd and :lcd (and synonyms) and also on autochdir.

Does it look better now?

There is now a single event 'DirChanged' which fires on these events.

('DirChangedLocal' from the first version didn't seem so useful any more).

allen haim

unread,
Jul 12, 2016, 5:48:04 AM7/12/16
to vim/vim, vim-dev ML, Manual

I'd be happy to write unit tests for this, but would first like to know if the approach is reasonable.

—
You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub, .

Justin M. Keyes

unread,
Jul 13, 2016, 2:24:26 AM7/13/16
to vim/vim, vim-dev ML, Manual

In src/ex_docmd.c:

> @@ -8928,6 +8928,10 @@ ex_cd(exarg_T *eap)
>  	    /* Echo the new current directory if the command was typed. */
>  	    if (KeyTyped || p_verbose >= 5)
>  		ex_pwd(eap);
> +
> +#ifdef FEAT_AUTOCMD
> +	    apply_autocmds(EVENT_DIRCHANGED, new_dir, new_dir, FALSE, curbuf);

The event "pattern" should say "global" or "window". expand('<afile>') can be used to get the directory name itself (though even that seems useless, because getcwd() should serve that purpose anyway).

apply_autocmds(EVENT_DIRCHANGED, "global", new_dir, FALSE, curbuf);

Advantages:

  • removes the need for multiple event types.
  • future-proof for when tab-local directories might be added in the future (Neovim has that already)

—
You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub, .

Justin M. Keyes

unread,
Jul 13, 2016, 2:26:59 AM7/13/16
to vim/vim, vim-dev ML, Manual

In src/ex_docmd.c:

> @@ -8928,6 +8928,10 @@ ex_cd(exarg_T *eap)
>  	    /* Echo the new current directory if the command was typed. */
>  	    if (KeyTyped || p_verbose >= 5)
>  		ex_pwd(eap);
> +
> +#ifdef FEAT_AUTOCMD
> +	    apply_autocmds(EVENT_DIRCHANGED, new_dir, new_dir, FALSE, curbuf);

Though I see you already removed DirChangedLocal, it could be useful for scripts to listen to only window-local directory changes. Though I cannot think of why.

autocmd DirChanged window ...

—
You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub, .

allen haim

unread,
Jul 16, 2016, 3:10:22 PM7/16/16
to vim/vim, vim-dev ML, Manual

In src/ex_docmd.c:

> @@ -8928,6 +8928,10 @@ ex_cd(exarg_T *eap)
>  	    /* Echo the new current directory if the command was typed. */
>  	    if (KeyTyped || p_verbose >= 5)
>  		ex_pwd(eap);
> +
> +#ifdef FEAT_AUTOCMD
> +	    apply_autocmds(EVENT_DIRCHANGED, new_dir, new_dir, FALSE, curbuf);

Hi, I don't think I agree that the pattern should be a keyword like "window" or "global". I find it more consistent with the other autocommands if it is a filename glob. Users can use '*' to match everything. Maybe we can think of another way to implement multiple event types if that's desired?

I'm also thinking that the third argument () should maybe just be NULL, because it's also confusing if is suddenly a directory.

And, I'm wondering if it should also check if the directory actually changed. Now it fires on each cd command, even if it didn't.

—
You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub, .

Justin M. Keyes

unread,
Jul 16, 2016, 3:38:39 PM7/16/16
to vim/vim, vim-dev ML, Manual

In src/ex_docmd.c:

> @@ -8928,6 +8928,10 @@ ex_cd(exarg_T *eap)
>  	    /* Echo the new current directory if the command was typed. */
>  	    if (KeyTyped || p_verbose >= 5)
>  		ex_pwd(eap);
> +
> +#ifdef FEAT_AUTOCMD
> +	    apply_autocmds(EVENT_DIRCHANGED, new_dir, new_dir, FALSE, curbuf);

I find it more consistent with the other autocommands if it is a filename glob.

The pattern is not always a filename (e.g. FileType autocommand), and it would be useless for this use-case.

The autocmd pattern is the primary way of selecting a subset of event messages to listen to.

—
You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub, .

Bram Moolenaar

unread,
Aug 14, 2016, 2:11:31 PM8/14/16
to vim/vim, vim-dev ML, Manual

I agree with Justin, using "global" or "window" works better.
I'l dropping the patch from the todo list until that's done and we have tests.

—
You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub, .

allen haim

unread,
Aug 30, 2016, 8:42:25 AM8/30/16
to vim/vim, vim-dev ML, Push

@misterfish pushed 4 commits.

  • 2a083fb Use "global" and "window" as the "file" pattern of the DirChanged
  • e0b3609 Add feature guard around buffer allocation.
  • 52b4f29 Don't try to expand the filename during DirChanged autocommand.
  • 336063e Complain and return if the pattern for DirChanged autocommand is not

—
You are receiving this because you are subscribed to this thread.

View it on GitHub

allen haim

unread,
Aug 30, 2016, 8:43:54 AM8/30/16
to vim/vim, vim-dev ML, Push

@misterfish pushed 1 commit.

—
You are receiving this because you are subscribed to this thread.

View it on GitHub

allen haim

unread,
Aug 30, 2016, 8:48:00 AM8/30/16
to vim/vim, vim-dev ML, Push

@misterfish pushed 1 commit.

  • 0d1189a Update documentation for DirChanged autocommand.

—
You are receiving this because you are subscribed to this thread.

View it on GitHub

allen haim

unread,
Aug 30, 2016, 9:04:00 AM8/30/16
to vim/vim, vim-dev ML, Push

@misterfish pushed 2 commits.

  • f2c5e68 Fire 'window' version of DirChanged autocommand on autochdir.
  • fab17e2 Match exact string length when checking DirChanged pattern.

—
You are receiving this because you are subscribed to this thread.

View it on GitHub

allen haim

unread,
Aug 30, 2016, 2:43:03 PM8/30/16
to vim/vim, vim-dev ML, Push

@misterfish pushed 1 commit.

  • 08f21c5 Use s: prefix for helper functions in unit test.

—
You are receiving this because you are subscribed to this thread.

View it on GitHub

allen haim

unread,
Aug 30, 2016, 4:37:52 PM8/30/16
to vim/vim, vim-dev ML, Push

@misterfish pushed 1 commit.

  • 3d4dd6d Fix explicit casts and compiler warnings.

—
You are receiving this because you are subscribed to this thread.

View it on GitHub

allen haim

unread,
Aug 30, 2016, 5:24:00 PM8/30/16
to vim/vim, vim-dev ML, Push

@misterfish pushed 2 commits.

  • bd0e3cf Add DirLocal to syntax file for vim.
  • 3c11a91 Add unit tests for DirChanged events.

—
You are receiving this because you are subscribed to this thread.

View it on GitHub

allen haim

unread,
Aug 30, 2016, 6:02:45 PM8/30/16
to vim/vim, vim-dev ML, Push

@misterfish pushed 1 commit.

  • 64e3f26 Add (char_u *) cast to string literal.

—
You are receiving this because you are subscribed to this thread.

View it on GitHub

Cimbali

unread,
Jan 7, 2018, 4:53:29 AM1/7/18
to vim/vim, vim-dev ML, Manual

This could probably use some squashing, but is there any other reason it isn't merged yet?

Andy Massimino

unread,
Jan 8, 2018, 7:18:14 PM1/8/18
to vim/vim, vim-dev ML, Manual

I cleaned up this PR and made some small changes (removed unnecessary validation on autocmd pattern) andymass/vim@4e24de9 .

It differs from neovim's implementation since it does not set v:event. But their v:event seems completely useless since it just tells you getcwd() and global/window which is in <amatch>.

allen haim

unread,
Jan 9, 2018, 7:41:34 AM1/9/18
to vim/vim, vim-dev ML, Manual

Thanks for cleaning it up, Andy.

Cimbali

unread,
Jan 9, 2018, 10:29:54 AM1/9/18
to vim/vim, vim-dev ML, Manual

@misterfish If you can reset your branch to andymass/vim@4e24de9 and push it, that would update the PR. Otherwise a new one should be opened and this one closed I guess.

allen haim

unread,
Jan 9, 2018, 4:15:03 PM1/9/18
to vim/vim, vim-dev ML, Push

@misterfish pushed 2 commits.

—
You are receiving this because you are subscribed to this thread.

View it on GitHub

allen haim

unread,
Jan 9, 2018, 4:38:42 PM1/9/18
to vim/vim, vim-dev ML, Manual

Done.

Bram Moolenaar

unread,
Feb 3, 2018, 11:37:12 AM2/3/18
to vim/vim, vim-dev ML, Manual

Closed #888 via b7407d3.

Reply all
Reply to author
Forward
0 new messages