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

Make -no-hooks default with execute-keys and evaluate-commands #1747

Closed
danr opened this issue Dec 6, 2017 · 4 comments
Closed

Make -no-hooks default with execute-keys and evaluate-commands #1747

danr opened this issue Dec 6, 2017 · 4 comments

Comments

@danr
Copy link
Contributor

danr commented Dec 6, 2017

This is a follow-up on #1692 where it was identified that hooks enabled being the default on exec and eval is a surprising default. Personally I typically add -no-hooks as often as I can remember to exec and eval and I can count without using any hands when I actually wanted to have them on.

This issue suggests that hooks should be off by default. They could be enabled with a -with-hooks flag.

@mawww
Copy link
Owner

mawww commented Dec 7, 2017

Yeah, I think such a change would make sense, but we need to experiment with it to see what it breaks.

@lenormf
Copy link
Contributor

lenormf commented Dec 7, 2017

Related #707.

@h-youhei
Copy link

How about hook -with-commands or hook -with-keys instead of eval -with-hooks.

I think it makes easy to handle side effect.

@mawww mawww closed this as completed in 9176de8 Apr 22, 2018
@mawww
Copy link
Owner

mawww commented May 7, 2018

I have been experimenting with this, and it is not as good as expected.

It seems the root of the problem is that we try to treat eval and exec the same way. But typically, we want hooks active in most eval, in particular any edit command should run with hooks, except when we are sure we do not want hooks to run. exec are often not expected to trigger hooks.

Also, some hooks really want to always run, such as the ones taking care of removing temporary folders when a fifo is closed. We do not want to leave temporary directories because a fifo was closed during a non-hooked eval. This is why the hook command gained a -always switch.

Finally, the initial issue would now be fixed because faces, and hence colorschemes, are now scoped, meaning those hooks would be less likely to interfere.

All that to say, I am still unsure what the best solution would be, but while disabling hooks by default felt like a nice idea, it proved to be unsufficient.

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

No branches or pull requests

4 participants