[PATCH] histedit: do not check experimental.histediting in extsetup

Jun Wu quark at fb.com
Thu Mar 10 12:11:03 EST 2016


On 03/10/2016 04:43 PM, Pierre-Yves David wrote:
> could we stick this in the histedit state?
>
> 1) the state is already passed around
> 2) If you start as ng, you need to keep being ng anyway.

The histeditstate is actually relying on the filesystem, which I think is
even worse than the top-level module variables.

> We could gathers them into an "actions" variable

But where do you put the "actions" variable?
If it is still a top-level module variable, I don't think the change is
worthwhile.
If it is to add a new parameter to every functions. As I said, it is a
huge because a lot of functions are involved, and consider about how
could other extensions add histedit actions.

> Given that the current code is using global variable to communication data
> between functions in the stacks I've an hard time to believe we can't do better

I assume you want to put "actions" (or whatever it called) to every functions.
I will argue the current design is actually "better":
1) Flexible (important)
    3rd extension can add histedit actions, which can be a deal breaker.
    For example, I can add an action called "spellcheck" to filter the message
    to a spellchecker and apply the result.
2) Short.
    The code is shorter and therefore needs less time to read (and write).

Think about histedit module as a closure and it's just maintaining its state
in the closure. I think it's pretty normal. Could you explain why this is bad?


More information about the Mercurial-devel mailing list