[PATCH 1 of 3] cmdutil: add a function to terse the status

[Pulkit Goyal]

> Thanks for working on this!
>
> I wonder if this tersestatus function couldn't be unit-tested (might be
> easier if it didn't need the "repo" argument). I'd make it easier to
> understand its behaviour I think.

Okay in that case, I will need to pass two other arguments for
repo.root and repo.dirstate._ignore as root and ignorefn respectively.
Or you mean to say not using them?
>
> A couple of remarks (mostly style) below.

Thanks for them, I will follow all of them in V2.
>
>> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>> --- a/mercurial/cmdutil.py
>> +++ b/mercurial/cmdutil.py
>> @@ -402,6 +402,115 @@
>>
>>      return commit(ui, repo, recordinwlock, pats, opts)
>>
>> +def tersestatus(repo, statlist, status, ignore):
>> +    """
>> +    Returns a list of statuses with directory collapsed if all the files
>> in the
>> +    directory has the same status.
>> +    """
>> +
>> +    def numfiles(dirname):
>> +        """
>> +        Calculates the number of tracked files in a given directory which
>> also
>> +        includes files which were removed or deleted. Considers ignored
>> files
>> +        if ignore argument is True or 'i' is present in status argument.
>> +        """
>> +        if 'i' in status or ignore:
>> +            def match(fname):
>> +                return False
>> +        else:
>> +            match = repo.dirstate._ignore
>> +        lendir = 0
>> +        abspath = repo.root + pycompat.ossep + dirname
>
>
> Could you use os.path.join() ?
>
>> +        for f in os.listdir(abspath):
>> +            if not match(f):
>> +                lendir += 1
>> +        if absentdir.get(dirname):
>> +            lendir += absentdir[dirname]
>
>
> Python nit: `if dict.get(key):` is better written as `if key in dict:`.
> Here, you could also have written `lendir += absentdir.get(dirname, 0)`
> and drop the 'if'.
>
>
>> +        return lendir
>> +
>> +    def absentones(removedfiles, missingfiles):
>> +        """
>> +        Returns a dictionary of directories and number of files which are
>> either
>> +        removed or missing (deleted) in them.
>> +        """
>> +        absentdir = {}
>> +        absentfiles = removedfiles + missingfiles
>> +        while absentfiles:
>> +            f = absentfiles.pop()
>> +            par = os.path.dirname(f)
>> +            if par == '':
>> +                continue
>> +            try:
>> +                absentdir[par] += 1
>> +            except KeyError:
>> +                absentdir[par] = 1
>> +            if par not in removedfiles:
>> +                absentfiles.append(par)
>> +        return absentdir
>> +
>> +    indexes = {'m': 0, 'a': 1, 'r': 2, 'd': 3, 'u': 4, 'i': 5, 'c': 6}
>> +    absentdir = absentones(statlist[2], statlist[3])
>> +    finalrs = [[], [], [], [], [], [], []]
>
>
> is it `finalrs = [[]] * len(indexes)` ?
>
>> +    didsomethingchanged = False
>> +
>> +    for st in pycompat.bytestr(status):
>> +
>> +        try:
>> +            ind = indexes[st]
>> +        except KeyError:
>> +            raise error.Abort("Unable to parse the terse status, use
>> marduic")
>
>
> What's "marduic"?
> Also, is it really an user error? If not an assertion might be better maybe.

I am not sure what a good user error will be. I am changing it to
("'%s' not recognized" % st) in V2.
>
>> +
>> +        sfiles = statlist[ind]
>> +        if not sfiles:
>> +            continue
>> +        pardict = {}
>> +        for a in sfiles:
>> +            par = os.path.dirname(a)
>> +            try:
>> +                pardict[par].append(a)
>> +            except KeyError:
>> +                pardict[par] = [a]
>
>
> Other python nit: `pardict.setdefault(par, []).append(a)` to replace the
> try/except block.
>
>> +
>> +        rs = []
>> +        newls = []
>> +        for par in iter(pardict):
>
>
> iteritems() since you further need the value (pardict[par])
>
>> +            lenpar = numfiles(par)
>> +            if lenpar == len(pardict[par]):
>> +                newls.append(par)
>> +
>> +        if not newls:
>> +            continue
>> +
>> +        while newls:
>> +            newel = newls.pop()
>> +            if newel == '':
>> +                continue
>> +            parn = os.path.dirname(newel)
>> +            pardict[newel] = []
>> +            try:
>> +                pardict[parn].append(newel + pycompat.ossep)
>> +            except KeyError:
>> +                pardict[parn] = [newel + pycompat.ossep]
>
>
> setdefault could be used here as well.
> Also, why the `+ pycompat.ossep`? Might be worth adding a comment.
>
>> +            lenpar = numfiles(parn)
>> +            if lenpar == len(pardict[parn]):
>> +                newls.append(parn)
>> +
>> +        for par, files in pardict.iteritems():
>
>
> iteritems() -> itervalues(), since "par" is not used.
>
>> +            rs.extend(files)
>> +
>> +        finalrs[ind] = rs
>> +        didsomethingchanged = True
>> +
>> +    # If nothing is changed, make sure the order of files is preserved.
>> +    if not didsomethingchanged:
>> +        return statlist
>> +
>> +    for x in xrange(7):
>> +        if not finalrs[x]:
>> +            finalrs[x] = statlist[x]
>
>

> for rs in finalrs:
>   if not rs:
>      rs[:] = statlist[x]

In this case 'x' won't be defined. So can't use this.
>
>> +
>> +    return finalrs
>> +
>>  def findpossible(cmd, table, strict=False):
>>      """
>>      Return cmd -> (aliases, command table entry)
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>
>