[PATCH V3] status: add a flag to terse the output (issue4119)

Jun Wu quark at fb.com
Fri Jun 30 10:57:30 EDT 2017


The general idea looks good to me. I think "numfiles" needs improve to
support non-working-copy revs and I have a question about an "if" in
"absentones".

To land this fast, I think there are just 3 blocking items:

  - '-t iu' does not imply 'iu', pointed out by Denis
  - typo of 'ignore', pointed out by Denis
  - forbid '-t' to be used together with double '--rev'

Other things could be fixed as follow-ups.


Excerpts from 's message of 2017-06-21 07:07:49 +0530:
> [...]  
> +def tersestatus(root, statlist, status, ignorefn, 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.
> +        """

This will have trouble with non-working copy revisions, like "hg status
--rev A --rev B". The more correct way might be using matcher and ctx to
check non-ignored files, avoiding os.listdir.

For the first version, I think we can say "-t" does not work with "--rev" to
avoid the issue.

> [...]
> +    def absentones(removedfiles, missingfiles):
> [...]
> +            if par not in removedfiles:
> +                absentfiles.append(par)

I don't get the  above "if". For example, with the following status:

  ! x/b/c/d/A
  ! x/b/c/d/B
  ! x/b/k
  ? y/b/c/d/e
  ? y/c/z

Using the current code, `hg st -t du` outputs:

  ! x/b/c/d/
  ! x/b/k
  ? y/

With that "if" removed, `hg st -t du` outputs:

  ! x/
  ? y/

Is the former output desired?

> [...]
> +        for par, files in pardict.iteritems():
> +            lenpar = numfiles(par)

There could be some caching around "numfiles" calls. Like if different "st"
needs data for a same "par". Previous result could be reused. That might be
also helpful to avoid race-condition. This is optional and shouldn't block
the first version.

> [...]
>      if opts.get('all'):
>          show += ui.quiet and (states[:4] + ['clean']) or states
> +        if ui.quiet and terse:
> +            for st in ('ignored', 'unknown'):
> +                if st[0] in terse:
> +                    show.append(st)
> +

Might remove this "if" as Denis suggested. That allows people to have an
alias like "status = status -t all" and do not print ignored files by
accident.


> [...]
> diff --git a/tests/test-terse-status.t b/tests/test-terse-status.t

Tests could be stronger by having nested directories cases.


More information about the Mercurial-devel mailing list