> 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@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>
>
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
