magit/magit — comment on commit d4a3dff143efbecd62f86b2bf2b307a74a6473cd
File:  line 0
By DarwinAwardWinner:

Well, if you replace `(mapcar ...)` with `nil` in my solution, it becomes identical to your solution, except that as you say, the user can remove the hook if they so desire. My solution as is is a bit more conservative, since it tries to run all the hook functions and just ignores any errors they may throw. But like I said, I haven't seen any function yet that I would really want to keep in `before-save-hook` for the rebase buffer, so just setting it to nil is probably fine. I vote in favor of doing it in a hook so that the user can remove the function from the hook if they don't want it, but I don't have a strong preference between the "set it to nil" solution versus the "ignore-errors" solution. Probably best to choose the nil solution because it is simpler.

In any case, neither of our solutions account for the user shooting themself in the foot by adding a buffer-modifying function to the buffer-local value of `before-save-hook` in an existing read-only buffer. Only the `rebase-mode-server-edit` function has any hope of addressing that. Note that adding a function to the global value of `before-save-hook` after the rebase buffer is created shouldn't affect the rebase buffer's local value, so that is not a problem.