[RFC] warn on second call to the dispatch function #30
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
whitequark
reviewed
Feb 28, 2017
src/hooks.ml
Outdated
| One@ should@ not@ setup@ hooks@ several@ times,@ but@ rather@ \ | ||
| combine@ several@ dispatch@ functions@ explicitly@ \ | ||
| in@ a@ single@ dispatch@ call.@ This@ lets@ you@ declare@ \ | ||
| the@ horder@ between@ sub-hooks@ instead@ of@ relying@ \ |
|
@gasche I'd like to nominate this for inclusion in 0.11. |
gasche
force-pushed
the
no-double-dispatch
branch
from
ddc8ed5
to
87cbc7c
Compare
|
I rebased, used |
gasche
force-pushed
the
no-double-dispatch
branch
from
87cbc7c
to
75dc9da
Compare
|
@gasche can you rebase this on top of master, to trigger the build? |
gasche
force-pushed
the
no-double-dispatch
branch
from
75dc9da
to
de3256b
Compare
|
Should we merge this one ? |
|
Sounds reasonable. Let's |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is an embarrassing shortcoming of the plugin-tags work that calling the
dispatchfunction several times just silently overrides its behaviour. Forcing users to calldispatchexactly once may be the right interface to expose, but not saying anything if they make a mistake is bad usability.One approach would be to keep a queue of hooks, push each new hook, and call them in order at dispatch time. I decided not to do that, however, because that would encourage an imperative way to provide functionality, with each plugin doing its own
dispatchcall; this would be fairly fragile as (1) users would have little control over the ordering on plugin rules and (2) they would have to jump through hoops to make sure the plugins are really linked, otherwise their effect would get silently dropped.By forcing that there is at most one dispatch call, we in fact enforce a pure, declarative interface between plugin providers and plugin users. This has worked out rather well in practice (despite the subpar usability of the current code), see js_of_ocaml ocamlbuild plugin for example.
This PR keeps this current interface of having a single dispatch call, but adds a warning each time a previously installed hook is discarded. (The formatted warning text is rather painful to read, so I will probably convert it to one of Daniel's Bünzli nice formatting functions instead.)