Thread (10 messages) flat view 10 messages, 3 authors, 2017-09-19

Re: [PATCH v1 1/1] dir: teach status to show ignored directories

From: Stefan Beller <hidden>
Date: 2017-08-14 21:05:11

On Fri, Aug 11, 2017 at 10:48 AM, Jameson Miller
[off-list ref] wrote:
The set of files listed by "--ignored" changes when different
values are given to "--untracked-files".  If would be nice to
be able to make the ignored output independent of the untracked
settings.  This patch attempts to do that while maintaining
backward compatibility with the existing behavior.

So after rereading this, maybe we could have
'--ignored[=as-untracked, new-mode]' with as-untracked as default?

This would satisfy your goal, while still maintaining backwards
compatibility.
We want to see the paths that explicitly match an ignore pattern.
This means that if a directory only contains ignored files, but the
directory itself is not explicitly ignored, then we want to see the
individual files. This is slightly different than the current behavior
of "--ignored".

I am open to suggestions on how to present the options to control
this behavior.
I would strongly favor not introducing yet another flag to tweak the
behavior of another command line option. (The UX of Git is already
bloated, IMHO. But that is just my general stance on things)

So we'd have two proposals:

(1) The current patch proposes --show-ignored-directory that modifies
    the behavior of other flags
(2) extend an already existing option to take another mode (which I
    propose)

Strong arguments for (1) would be if the flag is orthogonal and apply
to the other flags in a uniform way, i.e.:
    status --no-ignored --untracked=no --show-ignored-directory
would make sense and produce an output that a user doesn't
find confusing.

Arguments for (2), are
* less documentation for users (see below)
* if the two flags are truly orthogonal, test effort is decreased
* no exposure of an API that may yield strange results when the user
  gives strange input.
quoted
quoted
This
change exposes a flag to report all untracked files while not showing
individual files in ignored directories.
By the description up to here, it sounds as if you want to introduce
mode for --untracked, e.g. "normal-adjusted-for-ignored" (it's a bad
suggestion)? However the patch seems to add an orthogonal flag,
such that

   status --no-ignored --untracked=no --show-ignored-directory

would also be possible. What is a reasonable expectation for
the output of such?

The current patch does add another flag. This flag only has meaning if
the "--ignored" and "--untracked=all" flags are also specified. Another
option I had considered is to let the "--ignored" flag take an argument.
I would think that is a better solution, as then untracked and ignored
files (and their parameters) will be orthogonal in the long run.
Things like

  --untracked=all --ignored=none
  --untracked=none --ignored=only-dirs
  --untracked=reasonable-default --ignored=other-reasonable-default

will be possible and eventually the default for ignored files may be
independent of untracked settings.
Then, we could express this new behavior through (for example) a
"--ignored=exact" flag to reflect the fact that this new option
returns paths that match the ignore pattern, and does not
collapse directories that contain only ignored files.
Yes, and we would not need to worry about how the new
flag interacts with other flags. I would prefer that solution as it
seems the API will be cleaner that way.

Less things depend on each other, e.g. as a user I do not need to
know about the 'other' flag, be it ignored or untracked to solve my
problem. If we'd go with this third flag, we'd need to (a) document it,
but (b) also have to add a sentence to untracked and ignored flag
    "In the corner case of also having the third flag, we change
    behavior once again"
which I'd dislike from a users perspective.
quoted
quoted
Commands:
  1) status
  2) status --ignored
  3) status --ignored --untracked-files=all
  4) status --ignored --untracked-files=all --show-ignored-directory
(2) is --untracked-files=normal I'd presume, such that this flag
can be understood as a tweak to "normal" based on the similar size
between 2 and 4? (The timing improvement from 2 to 4 is huge though).
(2) is --untracked-files=normal. Although the count of ignored
files similar between 2 and 4, I consider this flag more of a
tweak on 3, as we want the untracked files reported with
the "--untracked=all" flag. The counts between 2 and 4 are
similar in this case because most of the ignored files are
contained in ignored directories.

Our application calls status including the following flags:

--porcelain=v2 --ignored --untracked-files=all --ignore-submodules=none

This means we have bad performance compared to just "git status"
when there is a large number of files in ignored directories
With this new behavior, our application would move from case 3 to
case 4 for this repository.

You also point out the timing difference between case 2 and 4. I
think there is an optimization we can make when running "git
status --ignored" logic that will improve the the timing
here. Currently, the logic in dir.c is iterating over all files
contained in an ignored directory to see if it is empty or
not. This is not necessary - we should be able to stop after
finding any file. I plan to follow up on this in a different
patch.
Thanks!
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help