git status --porcelain is a mess that needs fixing

git status --porcelain is a mess that needs fixing

[Eric Raymond]

I'm going to gripe a lot in this mail, possibly verging on flaming.
Therefore I want to start by making clear that I am not here to
complain without pitching in to help fix the problems.  If I can get
responsive answers to my questions, I will take responsibility for
editing them into the relevant git documentation,

Short version: "git status --porcelain" is horribly badly documented
and appears to be seriously maldesigned.  Both these problems need to
be fixed before git causes a lot of unnecessary grief for people
trying to use it.

Here is the entire documentation on this feature in HEAD:

=============================================================================

In short-format, the status of each path is shown as

	XY PATH1 -> PATH2

where `PATH1` is the path in the `HEAD`, and ` -> PATH2` part is
shown only when `PATH1` corresponds to a different path in the
index/worktree (i.e. renamed).

For unmerged entries, `X` shows the status of stage #2 (i.e. ours) and `Y`
shows the status of stage #3 (i.e. theirs).

For entries that do not have conflicts, `X` shows the status of the index,
and `Y` shows the status of the work tree.  For untracked paths, `XY` are
`??`.

    X          Y     Meaning
    -------------------------------------------------
              [MD]   not updated
    M        [ MD]   updated in index
    A        [ MD]   added to index
    D        [ MD]   deleted from index
    R        [ MD]   renamed in index
    C        [ MD]   copied in index
    [MARC]           index and work tree matches
    [ MARC]     M    work tree changed since index
    [ MARC]     D    deleted in work tree
    -------------------------------------------------
    D           D    unmerged, both deleted
    A           U    unmerged, added by us
    U           D    unmerged, deleted by them
    U           A    unmerged, added by them
    D           U    unmerged, deleted by us
    A           A    unmerged, both added
    U           U    unmerged, both modified
    -------------------------------------------------
    ?           ?    untracked
    -------------------------------------------------

=============================================================================

This was clearly written as an aide-memoire by someone intimately
familiar with the system, but I have to tell you it is so confusing
to me as to be nearly worse than useless.  

In addition, some of the design choices it appears to imply are quite
bad - so I hope I am wrong about those implications.  If I am not, you
have specified a misdesigned format that will frustrate and annoy your
customers (script and front-end writers). And that would be a problem.

As I criticize, bear in mind that (a) none of my issues are VC
specific, and (b) I am the author of several version-control front
ends - *I have done this before.* My objections are *not*
theoretical!

First, the documentation issues, in roughly increasing order of severity:

1. What separates the XY column from the first path?

I'd assume a tab, but it's not documented. It needs to be documented.

2. What separates the '->' on either side from the path columns?

Not documented.  Needs to be documented.  

3. What do the status codes M A D R C mean?

I can guess, but I should not have to guess.  They should be documented.

4. Some columns in the table have sets of codes enclosed by [].  Is
this indicating alternation?

My guess is yes, but I should not have to guess.  This should be documented.

5. What is 'us' versus 'them'? What are "stage #2" and "stage #3"?

It makes my brain hurt just trying to list all the things "us"
and "them" could mean.

Remember that because you're advertising a format for script use, your
audience for this page is not git hackers.  It's not git power
users. It's not even ordinary git users. It's people whose main
expertise is is *other tools*.  They want to get in, write their
script and get out, having learned as little about git as they can get
away with.

If 'us'/'them'/'stage #2'/'stage #3' are git terms of art that are well
defined elsewhere, you must reference that elsewhere.  If they are
not, you need to define them here.  And because of the special
audience for this page, it needs to be more self-contained and make
fewer assumptions about the reader's knowledge than usual.

Note: I, personally, read very fast and don't mind the mental effort
of skimming 50-100 pages of other documentation.  But you must *not*
assume I am anything but an exception.  This *particular* section on
this *particular* page needs (more than others) to be written so it
would be comprehensible to a lazy idiot who vaguely knows about
otther version-control systems and can't be bothered to read 
about this one, either.  


Now to the functional problems, again in roughly increasing order of
severity:

A. The '->' separator considered harmful

The '->' was superfluous and thus a poor design choice; the
distinction between two columns and three columns is easy enough to
make in any scripting language.  As it is, it's meaningless and
scripts will actually have to go to some extra effort to throw it
away.

I think the underlying problem here is that whoever designed this
never got past the idea that it needed to have cues for human
eyeballs in it.  That was a mistake.  If you're serious about it
being easily parseable, design it that way.

B. Does "untracked" include "ignored"?  

If so, that is a problem -- front ends care about the difference, for
example when C-x v v is trying to compute the logical next action.
For an unregistered file, it's to register it.  For an ignored file,
it's to throw a user-visible error.

C. If "untracked" does not include "ignored", how is an ignored file tagged?

If ignored files are not listed, that's another problem. Even more
serious, actually.

D. How do I tell the conflict/no-conflict cases apart?

You have three divisions in the table.  The first two are supposed
to pertain to "entries that do not have conflicts" and "unmerged
entries".  

They share code letters.  *How do I tell them apart?* 

Illustrative case: I see the status code "DD". How do I distinguish
between case 4 ("deleted from index") and case 10 ("unmerged, both
deleted")?

If the distinction is meaningless, then why are they listed
separately?

E. Are you *really* using a space as a status character?

It certainly appears so from the first and seventh rows of the table.
If so, this was a major blunder. It complicates parsing code
unnecessarily, because the easiest way to separate columns is with the
equivalent of a Python or Perl split() operation that will eat that
space.  Then we have to special-case depending on the field width.

The correct way to design a format like this for script parseability
is to (a) never make the difference between space and tab significant,
and (b) never use whitespace as anything but a field separator.  If
you want the equivalent of "blank" you use '-', as in Unix ls -l
output.

This may sound like a nitpick, but it's actually a crash landing, or
close to it.  Front-end writers look at things like this and think
"Idiots.  Can't trust them an inch...".  And git already has a bad
reputation for interface spikiness to live down.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

A right is not what someone gives you; it's what no one can take from you. 
	-- Ramsey Clark

Re: git status --porcelain is a mess that needs fixing

[Junio C Hamano]

Eric Raymond <esr@snark.thyrsus.com> writes:

> First, the documentation issues, in roughly increasing order of severity:
> ...

These all fall within "Patches welcome" category (meaning: I agree the
documentation can be improved and I don't object to changing them).

> D. How do I tell the conflict/no-conflict cases apart?
> ...
> Illustrative case: I see the status code "DD". How do I distinguish
> between case 4 ("deleted from index") and case 10 ("unmerged, both
> deleted")?

Is that DD really "illustrative", or did you mean to say "only/sole"?

You should never get "DD" in non-conflicting case.  I think I was fairly
careful not to make them ambiguous when I did that code, but apparently I
wasn't so careful about the documentation.

Thanks for going through this area with fine comb.

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 1cab91b..313dd04 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -86,7 +86,7 @@ and `Y` shows the status of the work tree.  For untracked paths, `XY` are
               [MD]   not updated
     M        [ MD]   updated in index
     A        [ MD]   added to index
-    D        [ MD]   deleted from index
+    D         [ M]    deleted from index
     R        [ MD]   renamed in index
     C        [ MD]   copied in index
     [MARC]           index and work tree matches

Re: git status --porcelain is a mess that needs fixing

[Jeff King]

On Fri, Apr 09, 2010 at 02:46:08PM -0400, Eric Raymond wrote:

> First, the documentation issues, in roughly increasing order of severity:

Note that "status --porcelain" is brand new in v1.7.0, so you may be
among the first to be seriously reading the documentation. As Junio
said, I think patches in this area are very welcome.

My answers below are meant to help you understand. I omitted the "...and
yes, this should be documented better" from the end of each, but you can
say it in your head if you want.

> 1. What separates the XY column from the first path?
> 
> I'd assume a tab, but it's not documented. It needs to be documented.

It's a space.

> 2. What separates the '->' on either side from the path columns?
> 
> Not documented.  Needs to be documented.

It's a space. But more importantly, the path columns are actually
C-quoted. E.g.:

  $ perl -e 'open foo, ">", "foo\n"'\
  $ git add .
  $ git status --porcelain
  A  "foo\n"

If your parser supports it, it will almost certainly be easier to use
"-z":

  $ git status --porcelain -z | cat -A
  A  foo$
  ^@

Do note that for the 'R'ename status, you will get _two_ NUL-terminated
entries, and they will be in the order of "to\0from\0", whereas the
non-NUL form is "from -> to" (and no, I doubt this is adequately
documented, either).

> 3. What do the status codes M A D R C mean?
> 
> I can guess, but I should not have to guess.  They should be documented.

They are the same as in "git diff --name-status", which in turn has kind
of crappy documentation. Patches welcome for both issues.

> 5. What is 'us' versus 'them'? What are "stage #2" and "stage #3"?
> 
> It makes my brain hurt just trying to list all the things "us"
> and "them" could mean.

The terms "us / ours" and "them / theirs" are frequently used in the git
documentation.  I'm not sure if they are ever defined rigorously. They
are only meaningful in a merging context, and basically refer to the two
sides of a merge. If I am on branch "master" and do "git merge foo",
then "us" refers to the master branch and the the contents of index
stage 2 (bear with me a moment, I'll define that in a second). "Them"
refers to branch "foo" and index stage 3.

Git's "index" is where it keeps uncommitted state about files it tracks
(sort of like CVS/Entries, if that helps, except that git exposes the
concept much more). Most of the time, you use it for building a commit
incrementally. You "git add" files to the index, and then "git commit"
creates a new commit from the contents of your index.

But the index actually has several different slots for each file entry,
which are called stages, and each has a number. "Stage #0" is the
"normal" stage, which you use as described in the last paragraph. During
a merge, entries with conflicts use the other stages. The stage #1 entry
contains the common ancestor. Stage #2 contains our original version
from before the merge. Stage #3 contains the other side's original
version from before the merge.

The details of how they are used is discussed in "git help read-tree",
under "3-Way Merge". Those details are way too gory for somebody
interested in "git status" output, but you might find them interesting.
For the "git status" documentation, it probably makes sense to keep
things simple and just indicate that XY shows what each side of a 2-way
merge did to the file.

> A. The '->' separator considered harmful
> 
> The '->' was superfluous and thus a poor design choice; the
> distinction between two columns and three columns is easy enough to
> make in any scripting language.  As it is, it's meaningless and
> scripts will actually have to go to some extra effort to throw it
> away.
> 
> I think the underlying problem here is that whoever designed this
> never got past the idea that it needed to have cues for human
> eyeballs in it.  That was a mistake.  If you're serious about it
> being easily parseable, design it that way.

Short answer: use -z.

Long answer:

This is my fault, to some degree. The "short-status" form _is_ meant for
human eyeballs, and was designed by Junio. Some people wanted a
scriptable status output, too, so I slapped a "--porcelain" on the same
format that turns off configurable features like relative pathnames and
colorizing, and makes an implicit promise that we won't make further
changes to the format. The idea was to prevent people from scripting
around --short, because it was never intended to be stable.

So yeah, while --porcelain by itself _is_ stable and scriptable, it is
perhaps not the most friendly to parsers. The "-z --porcelain" format is
much more so, and I would recommend it to anyone scripting around
"git status". I think a note in the documentation to that effect would
be helpful.

> B. Does "untracked" include "ignored"?
> 
> If so, that is a problem -- front ends care about the difference, for
> example when C-x v v is trying to compute the logical next action.
> For an unregistered file, it's to register it.  For an ignored file,
> it's to throw a user-visible error.

No. Ignored files are not listed at all.

If you really want a list of ignored files, I think you are stuck
comparing the output of "git ls-files -o" and "git ls-files -o
--exclude-standard".

> C. If "untracked" does not include "ignored", how is an ignored file tagged?
>
> If ignored files are not listed, that's another problem. Even more
> serious, actually.

See above.

It wouldn't be too hard to add them in, and would look something like
the patch below. But it would still need:

  1. For me to investigate that "ugh" comment below.

  2. It should be conditional on a command-line option. Many users won't
     want to see it.

  3. For full-length status (i.e., "git status") in my patch the
     information is just ignored. If the user specified it on the
     command-line, I guess we should show it.

> D. How do I tell the conflict/no-conflict cases apart?

Junio already answered this one, and I agree with his analysis that it
is a documentation bug.

> E. Are you *really* using a space as a status character?

Yes. I agree that a "-" probably would have been nicer for parsing, but:

> It certainly appears so from the first and seventh rows of the table.
> If so, this was a major blunder. It complicates parsing code
> unnecessarily, because the easiest way to separate columns is with the
> equivalent of a Python or Perl split() operation that will eat that
> space.  Then we have to special-case depending on the field width.

Your parser is already broken if you are calling split, as the filenames
may contain spaces (and will be quoted in that case, and you need to
unmangle). You should use "-z".

You will probably then realize that the "-z" format looks like:

  XY file1\0file2\0

which still sucks. It would be more friendly as:

  XY\0file1\0file2\0

So you could split on "\0". But even with that, you can't just blindly
split, as the column and record separators are the same, and you might
have one or two filenames.

So you really are stuck parsing it as one char of X, one char of Y, a
junk space, and then depending on X/Y, either one or two filenames.

> This may sound like a nitpick, but it's actually a crash landing, or
> close to it.  Front-end writers look at things like this and think
> "Idiots.  Can't trust them an inch...".  And git already has a bad
> reputation for interface spikiness to live down.

I agree with most of your criticisms. The question is what we want to do
about it.

Documentation fixes and an optional --show-ignored are easy. Output
format problems are harder. We can:

  (1) Ignore it. The problems make parsing harder, but it isn't
      completely broken (which I would consider it to be if, for
      example, there was impossibly ambiguous output).

  (2) Quietly change it.  The --porcelain format has been released in
      one major version. I don't know if anybody is actually using it
      yet. It is tempting to just fix it and say "we botched v1.7.0,
      don't use it". It is such a new feature that script writers
      already have to check the version to see if we even support
      --porcelain at all (or accept breakage for older versions).

      But usually we have more restraint than that about backwards
      incompatible changes. And given that it _isn't_ totally broken,
      I don't think it's justified.

  (3) Introduce --porcelain=v2 with an alternate format.

Personally, I think I am in favor of (1). Option (3) is going to
introduce maintenance headaches, but more importantly, I wonder if it is
just going to confuse people more with "Which porcelain version should I
use?  Which versions of git support which porcelain versions?"
questions.

-Peff

Re: git status --porcelain is a mess that needs fixing

[Jonathan Nieder]

Jeff King wrote:

> If you really want a list of ignored files, I think you are stuck
> comparing the output of "git ls-files -o" and "git ls-files -o
> --exclude-standard".

"git clean -n -d" may help.

Just my 2¢,
Jonathan

Re: git status --porcelain is a mess that needs fixing

[Jonathan Nieder]

Jonathan Nieder wrote:
> Jeff King wrote:

>> If you really want a list of ignored files, I think you are stuck
>> comparing the output of "git ls-files -o" and "git ls-files -o
>> --exclude-standard".
>
> "git clean -n -d" may help.

err, "git clean -n -d -X".

I am also not sure how stable the "Would remove " output format is,
or how stable we want it to be.  Probably not stable at all, so
sorry about that.

Jonathan

Re: git status --porcelain is a mess that needs fixing

[Eric Raymond]

Jeff King <peff@peff.net>:
> My answers below are meant to help you understand.

They do that quite well.  Thank you.

I've got a couple of other things on my plate, including prepping for 
a GPSD point release early next week, so I can't respond immediately.
Expect a response and some patches Tueday, Wednesday, or Thursday
of next week.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: git status --porcelain is a mess that needs fixing

[Jeff King]

On Sat, Apr 10, 2010 at 12:51:24AM -0500, Jonathan Nieder wrote:

> >> If you really want a list of ignored files, I think you are stuck
> >> comparing the output of "git ls-files -o" and "git ls-files -o
> >> --exclude-standard".
> >
> > "git clean -n -d" may help.
> 
> err, "git clean -n -d -X".
> 
> I am also not sure how stable the "Would remove " output format is,
> or how stable we want it to be.  Probably not stable at all, so
> sorry about that.

That's the same information, isn't it? You do "git clean -ndX" to see
_everything_ that is untracked, and "git clean -nd" to see things that
are untracked but not ignored. So I think it is just as painful to use
as ls-files, but as you noted, it is not really plumbing.

-Peff

Re: git status --porcelain is a mess that needs fixing

[Jonathan Nieder]

Jeff King wrote:

> You do "git clean -ndX" to see
> _everything_ that is untracked, and "git clean -nd" to see things that
> are untracked but not ignored.

No, the capital X tells clean to only list excluded files.  The
standard use is as a poor man’s “make maintainer-clean”, leaving
unrelated files alone.

I only learned about it just now.  I’m glad I did (I often use the
lowercase version for this because I just didn’t know about -X), but
as you mentioned, it is not so applicable here because not plumbing.

Jonathan

Re: git status --porcelain is a mess that needs fixing

[Jeff King]

On Sat, Apr 10, 2010 at 01:12:24AM -0500, Jonathan Nieder wrote:

> Jeff King wrote:
> 
> > You do "git clean -ndX" to see
> > _everything_ that is untracked, and "git clean -nd" to see things that
> > are untracked but not ignored.
> 
> No, the capital X tells clean to only list excluded files.  The
> standard use is as a poor man’s “make maintainer-clean”, leaving
> unrelated files alone.

Ah, I read it as "-x" (probably because I had never heard of "-X"
either...).

So yes, it would do the right thing. I still think a --show-ignored
option to git-status would probably be better (in addition to being
sanctioned plumbing, it means we only have to traverse the tree once
for Eric's case, instead of twice).

> I only learned about it just now.  I’m glad I did (I often use the
> lowercase version for this because I just didn’t know about -X), but
> as you mentioned, it is not so applicable here because not plumbing.

The "-X" mode seems much safer to me, as you are less likely to blow
away things you actually wanted to keep while cleaning the tree of
crufty build products. It seems like it should have been the
easier-to-type "-x", but it is far too late for such bikeshedding at
this point.

Thanks for the pointer.

-Peff

[PATCH 0/5] "status --ignored"

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> If ignored files are not listed, that's another problem. Even more
>> serious, actually.
>
> See above.
>
> It wouldn't be too hard to add them in, and would look something like
> the patch below.

As I didn't see a patch, I did a rough outline just for fun.

Junio C Hamano (5):
      wt-status: remove unused workdir_untracked field
      wt-status: plug memory leak while collecting untracked files
      wt-status: collect ignored files
      wt-status: rename and restructure status-print-untracked
      status: --ignored option shows ignored files

[PATCH 1/5] wt-status: remove unused workdir_untracked field

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 wt-status.c |    1 -
 wt-status.h |    1 -
 2 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 8ca59a2..db20b86 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -378,7 +378,6 @@ static void wt_status_collect_untracked(struct wt_status *s)
 			continue;
 		if (!match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL))
 			continue;
-		s->workdir_untracked = 1;
 		string_list_insert(ent->name, &s->untracked);
 	}
 }
diff --git a/wt-status.h b/wt-status.h
index 9120673..2c49f46 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -47,7 +47,6 @@ struct wt_status {
 	/* These are computed during processing of the individual sections */
 	int commitable;
 	int workdir_dirty;
-	int workdir_untracked;
 	const char *index_file;
 	FILE *fp;
 	const char *prefix;
-- 
1.7.1.rc0.264.g94f6e

[PATCH 2/5] wt-status: plug memory leak while collecting untracked files

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 wt-status.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index db20b86..c88159a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -379,7 +379,10 @@ static void wt_status_collect_untracked(struct wt_status *s)
 		if (!match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL))
 			continue;
 		string_list_insert(ent->name, &s->untracked);
+		free(ent);
 	}
+
+	free(dir.entries);
 }
 
 void wt_status_collect(struct wt_status *s)
-- 
1.7.1.rc0.264.g94f6e

[PATCH 3/5] wt-status: collect ignored files

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 wt-status.c |   16 ++++++++++++++++
 wt-status.h |    2 ++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index c88159a..f13c7da 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -42,6 +42,7 @@ void wt_status_prepare(struct wt_status *s)
 	s->index_file = get_index_file();
 	s->change.strdup_strings = 1;
 	s->untracked.strdup_strings = 1;
+	s->ignored.strdup_strings = 1;
 }
 
 static void wt_status_print_unmerged_header(struct wt_status *s)
@@ -382,6 +383,21 @@ static void wt_status_collect_untracked(struct wt_status *s)
 		free(ent);
 	}
 
+	if (s->show_ignored_files) {
+		dir.nr = 0;
+		dir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;
+		fill_directory(&dir, s->pathspec);
+		for (i = 0; i < dir.nr; i++) {
+			struct dir_entry *ent = dir.entries[i];
+			if (!cache_name_is_other(ent->name, ent->len))
+				continue;
+			if (!match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL))
+				continue;
+			string_list_insert(ent->name, &s->ignored);
+			free(ent);
+		}
+	}
+
 	free(dir.entries);
 }
 
diff --git a/wt-status.h b/wt-status.h
index 2c49f46..1093e65 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -41,6 +41,7 @@ struct wt_status {
 	int use_color;
 	int relative_paths;
 	int submodule_summary;
+	int show_ignored_files;
 	enum untracked_status_type show_untracked_files;
 	char color_palette[WT_STATUS_UNMERGED+1][COLOR_MAXLEN];
 
@@ -52,6 +53,7 @@ struct wt_status {
 	const char *prefix;
 	struct string_list change;
 	struct string_list untracked;
+	struct string_list ignored;
 };
 
 void wt_status_prepare(struct wt_status *s);
-- 
1.7.1.rc0.264.g94f6e

[PATCH 4/5] wt-status: rename and restructure status-print-untracked

[Junio C Hamano]

I will be reusing this to show ignored stuff in the next patch.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 wt-status.c |   22 ++++++++++++++--------
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index f13c7da..2c9a05d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -97,13 +97,15 @@ static void wt_status_print_dirty_header(struct wt_status *s,
 	color_fprintf_ln(s->fp, c, "#");
 }
 
-static void wt_status_print_untracked_header(struct wt_status *s)
+static void wt_status_print_other_header(struct wt_status *s,
+					 const char *what,
+					 const char *how)
 {
 	const char *c = color(WT_STATUS_HEADER, s);
-	color_fprintf_ln(s->fp, c, "# Untracked files:");
+	color_fprintf_ln(s->fp, c, "# %s files:", what);
 	if (!advice_status_hints)
 		return;
-	color_fprintf_ln(s->fp, c, "#   (use \"git add <file>...\" to include in what will be committed)");
+	color_fprintf_ln(s->fp, c, "#   (use \"git %s <file>...\" to include in what will be committed)", how);
 	color_fprintf_ln(s->fp, c, "#");
 }
 
@@ -541,7 +543,10 @@ static void wt_status_print_submodule_summary(struct wt_status *s, int uncommitt
 	run_command(&sm_summary);
 }
 
-static void wt_status_print_untracked(struct wt_status *s)
+static void wt_status_print_other(struct wt_status *s,
+				  struct string_list *l,
+				  const char *what,
+				  const char *how)
 {
 	int i;
 	struct strbuf buf = STRBUF_INIT;
@@ -549,10 +554,11 @@ static void wt_status_print_untracked(struct wt_status *s)
 	if (!s->untracked.nr)
 		return;
 
-	wt_status_print_untracked_header(s);
-	for (i = 0; i < s->untracked.nr; i++) {
+	wt_status_print_other_header(s, what, how);
+
+	for (i = 0; i < l->nr; i++) {
 		struct string_list_item *it;
-		it = &(s->untracked.items[i]);
+		it = &(l->items[i]);
 		color_fprintf(s->fp, color(WT_STATUS_HEADER, s), "#\t");
 		color_fprintf_ln(s->fp, color(WT_STATUS_UNTRACKED, s), "%s",
 				 quote_path(it->string, strlen(it->string),
@@ -641,7 +647,7 @@ void wt_status_print(struct wt_status *s)
 		wt_status_print_submodule_summary(s, 1);  /* unstaged */
 	}
 	if (s->show_untracked_files)
-		wt_status_print_untracked(s);
+		wt_status_print_other(s, &s->untracked, "Untracked", "add");
 	else if (s->commitable)
 		 fprintf(s->fp, "# Untracked files not listed (use -u option to show untracked files)\n");
 
-- 
1.7.1.rc0.264.g94f6e

[PATCH 5/5] status: --ignored option shows ignored files

[Junio C Hamano]

There is no stronger reason behind the choice of "!!" than just I happened
to have typed them.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/commit.c |    6 +++++-
 wt-status.c      |   22 +++++++++++++++-------
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index c5ab683..761ca07 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -83,6 +83,7 @@ static enum {
 static char *cleanup_arg;
 
 static int use_editor = 1, initial_commit, in_merge, include_status = 1;
+static int show_ignored_in_status;
 static const char *only_include_assumed;
 static struct strbuf message;
 
@@ -1031,6 +1032,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		  "mode",
 		  "show untracked files, optional modes: all, normal, no. (Default: all)",
 		  PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
+		OPT_BOOLEAN(0, "ignored", &show_ignored_in_status,
+			    "show ignored files"),
 		OPT_END(),
 	};
 
@@ -1044,7 +1047,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 			     builtin_status_options,
 			     builtin_status_usage, 0);
 	handle_untracked_files_arg(&s);
-
+	if (show_ignored_in_status)
+		s.show_ignored_files = 1;
 	if (*argv)
 		s.pathspec = get_pathspec(prefix, argv);
 
diff --git a/wt-status.c b/wt-status.c
index 2c9a05d..7bda995 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -646,9 +646,11 @@ void wt_status_print(struct wt_status *s)
 		wt_status_print_submodule_summary(s, 0);  /* staged */
 		wt_status_print_submodule_summary(s, 1);  /* unstaged */
 	}
-	if (s->show_untracked_files)
+	if (s->show_untracked_files) {
 		wt_status_print_other(s, &s->untracked, "Untracked", "add");
-	else if (s->commitable)
+		if (s->show_ignored_files)
+			wt_status_print_other(s, &s->ignored, "Ignored", "add -f");
+	} else if (s->commitable)
 		 fprintf(s->fp, "# Untracked files not listed (use -u option to show untracked files)\n");
 
 	if (s->verbose)
@@ -730,16 +732,16 @@ static void wt_shortstatus_status(int null_termination, struct string_list_item
 	}
 }
 
-static void wt_shortstatus_untracked(int null_termination, struct string_list_item *it,
-			    struct wt_status *s)
+static void wt_shortstatus_other(int null_termination, struct string_list_item *it,
+				 struct wt_status *s, const char *sign)
 {
 	if (null_termination) {
-		fprintf(stdout, "?? %s%c", it->string, 0);
+		fprintf(stdout, "%s %s%c", sign, it->string, 0);
 	} else {
 		struct strbuf onebuf = STRBUF_INIT;
 		const char *one;
 		one = quote_path(it->string, -1, &onebuf, s->prefix);
-		color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "??");
+		color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), sign);
 		printf(" %s\n", one);
 		strbuf_release(&onebuf);
 	}
@@ -763,7 +765,13 @@ void wt_shortstatus_print(struct wt_status *s, int null_termination)
 		struct string_list_item *it;
 
 		it = &(s->untracked.items[i]);
-		wt_shortstatus_untracked(null_termination, it, s);
+		wt_shortstatus_other(null_termination, it, s, "??");
+	}
+	for (i = 0; i < s->ignored.nr; i++) {
+		struct string_list_item *it;
+
+		it = &(s->ignored.items[i]);
+		wt_shortstatus_other(null_termination, it, s, "!!");
 	}
 }
 
-- 
1.7.1.rc0.264.g94f6e

Re: [PATCH 0/5] "status --ignored"

[Jeff King]

On Sat, Apr 10, 2010 at 12:40:51AM -0700, Junio C Hamano wrote:

> > It wouldn't be too hard to add them in, and would look something like
> > the patch below.
> 
> As I didn't see a patch, I did a rough outline just for fun.

Gah. Yours is undoubtedly less ugly, but here was mine for reference.

diff --git a/dir.c b/dir.c
index cb83332..deee7c8 100644
--- a/dir.c
+++ b/dir.c
@@ -619,7 +619,9 @@ static int exclude_matches_pathspec(const char *path, int len,
 				return 1;
 		}
 	}
-	return 0;
+	/* ugh, I think this may be a long-standing bug, but
+	 * this code was never reachable before. */
+	return 1;
 }
 
 static int get_index_dtype(const char *path, int len)
diff --git a/wt-status.c b/wt-status.c
index 8ca59a2..53a7a5a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -366,6 +366,8 @@ static void wt_status_collect_untracked(struct wt_status *s)
 	if (!s->show_untracked_files)
 		return;
 	memset(&dir, 0, sizeof(dir));
+	/* should be conditional on s->show_ignored_files */
+	dir.flags |= DIR_COLLECT_IGNORED;
 	if (s->show_untracked_files != SHOW_ALL_UNTRACKED_FILES)
 		dir.flags |=
 			DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;
@@ -381,6 +383,14 @@ static void wt_status_collect_untracked(struct wt_status *s)
 		s->workdir_untracked = 1;
 		string_list_insert(ent->name, &s->untracked);
 	}
+	for (i = 0; i < dir.ignored_nr; i++) {
+		struct dir_entry *ent = dir.ignored[i];
+		if (!cache_name_is_other(ent->name, ent->len))
+			continue;
+		if (!match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL))
+			continue;
+		string_list_insert(ent->name, &s->ignored);
+	}
 }
 
 void wt_status_collect(struct wt_status *s)
@@ -707,15 +717,15 @@ static void wt_shortstatus_status(int null_termination, struct string_list_item
 }
 
 static void wt_shortstatus_untracked(int null_termination, struct string_list_item *it,
-			    struct wt_status *s)
+			    struct wt_status *s, const char *marker)
 {
 	if (null_termination) {
-		fprintf(stdout, "?? %s%c", it->string, 0);
+		fprintf(stdout, "%s %s%c", marker, it->string, 0);
 	} else {
 		struct strbuf onebuf = STRBUF_INIT;
 		const char *one;
 		one = quote_path(it->string, -1, &onebuf, s->prefix);
-		color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "??");
+		color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), marker);
 		printf(" %s\n", one);
 		strbuf_release(&onebuf);
 	}
@@ -739,7 +749,11 @@ void wt_shortstatus_print(struct wt_status *s, int null_termination)
 		struct string_list_item *it;
 
 		it = &(s->untracked.items[i]);
-		wt_shortstatus_untracked(null_termination, it, s);
+		wt_shortstatus_untracked(null_termination, it, s, "??");
+	}
+	for (i = 0; i < s->ignored.nr; i++) {
+		struct string_list_item *it = &(s->ignored.items[i]);
+		wt_shortstatus_untracked(null_termination, it, s, "II");
 	}
 }
 
diff --git a/wt-status.h b/wt-status.h
index 9120673..0f5cf38 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -53,6 +53,7 @@ struct wt_status {
 	const char *prefix;
 	struct string_list change;
 	struct string_list untracked;
+	struct string_list ignored;
 };
 
 void wt_status_prepare(struct wt_status *s);

Re: [PATCH 3/5] wt-status: collect ignored files

[Jeff King]

On Sat, Apr 10, 2010 at 12:40:54AM -0700, Junio C Hamano wrote:

> +	if (s->show_ignored_files) {
> +		dir.nr = 0;
> +		dir.flags = DIR_SHOW_IGNORED | DIR_SHOW_OTHER_DIRECTORIES;
> +		fill_directory(&dir, s->pathspec);
> +		for (i = 0; i < dir.nr; i++) {
> +			struct dir_entry *ent = dir.entries[i];
> +			if (!cache_name_is_other(ent->name, ent->len))
> +				continue;
> +			if (!match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL))
> +				continue;
> +			string_list_insert(ent->name, &s->ignored);
> +			free(ent);
> +		}
> +	}
> +

Why not use DIR_COLLECT_IGNORED, which would mean we only have to
traverse the directory tree once?

-Peff

Re: [PATCH 0/5] "status --ignored"

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> +	dir.flags |= DIR_COLLECT_IGNORED;

I thought about using collect-ignored so that fill_directory() does not
have to be run twice, but IIRC it can short-circuit an entire directory
using the "simplify" logic (as it was designed to be used for "git add"),
no?

Mine is not "less ugly", but just "simple and stupid" ;-).

Re: [PATCH 0/5] "status --ignored"

[Jeff King]

On Sat, Apr 10, 2010 at 12:48:49AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > +	dir.flags |= DIR_COLLECT_IGNORED;
> 
> I thought about using collect-ignored so that fill_directory() does not
> have to be run twice, but IIRC it can short-circuit an entire directory
> using the "simplify" logic (as it was designed to be used for "git add"),
> no?

Hmm.  I didn't consider that. It may be sufficient to say "foo/ is
ignored" and let callers decide for themselves that "foo/bar" is
ignored, too (unless, of course, it appears as tracked earlier in the
list).

I just worry that your version will perform less well. I'm not that
worried about the double traversal (on any reasonable system everything
will be cached after the first traversal). But finding every file means
we have to traverse areas that git otherwise wouldn't look at, which
might mean going to disk to pull in all of the "foo/" directory
structure (which is less likely to be cached, since the rest of git
isn't touching it), even though it may not even be of interest to us.

-Peff

Re: git status --porcelain is a mess that needs fixing

[Julian Phillips]

On Sat, 10 Apr 2010 00:09:59 -0400, Jeff King <peff@peff.net> wrote:
> Your parser is already broken if you are calling split, as the filenames
> may contain spaces (and will be quoted in that case, and you need to
> unmangle). You should use "-z".
> 
> You will probably then realize that the "-z" format looks like:
> 
>   XY file1\0file2\0
> 
> which still sucks. It would be more friendly as:
> 
>   XY\0file1\0file2\0
> 
> So you could split on "\0". But even with that, you can't just blindly
> split, as the column and record separators are the same, and you might
> have one or two filenames.

Not true.  If the second form was used, then you _can_ split on \0.  It
will tokenise the data for you, and then you consume ether two or three
tokens depending on the status flags.  So it would make the parsing
simpler.  But to make it even easier, how about adding a -Z that makes the
output format "XY\0file1\0[file2]\0" (i.e. always three tokens per record,
with the third token being empty if there is no second filename)?  Though
if future expandability was wanted you could end each record with \0\0 and
then parsing would be a two stages of split on \0\0 for records and then
split on \0 for entries?  The is already precedence for the -z option to
change the output format, so a second similar switch should be ok?  Then
the updated documentation could recommend --porcelain -Z for new users
without affecting old ones.

-- 
Julian

Re: [PATCH 0/5] "status --ignored"

[Eric Raymond]

Jeff King <peff@peff.net>:
> On Sat, Apr 10, 2010 at 12:48:49AM -0700, Junio C Hamano wrote:
> 
> > Jeff King <peff@peff.net> writes:
> > 
> > > +	dir.flags |= DIR_COLLECT_IGNORED;
> > 
> > I thought about using collect-ignored so that fill_directory() does not
> > have to be run twice, but IIRC it can short-circuit an entire directory
> > using the "simplify" logic (as it was designed to be used for "git add"),
> > no?
> 
> Hmm.  I didn't consider that. It may be sufficient to say "foo/ is
> ignored" and let callers decide for themselves that "foo/bar" is
> ignored, too (unless, of course, it appears as tracked earlier in the
> list).

Speaking as  a front-end author, I would prefer that all ignored paths 
be listed so I don't have to do globbing.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: git status --porcelain is a mess that needs fixing

[Eric Raymond]

Julian Phillips <julian@quantumfyre.co.uk>:
> On Sat, 10 Apr 2010 00:09:59 -0400, Jeff King <peff@peff.net> wrote:
> > Your parser is already broken if you are calling split, as the filenames
> > may contain spaces (and will be quoted in that case, and you need to
> > unmangle). You should use "-z".
> > 
> > You will probably then realize that the "-z" format looks like:
> > 
> >   XY file1\0file2\0
> > 
> > which still sucks. It would be more friendly as:
> > 
> >   XY\0file1\0file2\0
> > 
> > So you could split on "\0". But even with that, you can't just blindly
> > split, as the column and record separators are the same, and you might
> > have one or two filenames.
> 
> Not true.  If the second form was used, then you _can_ split on \0.  It
> will tokenise the data for you, and then you consume ether two or three
> tokens depending on the status flags.  So it would make the parsing
> simpler.  But to make it even easier, how about adding a -Z that makes the
> output format "XY\0file1\0[file2]\0" (i.e. always three tokens per record,
> with the third token being empty if there is no second filename)?  Though
> if future expandability was wanted you could end each record with \0\0 and
> then parsing would be a two stages of split on \0\0 for records and then
> split on \0 for entries?  The is already precedence for the -z option to
> change the output format, so a second similar switch should be ok?  Then
> the updated documentation could recommend --porcelain -Z for new users
> without affecting old ones.

+1

-Z could fix some of the other issues, as well, like use of space
as a flag character.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: git status --porcelain is a mess that needs fixing

[Jon Seymour]

On Sat, Apr 10, 2010 at 11:35 PM, Julian Phillips
<julian@quantumfyre.co.uk> wrote:
> On Sat, 10 Apr 2010 00:09:59 -0400, Jeff King <peff@peff.net> wrote:
>> Your parser is already broken if you are calling split, as the filenames
>> may contain spaces (and will be quoted in that case, and you need to
>> unmangle). You should use "-z".
>>
>> You will probably then realize that the "-z" format looks like:
>>
>>   XY file1\0file2\0
>>
>> which still sucks. It would be more friendly as:
>>
>>   XY\0file1\0file2\0
>>
>> So you could split on "\0". But even with that, you can't just blindly
>> split, as the column and record separators are the same, and you might
>> have one or two filenames.
>
> Not true.  If the second form was used, then you _can_ split on \0.  It
> will tokenise the data for you, and then you consume ether two or three
> tokens depending on the status flags.  So it would make the parsing
> simpler.  But to make it even easier, how about adding a -Z that makes the
> output format "XY\0file1\0[file2]\0" (i.e. always three tokens per record,
> with the third token being empty if there is no second filename)?  Though
> if future expandability was wanted you could end each record with \0\0 and
> then parsing would be a two stages of split on \0\0 for records and then
> split on \0 for entries?

Surely that won't work - if file2 can be empty, \0[file2]\0 reduces to
\0\0 which would be confused with the \0\0 proposed as a record
separator.

jon.

Re: git status --porcelain is a mess that needs fixing

[Julian Phillips]

On Sun, 11 Apr 2010 00:56:47 +1000, Jon Seymour <jon.seymour@gmail.com>
wrote:
> On Sat, Apr 10, 2010 at 11:35 PM, Julian Phillips
> <julian@quantumfyre.co.uk> wrote:
>> On Sat, 10 Apr 2010 00:09:59 -0400, Jeff King <peff@peff.net> wrote:
>>> Your parser is already broken if you are calling split, as the
filenames
>>> may contain spaces (and will be quoted in that case, and you need to
>>> unmangle). You should use "-z".
>>>
>>> You will probably then realize that the "-z" format looks like:
>>>
>>>   XY file1\0file2\0
>>>
>>> which still sucks. It would be more friendly as:
>>>
>>>   XY\0file1\0file2\0
>>>
>>> So you could split on "\0". But even with that, you can't just blindly
>>> split, as the column and record separators are the same, and you might
>>> have one or two filenames.
>>
>> Not true.  If the second form was used, then you _can_ split on \0.  It
>> will tokenise the data for you, and then you consume ether two or three
>> tokens depending on the status flags.  So it would make the parsing
>> simpler.  But to make it even easier, how about adding a -Z that makes
>> the
>> output format "XY\0file1\0[file2]\0" (i.e. always three tokens per
>> record,
>> with the third token being empty if there is no second filename)?
>>  Though
>> if future expandability was wanted you could end each record with \0\0
>> and
>> then parsing would be a two stages of split on \0\0 for records and
then
>> split on \0 for entries?
> 
> Surely that won't work - if file2 can be empty, \0[file2]\0 reduces to
> \0\0 which would be confused with the \0\0 proposed as a record
> separator.

Yes.  But they were alternative suggestions, so if using \0\0 as the
record marker you would omit the second filename when empty as is currently
done.

-- 
Julian

Re: [PATCH 0/5] "status --ignored"

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> ... But finding every file means
> we have to traverse areas that git otherwise wouldn't look at, which
> might mean going to disk to pull in all of the "foo/" directory
> structure (which is less likely to be cached, since the rest of git
> isn't touching it), even though it may not even be of interest to us.

Yes, that is why it is adequate for us use COLLECT_IGNORED in "git add"
and give "foo/ is outside---you as an intelligent human should be able to
infer that your foo/bar is also" without double traversal.

Eric's tool might want the same abbreviated information if it is just
relaying our output to an intelligent human.  Or it might want to get all
paths if it wants to operate on them itself.  Knowing VC I chose to
illustrate how to do the latter, but in the real implementation, we may
want --show-ignored=normal vs --show-ignored=expanded to support both
uses.

Re: [PATCH 0/5] "status --ignored"

[Jakub Narebski]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
> 
> > ... But finding every file means
> > we have to traverse areas that git otherwise wouldn't look at, which
> > might mean going to disk to pull in all of the "foo/" directory
> > structure (which is less likely to be cached, since the rest of git
> > isn't touching it), even though it may not even be of interest to us.
> 
> Yes, that is why it is adequate for us use COLLECT_IGNORED in "git add"
> and give "foo/ is outside---you as an intelligent human should be able to
> infer that your foo/bar is also" without double traversal.
> 
> Eric's tool might want the same abbreviated information if it is just
> relaying our output to an intelligent human.  Or it might want to get all
> paths if it wants to operate on them itself.  Knowing VC I chose to
> illustrate how to do the latter, but in the real implementation, we may
> want --show-ignored=normal vs --show-ignored=expanded to support both
> uses.

Or rather -i / --ignored-files[=(no|normal|all)], default to 'normal' 
in the '--ignored-files' form (without optional <mode>), similarly
to currently existing -u / --untracked-files option to git-status.

Let's not introduce yet another CLI inconsistency^TM in Git... ;-)
-- 
Jakub Narebski
Poland
ShadeHawk on #git

[RFC/PATCH] status: Add a new NUL separated output format

[Julian Phillips]

Add a new output format option to git-status that is a more extreme
form of the -z output that places a NUL between all parts of the
record, and always has three entries per record, even when only two
are relevant.  This make the parsing of --porcelain output much
simpler for the consumer.

Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>
---

On Sat, 10 Apr 2010, Julian Phillips wrote:

> Not true.  If the second form was used, then you _can_ split on \0.  It
> will tokenise the data for you, and then you consume ether two or three
> tokens depending on the status flags.  So it would make the parsing
> simpler.  But to make it even easier, how about adding a -Z that makes the
> output format "XY\0file1\0[file2]\0" (i.e. always three tokens per record,
> with the third token being empty if there is no second filename)?  Though
> if future expandability was wanted you could end each record with \0\0 and
> then parsing would be a two stages of split on \0\0 for records and then
> split on \0 for entries?  The is already precedence for the -z option to
> change the output format, so a second similar switch should be ok?  Then
> the updated documentation could recommend --porcelain -Z for new users
> without affecting old ones.

Something like this for the first variant (fixed three entries per record)
perhaps ... (though a proper patch would probably want some tests too)

 builtin/commit.c |    6 ++++--
 wt-status.c      |   19 ++++++++++++++-----
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index c5ab683..acbcefc 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1025,8 +1025,10 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT(0, "porcelain", &status_format,
 			    "show porcelain output format",
 			    STATUS_FORMAT_PORCELAIN),
-		OPT_BOOLEAN('z', "null", &null_termination,
-			    "terminate entries with NUL"),
+		OPT_SET_INT('z', "null", &null_termination,
+			    "terminate entries with NUL", 1),
+		OPT_SET_INT('Z', "intense-null", &null_termination,
+			    "use NUL for all seperators, including absent values", 2),
 		{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg,
 		  "mode",
 		  "show untracked files, optional modes: all, normal, no. (Default: all)",
diff --git a/wt-status.c b/wt-status.c
index 8ca59a2..9f23ec6 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -663,7 +663,9 @@ static void wt_shortstatus_unmerged(int null_termination, struct string_list_ite
 	case 7: how = "UU"; break; /* both modified */
 	}
 	color_fprintf(s->fp, color(WT_STATUS_UNMERGED, s), "%s", how);
-	if (null_termination) {
+	if (null_termination == 2) {
+		fprintf(stdout, "%c%s%c%c", 0, it->string, 0, 0);
+	} else if (null_termination) {
 		fprintf(stdout, " %s%c", it->string, 0);
 	} else {
 		struct strbuf onebuf = STRBUF_INIT;
@@ -687,14 +689,19 @@ static void wt_shortstatus_status(int null_termination, struct string_list_item
 		color_fprintf(s->fp, color(WT_STATUS_CHANGED, s), "%c", d->worktree_status);
 	else
 		putchar(' ');
-	putchar(' ');
-	if (null_termination) {
-		fprintf(stdout, "%s%c", it->string, 0);
+	if (null_termination == 2) {
+		char *file2 = "";
+		if (d->head_path)
+			file2 = d->head_path;
+		fprintf(stdout, "%c%s%c%s%c", 0, it->string, 0, file2, 0);
+	} else if (null_termination) {
+		fprintf(stdout, " %s%c", it->string, 0);
 		if (d->head_path)
 			fprintf(stdout, "%s%c", d->head_path, 0);
 	} else {
 		struct strbuf onebuf = STRBUF_INIT;
 		const char *one;
+		putchar(' ');
 		if (d->head_path) {
 			one = quote_path(d->head_path, -1, &onebuf, s->prefix);
 			printf("%s -> ", one);
@@ -709,7 +716,9 @@ static void wt_shortstatus_status(int null_termination, struct string_list_item
 static void wt_shortstatus_untracked(int null_termination, struct string_list_item *it,
 			    struct wt_status *s)
 {
-	if (null_termination) {
+	if (null_termination == 2) {
+		fprintf(stdout, "??%c%s%c%c", 0, it->string, 0, 0);
+	} else if (null_termination) {
 		fprintf(stdout, "?? %s%c", it->string, 0);
 	} else {
 		struct strbuf onebuf = STRBUF_INIT;
-- 
1.7.0.4

Re: [RFC/PATCH] status: Add a new NUL separated output format

[Eric Raymond]

Julian Phillips <julian@quantumfyre.co.uk>:
> Add a new output format option to git-status that is a more extreme
> form of the -z output that places a NUL between all parts of the
> record, and always has three entries per record, even when only two
> are relevant.  This make the parsing of --porcelain output much
> simpler for the consumer.

If you're open to changing this to lose the exiguous "-> " and use "-"
instead of " " as a status character, that would make me happy 
and fix the rest of the design problems with the format.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

Re: [RFC/PATCH] status: Add a new NUL separated output format

[Julian Phillips]

On Sat, 10 Apr 2010 15:50:03 -0400, Eric Raymond <esr@thyrsus.com> wrote:
> Julian Phillips <julian@quantumfyre.co.uk>:
>> Add a new output format option to git-status that is a more extreme
>> form of the -z output that places a NUL between all parts of the
>> record, and always has three entries per record, even when only two
>> are relevant.  This make the parsing of --porcelain output much
>> simpler for the consumer.
> 
> If you're open to changing this to lose the exiguous "-> " and use "-"
> instead of " " as a status character, that would make me happy 
> and fix the rest of the design problems with the format.

If you use "--porcelain -Z" then you don't get the "->", the format is
always XY<NUL><file1><NUL><file2><NUL>, with <file2> being an empty string
if only file1 is relevant.

I didn't use "-" instead of " " as that seemed out of scope for a output
formatting option.  Though I don't personally have an objection to it, I
also don't see a particularly strong need for it as with the -Z format
there is no ambiguity.

If you're talking about the output without -Z, then changing the format
raises compatibility issues, and were talking about something more like
--porcelain2 or --porcelain=new and I don't know if that would be
considered acceptable.

-- 
Julian

Re: [RFC/PATCH] status: Add a new NUL separated output format

[Eric Raymond]

Julian Phillips <julian@quantumfyre.co.uk>:
> I didn't use "-" instead of " " as that seemed out of scope for a output
> formatting option.  Though I don't personally have an objection to it, I
> also don't see a particularly strong need for it as with the -Z format
> there is no ambiguity.

Good point.  OK, the combinaation of -Z and a switch to list ignored
files should solve Emacs VC's problem.  

Having some sort of JSON dump might still not be a bad idea.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

[RFC/PATCH] status: Add json output format

[Julian Phillips]

This adds a --json switch to status, which enables a json output
format.  This provides a standard output format that should be easily
parsed by scripts using any of the large number of readily available
json libraries.

Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>
---
On Sat, 10 Apr 2010, Eric Raymond wrote:

> Julian Phillips <julian@quantumfyre.co.uk>:
>> I didn't use "-" instead of " " as that seemed out of scope for a output
>> formatting option.  Though I don't personally have an objection to it, I
>> also don't see a particularly strong need for it as with the -Z format
>> there is no ambiguity.
>
> Good point.  OK, the combinaation of -Z and a switch to list ignored
> files should solve Emacs VC's problem.
>
> Having some sort of JSON dump might still not be a bad idea.

Starter for 10 ...

 builtin/commit.c |   10 ++++
 wt-status.c      |  132 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 wt-status.h      |    1 +
 3 files changed, 143 insertions(+), 0 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index c5ab683..f2b5cfa 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -91,6 +91,7 @@ static enum {
 	STATUS_FORMAT_LONG,
 	STATUS_FORMAT_SHORT,
 	STATUS_FORMAT_PORCELAIN,
+	STATUS_FORMAT_JSON,
 } status_format = STATUS_FORMAT_LONG;
 
 static int opt_parse_m(const struct option *opt, const char *arg, int unset)
@@ -422,6 +423,9 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
 	case STATUS_FORMAT_PORCELAIN:
 		wt_porcelain_print(s, null_termination);
 		break;
+	case STATUS_FORMAT_JSON:
+		wt_json_print(s);
+		break;
 	case STATUS_FORMAT_LONG:
 		wt_status_print(s);
 		break;
@@ -1025,6 +1029,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT(0, "porcelain", &status_format,
 			    "show porcelain output format",
 			    STATUS_FORMAT_PORCELAIN),
+		OPT_SET_INT(0, "json", &status_format,
+			    "show json output format",
+			    STATUS_FORMAT_JSON),
 		OPT_BOOLEAN('z', "null", &null_termination,
 			    "terminate entries with NUL"),
 		{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg,
@@ -1068,6 +1075,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	case STATUS_FORMAT_PORCELAIN:
 		wt_porcelain_print(&s, null_termination);
 		break;
+	case STATUS_FORMAT_JSON:
+		wt_json_print(&s);
+		break;
 	case STATUS_FORMAT_LONG:
 		s.verbose = verbose;
 		wt_status_print(&s);
diff --git a/wt-status.c b/wt-status.c
index 8ca59a2..ab934e7 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -750,3 +750,135 @@ void wt_porcelain_print(struct wt_status *s, int null_termination)
 	s->prefix = NULL;
 	wt_shortstatus_print(s, null_termination);
 }
+
+static char *json_quote(char *s)
+{
+	struct strbuf buf = STRBUF_INIT;
+
+	while (*s) {
+		switch (*s) {
+		case '"':
+			strbuf_addstr(&buf, "\\\"");
+			break;
+		case '\\':
+			strbuf_addstr(&buf, "\\\\");
+			break;
+		case '\b':
+			strbuf_addstr(&buf, "\\b");
+			break;
+		case '\f':
+			strbuf_addstr(&buf, "\\f");
+			break;
+		case '\n':
+			strbuf_addstr(&buf, "\\n");
+			break;
+		case '\r':
+			strbuf_addstr(&buf, "\\r");
+			break;
+		case '\t':
+			strbuf_addstr(&buf, "\\t");
+			break;
+		default:
+			/* All control characters must be encode, even if they
+			 * don't have a specific escape character of their own */
+			if (*s < 0x20)
+				strbuf_addf(&buf, "\\u%04x", *s);
+			else
+				strbuf_addch(&buf, *s);
+			break;
+		}
+		s++;
+	}
+
+	return strbuf_detach(&buf, NULL);
+}
+
+static void wt_json_unmerged(struct string_list_item *it,
+			   struct wt_status *s)
+{
+	struct wt_status_change_data *d = it->util;
+	char ours = '?', theirs = '?';
+	char *name = json_quote(it->string);
+
+	switch (d->stagemask) {
+	case 1: ours = 'D'; theirs = 'D'; break; /* both deleted */
+	case 2: ours = 'A'; theirs = 'U'; break; /* added by us */
+	case 3: ours = 'U'; theirs = 'D'; break; /* deleted by them */
+	case 4: ours = 'U'; theirs = 'A'; break; /* added by them */
+	case 5: ours = 'D'; theirs = 'U'; break; /* deleted by us */
+	case 6: ours = 'A'; theirs = 'A'; break; /* both added */
+	case 7: ours = 'U'; theirs = 'U'; break; /* both modified */
+	}
+
+	fprintf(stdout, "{");
+	fprintf(stdout, "\"ours\" : \"%c\", ", ours);
+	fprintf(stdout, "\"theirs\" : \"%c\", ", theirs);
+	fprintf(stdout, "\"name\" : \"%s\"", name);
+	fprintf(stdout, "}");
+
+	free(name);
+}
+
+static void wt_json_status(struct string_list_item *it,
+			 struct wt_status *s)
+{
+	struct wt_status_change_data *d = it->util;
+	char index = '-', worktree = '-';
+	char *name = json_quote(it->string);
+
+	if (d->index_status)
+		index = d->index_status;
+	if (d->worktree_status)
+		worktree = d->worktree_status;
+
+	fprintf(stdout, "{");
+	fprintf(stdout, "\"index\" : \"%c\", ", index);
+	fprintf(stdout, "\"worktree\" : \"%c\", ", worktree);
+	fprintf(stdout, "\"name\" : \"%s\"", name);
+
+	if (d->head_path) {
+		free(name);
+		name = json_quote(d->head_path);
+		fprintf(stdout, ", \"orig_name\" : \"%s\"", name);
+	}
+
+	fprintf(stdout, "}");
+
+	free(name);
+}
+
+void wt_json_print(struct wt_status *s)
+{
+	int i;
+	fprintf(stdout, "[");
+	for (i = 0; i < s->change.nr; i++) {
+		struct wt_status_change_data *d;
+		struct string_list_item *it;
+
+		if (i > 0)
+			fprintf(stdout, ",\n");
+		it = &(s->change.items[i]);
+		d = it->util;
+		if (d->stagemask)
+			wt_json_unmerged(it, s);
+		else
+			wt_json_status(it, s);
+	}
+	if (s->change.nr > 0 && s->untracked.nr > 0)
+		fprintf(stdout, ",\n");
+	for (i = 0; i < s->untracked.nr; i++) {
+		char *name = json_quote(s->untracked.items[i].string);
+
+		if (i > 0)
+			fprintf(stdout, ",\n");
+
+		fprintf(stdout, "{");
+		fprintf(stdout, "\"index\" : \"?\", ");
+		fprintf(stdout, "\"worktree\" : \"?\", ");
+		fprintf(stdout, "\"name\" : \"%s\"", name);
+		fprintf(stdout, "}");
+
+		free(name);
+	}
+	fprintf(stdout, "]\n");
+}
diff --git a/wt-status.h b/wt-status.h
index 9120673..effd3fd 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -61,5 +61,6 @@ void wt_status_collect(struct wt_status *s);
 
 void wt_shortstatus_print(struct wt_status *s, int null_termination);
 void wt_porcelain_print(struct wt_status *s, int null_termination);
+void wt_json_print(struct wt_status *s);
 
 #endif /* STATUS_H */
-- 
1.7.0.4

Re: git status --porcelain is a mess that needs fixing

[Jon Seymour]

On Sun, Apr 11, 2010 at 1:50 AM, Julian Phillips
<julian@quantumfyre.co.uk> wrote:
> On Sun, 11 Apr 2010 00:56:47 +1000, Jon Seymour <jon.seymour@gmail.com>
> wrote:
>> On Sat, Apr 10, 2010 at 11:35 PM, Julian Phillips
>> <julian@quantumfyre.co.uk> wrote:
>>> ...
>>> tokens depending on the status flags.  So it would make the parsing
>>> simpler.  But to make it even easier, how about adding a -Z that makes
>>> the
>>> output format "XY\0file1\0[file2]\0" (i.e. always three tokens per
>>> record,
>>> with the third token being empty if there is no second filename)?
>>>  Though
>>> if future expandability was wanted you could end each record with \0\0
>>> and
>>> then parsing would be a two stages of split on \0\0 for records and
> then
>>> split on \0 for entries?
>>
>> Surely that won't work - if file2 can be empty, \0[file2]\0 reduces to
>> \0\0 which would be confused with the \0\0 proposed as a record
>> separator.
>
> Yes.  But they were alternative suggestions, so if using \0\0 as the
> record marker you would omit the second filename when empty as is currently
> done.

Ah, apologies. I appear to have failed to parse a necessary disjunctive :-)

jon.

Re: [PATCH 0/5] "status --ignored"

[Jeff King]

On Sat, Apr 10, 2010 at 11:27:39AM -0700, Junio C Hamano wrote:

> > ... But finding every file means
> > we have to traverse areas that git otherwise wouldn't look at, which
> > might mean going to disk to pull in all of the "foo/" directory
> > structure (which is less likely to be cached, since the rest of git
> > isn't touching it), even though it may not even be of interest to us.
> 
> Yes, that is why it is adequate for us use COLLECT_IGNORED in "git add"
> and give "foo/ is outside---you as an intelligent human should be able to
> infer that your foo/bar is also" without double traversal.

It would be adequate here, too, if we want to know whether a specific
file is ignored. It just takes more work from the caller. But Eric has
already said he would prefer to just get all files, so let's go with
what you wrote.

If another caller wants a more restricted but efficient interface later,
we can add it as you suggest (or more likely, they want to check a
particular _set_ of files, so we don't want to do a full traversal
anyway. We would want to get their list of candidates and traverse just
enough of the .gitignore stack to get answers for their set).

-Peff