Patchwork [RFC] ui: provide official way to reset internal state per command

login
register
mail settings
Submitter Yuya Nishihara
Date June 12, 2016, 5:31 a.m.
Message ID <58d3b0dd7f03e318fb53.1465709475@mimosa>
Download mbox | patch
Permalink /patch/15470/
State Accepted
Headers show

Comments

Yuya Nishihara - June 12, 2016, 5:31 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1465708046 -32400
#      Sun Jun 12 14:07:26 2016 +0900
# Node ID 58d3b0dd7f03e318fb534103b39bf93aeb4c03b1
# Parent  c27dc3c31222c7f74331221a3d25566146feecac
ui: provide official way to reset internal state per command

This will allow us to clear in-memory password storage per runcommand().

I've updated commandserver to call resetstate() of both ui and repo.ui because
they may have different states in theory.
Jun Wu - June 12, 2016, 11:54 a.m.
This looks good to me. It does not conflict with my ui refactoring idea,
although the refactor won't come soon.

For ui, I'd prefer simper apis and wonder if we have alternative options.

Extensions can always wrap functions to do whatever they want. It's possible
to do extension-specific reset state by wrapping dispatch.runcommand. And
progbar.resetstate() looks like progress.py specific.

Consider the fact progress.py is in core, wrapping looks strange, I think
it's reasonable to just move progbar.resetstate() to dispatch.py.

Excerpts from Yuya Nishihara's message of 2016-06-12 14:31:15 +0900:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1465708046 -32400
> #      Sun Jun 12 14:07:26 2016 +0900
> # Node ID 58d3b0dd7f03e318fb534103b39bf93aeb4c03b1
> # Parent  c27dc3c31222c7f74331221a3d25566146feecac
> ui: provide official way to reset internal state per command
Augie Fackler - June 14, 2016, 12:41 a.m.
Queued, thanks

> On Jun 12, 2016, at 7:54 AM, Jun Wu <quark@fb.com> wrote:
> 
> This looks good to me. It does not conflict with my ui refactoring idea,
> although the refactor won't come soon.
> 
> For ui, I'd prefer simper apis and wonder if we have alternative options.
> 
> Extensions can always wrap functions to do whatever they want. It's possible
> to do extension-specific reset state by wrapping dispatch.runcommand. And
> progbar.resetstate() looks like progress.py specific.
> 
> Consider the fact progress.py is in core, wrapping looks strange, I think
> it's reasonable to just move progbar.resetstate() to dispatch.py.
> 
> Excerpts from Yuya Nishihara's message of 2016-06-12 14:31:15 +0900:
>> # HG changeset patch
>> # User Yuya Nishihara <yuya@tcha.org>
>> # Date 1465708046 -32400
>> #      Sun Jun 12 14:07:26 2016 +0900
>> # Node ID 58d3b0dd7f03e318fb534103b39bf93aeb4c03b1
>> # Parent  c27dc3c31222c7f74331221a3d25566146feecac
>> ui: provide official way to reset internal state per command

Patch

diff --git a/mercurial/commandserver.py b/mercurial/commandserver.py
--- a/mercurial/commandserver.py
+++ b/mercurial/commandserver.py
@@ -229,12 +229,8 @@  class server(object):
             self.repo.ui = self.repo.dirstate._ui = repoui
             self.repo.invalidateall()
 
-        # reset last-print time of progress bar per command
-        # (progbar is singleton, we don't have to do for all uis)
-        if copiedui._progbar:
-            copiedui._progbar.resetstate()
-
         for ui in uis:
+            ui.resetstate()
             # any kind of interaction must use server channels, but chg may
             # replace channels by fully functional tty files. so nontty is
             # enforced only if cin is a channel.
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -138,6 +138,11 @@  class ui(object):
     def copy(self):
         return self.__class__(self)
 
+    def resetstate(self):
+        """Clear internal state that shouldn't persist across commands"""
+        if self._progbar:
+            self._progbar.resetstate()  # reset last-print time of progress bar
+
     def formatter(self, topic, opts):
         return formatter.formatter(self, topic, opts)