[PATCH 04 of 10 shelve-ext v4] shelve: move node-pruning functionality to be member of shelvedstate

[Augie Fackler]

On Sat, Mar 11, 2017 at 01:00:23PM -0800, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia at fb.com>
> # Date 1489191652 28800
> #      Fri Mar 10 16:20:52 2017 -0800
> # Node ID 3b034fb4bfcfdfd7700fd43edef33d032efd4924
> # Parent  8063b183d2396f14093ebe84da9cedf17b13881d
> shelve: move node-pruning functionality to be member of shelvedstate
>
> Node-pruning can be node stripping or marker creation, depending on
> whether shelve is traditional or obs-based. Thus it makes sense to
> move it to a separate function. Also, since we already have
> shelvedstate object and this functionality operates on that object,
> it makes sense to make it a method.
>
> Having shelvedstate object contain repo and ui as members allows for
> calling 'state.prunenodes()' instead of 'state.prunenodes(repo, ui)'
> which is better IMO.

I'm torn. It honestly feels a little weird to hold on to repo and ui
variables for this one function call. Maybe pass them in for now and
we can revisit the architecture later?

>
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -171,8 +171,12 @@ class shelvedstate(object):
>      _keep = 'keep'
>      _nokeep = 'nokeep'
>
> +    def __init__(self, ui, repo):
> +        self.ui = ui
> +        self.repo = repo
> +
>      @classmethod
> -    def load(cls, repo):
> +    def load(cls, ui, repo):
>          fp = repo.vfs(cls._filename)
>          try:
>              version = int(fp.readline().strip())
> @@ -193,7 +197,7 @@ class shelvedstate(object):
>              fp.close()
>
>          try:
> -            obj = cls()
> +            obj = cls(ui, repo)
>              obj.name = name
>              obj.wctx = repo[wctx]
>              obj.pendingctx = repo[pendingctx]
> @@ -226,6 +230,11 @@ class shelvedstate(object):
>      def clear(cls, repo):
>          util.unlinkpath(repo.join(cls._filename), ignoremissing=True)
>
> +    def prunenodes(self):
> +        """Cleanup temporary nodes from the repo"""
> +        repair.strip(self.ui, self.repo, self.nodestoprune, backup=False,
> +                     topic='shelve')
> +
>  def cleanupoldbackups(repo):
>      vfs = vfsmod.vfs(repo.join(backupdir))
>      maxbackups = repo.ui.configint('shelve', 'maxbackups', 10)
> @@ -569,8 +578,7 @@ def unshelveabort(ui, repo, state, opts)
>                  raise
>
>              mergefiles(ui, repo, state.wctx, state.pendingctx)
> -            repair.strip(ui, repo, state.nodestoprune, backup=False,
> -                         topic='shelve')
> +            state.prunenodes()
>          finally:
>              shelvedstate.clear(repo)
>              ui.warn(_("unshelve of '%s' aborted\n") % state.name)
> @@ -647,7 +655,7 @@ def unshelvecontinue(ui, repo, state, op
>          mergefiles(ui, repo, state.wctx, shelvectx)
>          restorebranch(ui, repo, state.branchtorestore)
>
> -        repair.strip(ui, repo, state.nodestoprune, backup=False, topic='shelve')
> +        state.prunenodes()
>          shelvedstate.clear(repo)
>          unshelvecleanup(ui, repo, state.name, opts)
>          ui.status(_("unshelve of '%s' complete\n") % state.name)
> @@ -819,7 +827,7 @@ def _dounshelve(ui, repo, *shelved, **op
>              ui.warn(_('tool option will be ignored\n'))
>
>          try:
> -            state = shelvedstate.load(repo)
> +            state = shelvedstate.load(ui, repo)
>              if opts.get('keep') is None:
>                  opts['keep'] = state.keep
>          except IOError as err:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel