Patchwork [1,of,3,remove-makememctx,V2] memctx: refactor inline getfilectx into convenience method

# HG changeset patch
# User Sean Farley <sean@farley.io>
# Date 1497039902 25200
#      Fri Jun 09 13:25:02 2017 -0700
# Branch remove-makememctx
# Node ID dfe7dc826d3b1c6b0c933cc74c9ad14042f39877
# Parent  776d077eb4ef815e08631fb1e7b33375adca3ef1
memctx: refactor inline getfilectx into convenience method

No actual logic is changing, just moving code so __init__ is easier to
read.

On Fri, Jun 9, 2017 at 2:20 PM, Sean Farley <sean@farley.io> wrote:
> # HG changeset patch
> # User Sean Farley <sean@farley.io>
> # Date 1497039902 25200
> #      Fri Jun 09 13:25:02 2017 -0700
> # Branch remove-makememctx
> # Node ID dfe7dc826d3b1c6b0c933cc74c9ad14042f39877
> # Parent  776d077eb4ef815e08631fb1e7b33375adca3ef1
> memctx: refactor inline getfilectx into convenience method
>
> No actual logic is changing, just moving code so __init__ is easier to
> read.
>
> diff --git a/mercurial/context.py b/mercurial/context.py
> index 91b5bf1..5960d02 100644
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -2039,10 +2039,29 @@ def makecachingfilectxfn(func):
>              cache[path] = func(repo, memctx, path)
>          return cache[path]
>
>      return getfilectx
>
> +def memfilefromctx(ctx):

Should this have a '_' prefix or do you plan on adding users outside
of the module?

> +    """Given a context return a memfilectx for ctx[path]
> +
> +    This is a convenience method for building a memctx based on another
> +    context.

But it doesn't build a memctx. Was it simply meant to say
"memfilectx"? It becomes pretty similar to the first line then.

> +    """
> +    def getfilectx(repo, memctx, path):
> +        fctx = ctx[path]
> +        # this is weird but apparently we only keep track of one parent
> +        # (why not only store that instead of a tuple?)
> +        copied = fctx.renamed()
> +        if copied:
> +            copied = copied[0]
> +        return memfilectx(repo, path, fctx.data(),
> +                          islink=fctx.islink(), isexec=fctx.isexec(),
> +                          copied=copied, memctx=memctx)
> +
> +    return getfilectx
> +
>  class memctx(committablectx):
>      """Use memctx to perform in-memory commits via localrepo.commitctx().
>
>      Revision information is supplied at initialization time while
>      related files data and is made available through a callback
> @@ -2086,21 +2105,11 @@ class memctx(committablectx):
>          self._files = files
>          self.substate = {}
>
>          # if store is not callable, wrap it in a function
>          if not callable(filectxfn):
> -            def getfilectx(repo, memctx, path):
> -                fctx = filectxfn[path]
> -                # this is weird but apparently we only keep track of one parent
> -                # (why not only store that instead of a tuple?)
> -                copied = fctx.renamed()
> -                if copied:
> -                    copied = copied[0]
> -                return memfilectx(repo, path, fctx.data(),
> -                                  islink=fctx.islink(), isexec=fctx.isexec(),
> -                                  copied=copied, memctx=memctx)
> -            self._filectxfn = getfilectx
> +            self._filectxfn = memfilefromctx(filectxfn)
>          else:
>              # memoizing increases performance for e.g. vcs convert scenarios.
>              self._filectxfn = makecachingfilectxfn(filectxfn)
>
>          if editor:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Martin von Zweigbergk <martinvonz@google.com> writes:

> On Fri, Jun 9, 2017 at 2:20 PM, Sean Farley <sean@farley.io> wrote:
>> # HG changeset patch
>> # User Sean Farley <sean@farley.io>
>> # Date 1497039902 25200
>> #      Fri Jun 09 13:25:02 2017 -0700
>> # Branch remove-makememctx
>> # Node ID dfe7dc826d3b1c6b0c933cc74c9ad14042f39877
>> # Parent  776d077eb4ef815e08631fb1e7b33375adca3ef1
>> memctx: refactor inline getfilectx into convenience method
>>
>> No actual logic is changing, just moving code so __init__ is easier to
>> read.
>>
>> diff --git a/mercurial/context.py b/mercurial/context.py
>> index 91b5bf1..5960d02 100644
>> --- a/mercurial/context.py
>> +++ b/mercurial/context.py
>> @@ -2039,10 +2039,29 @@ def makecachingfilectxfn(func):
>>              cache[path] = func(repo, memctx, path)
>>          return cache[path]
>>
>>      return getfilectx
>>
>> +def memfilefromctx(ctx):
>
> Should this have a '_' prefix or do you plan on adding users outside
> of the module?

I was on the fence about it but you're correct; there are no users
outside of context.py so feel free to prefix it with a '_'.

>> +    """Given a context return a memfilectx for ctx[path]
>> +
>> +    This is a convenience method for building a memctx based on another
>> +    context.
>
> But it doesn't build a memctx. Was it simply meant to say
> "memfilectx"? It becomes pretty similar to the first line then.

I did mean memfilectx; good catch (also feel free to change in flight).