[PATCH 09 of 12] diff: rewrite addmodehdr into addflagchangemeta

[Kevin Bullock]

On 15 Nov 2012, at 5:23 PM, Guillermo Pérez wrote:

> # HG changeset patch
> # User Guillermo Pérez <bisho at fb.com>
> # Date 1352320114 28800
> # Node ID 2dba4713ea5373a11948b6cd9fa74d8dc7b829c9
> # Parent  197eae6f1822b5266cc683773ea0cfefdf82595d
> diff: rewrite addmodehdr into addflagchangemeta
> 
> Mercurial handles flags rather than file modes, so it's better
> to signal flag changes and let the meta handler function
> adapt it to the needed header, file modes for git in particular
> but might be different if other patch methods are implemented.
> 
> diff --git a/mercurial/patch.py b/mercurial/patch.py
> --- a/mercurial/patch.py
> +++ b/mercurial/patch.py
> @@ -1658,10 +1658,12 @@
>         return os.path.join(prefix, f)
> 
>     ''' Helper header functions '''
> -    def addmodehdr(header, omode, nmode):
> -        if omode != nmode:
> -            header.append('old mode %s\n' % omode)
> -            header.append('new mode %s\n' % nmode)
> +    gitmode = {'l': '120000', 'x': '100755', '': '100644'}
> +
> +    def addflagchangemeta(meta, fa, fb):
> +        if opts.git:
> +            meta.append('old mode %s\n' %  gitmode[fa])
> +            meta.append('new mode %s\n' %  gitmode[fb])
> 
>     def addindexmeta(meta, revs):
>         if opts.git:
> @@ -1696,7 +1698,6 @@
>     man1 = ctx1.manifest()
> 
>     gone = set()
> -    gitmode = {'l': '120000', 'x': '100755', '': '100644'}
> 
>     copyto = dict([(v, k) for k, v in copy.items()])
> 
> @@ -1706,6 +1707,8 @@
>     for f in sorted(modified + added + removed):
>         to = None
>         tn = None
> +        oflag = None
> +        nflag = None
>         dodiff = True
>         header = []
>         if f in man1:
> @@ -1715,15 +1718,14 @@
>         a, b = f, f
>         if opts.git or losedatafn:
>             if f in added:
> -                mode = gitmode[ctx2.flags(f)]
> +                nflag = ctx2.flags(f)
>                 if f in copy or f in copyto:
>                     if opts.git:
>                         if f in copy:
>                             a = copy[f]
>                         else:
>                             a = copyto[f]
> -                        omode = gitmode[man1.flags(a)]
> -                        addmodehdr(header, omode, mode)
> +                        oflag = man1.flags(a)
>                         if a in removed and a not in gone:
>                             op = 'rename'
>                             gone.add(a)
> @@ -1736,7 +1738,7 @@
>                         losedatafn(f)
>                 else:
>                     if opts.git:
> -                        header.append('new file mode %s\n' % mode)
> +                        header.append('new file mode %s\n' % gitmode[nflag])

Couldn't this be morphed into a call to addflagchangemeta, by checking 'if fa is None' there? It'd be nice to not duplicate the logic here, as trivial as it is.

This could be done in a follow-up patch.

>                     elif ctx2.flags(f):
>                         losedatafn(f)
>                 # In theory, if tn was copied or renamed we should check
> @@ -1769,12 +1771,13 @@
>                 nflag = ctx2.flags(f)
>                 binary = util.binary(to) or util.binary(tn)
>                 if opts.git:
> -                    addmodehdr(header, gitmode[oflag], gitmode[nflag])
>                     if binary:
>                         dodiff = 'binary'
>                 elif binary or nflag != oflag:
>                     losedatafn(f)
> 
> +        if nflag is not None and oflag is not None and nflag != oflag:
> +            addflagchangemeta(header, oflag, nflag)
>         if dodiff:
>             if opts.git or revs:
>                 header.insert(0, diffline(join(a), join(b), revs))
> diff --git a/tests/test-git-export.t b/tests/test-git-export.t
> --- a/tests/test-git-export.t
> +++ b/tests/test-git-export.t
> @@ -75,10 +75,10 @@
>   $ hg ci -mrenamemod
>   $ hg diff --git -r 6:tip
>   diff --git a/src b/dst
> +  rename from src
> +  rename to dst
>   old mode 100755
>   new mode 100644
> -  rename from src
> -  rename to dst

Does the change in order here present a compatibility issue?

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock