From: Junio C Hamano <hidden> Date: 2016-06-15 22:48:37
Jakub Narebski [off-list ref] writes:
Well, this whole idea started with the fact, that "git status --short"
was hard (or impossible) to parse unambigously by scripts[1], and even
"git status --porcelain -z"[2] is not that easy to parse[3].
And you apparently seem to agree with that claim, but I don't. I think
Jeff (who did the --porcelain stuff; by the way, why did we lose him from
Cc list?) has already said that he is open to an update.
From: Jeff King <hidden> Date: 2016-06-15 22:48:37
On Wed, Apr 14, 2010 at 02:34:01PM -0700, Junio C Hamano wrote:
Jakub Narebski [off-list ref] writes:
quoted
Well, this whole idea started with the fact, that "git status --short"
was hard (or impossible) to parse unambigously by scripts[1], and even
"git status --porcelain -z"[2] is not that easy to parse[3].
And you apparently seem to agree with that claim, but I don't. I think
Jeff (who did the --porcelain stuff; by the way, why did we lose him from
Cc list?) has already said that he is open to an update.
I haven't seen any evidence that status --porcelain (or its -z form) is
impossible to parse unambiguously. I don't even think it's that hard,
but it certainly could be easier. But more importantly, from looking at
the output it's not necessarily _obvious_ how to parse it correctly
(e.g., whitespace as value and as field separator, syntax of "-z"
depends on semantics of field contents).
The approach I proposed was to leave it be and document it a bit better.
Adding some format that is close but subtly different is just going to
lead to more confusion.
But since Julian was willing to do the JSON work, I think that is a much
nicer approach. It's not subtly different; it's very different and way
easier to read and parse. And I'm really happy with the way he has
structured the code to handle multiple output formats. It keeps the code
much cleaner, and it should silence any "but YAML is better than JSON is
better than XML" debates.
Even with Julian's patches, we should still better document the regular
and "-z" forms. Eric promised to send some patches this week; I'm hoping
he is still interested in doing so after seeing a better solution arise.
:)
-Peff
From: Jakub Narebski <hidden> Date: 2016-06-15 22:48:37
On Thu, 15 April 2010, Jeff King wrote:
On Wed, Apr 14, 2010 at 02:34:01PM -0700, Junio C Hamano wrote:
quoted
Jakub Narebski [off-list ref] writes:
quoted
Well, this whole idea started with the fact, that "git status --short"
was hard (or impossible) to parse unambigously by scripts[1], and even
"git status --porcelain -z"[2] is not that easy to parse[3].
And you apparently seem to agree with that claim, but I don't. I think
Jeff (who did the --porcelain stuff; by the way, why did we lose him from
Cc list?) has already said that he is open to an update.
I haven't seen any evidence that status --porcelain (or its -z form) is
impossible to parse unambiguously. I don't even think it's that hard,
but it certainly could be easier. But more importantly, from looking at
the output it's not necessarily _obvious_ how to parse it correctly
(e.g., whitespace as value and as field separator, syntax of "-z"
depends on semantics of field contents).
Well, IMVHO output of "git status --short" / "git status --porcelain"
(without '-z') is very hard to parse. Even assuming that in the case
of ambiguity filenames are quoted (which also means that in the case of
ambiguity whether they are quoted they must be quoted), the fact that
separator between source and destination filename in the case of rename
detection is " -> " (if I understand it correctly), and neither of ' '
(SPC), '-' nor '>' is replaced by escape sequence means that one needs
to detect where quoted filename begins and where ends. This means
either parsing character by character, taking into account quoting and
escaping (e.g. '\\', '\"' etc.), or using 'balanced quote' regexp like
the one from Text::Balanced, e.g.: (?:\"(?:[^\\\"]*(?:\\.[^\\\"]*)*)\")
What was the reason behind choosing " -> " as separator between pair[1]
of filenames in rename, instead of using default "git diff --stat" format
i.e. 'arch/{i386 => x86}/Makefile' for "git status --short" which is
meant for end user, and for "git status --porcelain" the same format
that raw diff format, i.e. with TAB as separator between filenames,
and filename quited if it contains TAB (then TAB is relaced by '\t',
and does not appear in filename, therefore you can split on TAB)?
IMVHO "git status --porcelain -z" format is not easy to parse either.
(The same can be said for "git diff --raw -z" output format.) You
can't just split on record separator; you have to take into account
status to check if there are two filenames or one.
[1] A question: we have working area version, index version, and HEAD
version of file. Isn't it possible for *each* of them to have
different filename? What about the case of rename/rename merge
conflict?
The approach I proposed was to leave it be and document it a bit better.
Adding some format that is close but subtly different is just going to
lead to more confusion.
Well, the proposed '-Z' output format, in the OFS="\0", ORS="\0\0"
variant, would be very easy to parse. If I understand it correctly
it is also one of available format in outputification^W in this series.
But since Julian was willing to do the JSON work, I think that is a much
nicer approach. It's not subtly different; it's very different and way
easier to read and parse. And I'm really happy with the way he has
structured the code to handle multiple output formats. It keeps the code
much cleaner, and it should silence any "but YAML is better than JSON is
better than XML" debates.
I really like this outputification ;-) too.
Although if possible I'd like to have it wrapped in utility macros,
like parseopt, so one does not need to write output_str / output_int
etc.... but currently it is very, very vague sketch of an idea, rather
than realized concept.
Even with Julian's patches, we should still better document the regular
and "-z" forms. Eric promised to send some patches this week; I'm hoping
he is still interested in doing so after seeing a better solution arise.
:)
From: Jeff King <hidden> Date: 2016-06-15 22:48:39
On Thu, Apr 15, 2010 at 11:07:32AM +0200, Jakub Narebski wrote:
Well, IMVHO output of "git status --short" / "git status --porcelain"
(without '-z') is very hard to parse. Even assuming that in the case
of ambiguity filenames are quoted (which also means that in the case of
ambiguity whether they are quoted they must be quoted), the fact that
For the record, they are properly quoted in the non-z form.
[some reasons it's hard to parse]
Yeah, I don't disagree with your reasons (which are largely the same as
Eric's). I just don't think it's "oh no this is useless and we have to
start again" hard.
What was the reason behind choosing " -> " as separator between pair[1]
of filenames in rename, instead of using default "git diff --stat" format
i.e. 'arch/{i386 => x86}/Makefile' for "git status --short" which is
meant for end user, and for "git status --porcelain" the same format
that raw diff format, i.e. with TAB as separator between filenames,
and filename quited if it contains TAB (then TAB is relaced by '\t',
and does not appear in filename, therefore you can split on TAB)?
I don't know Junio's reason for using " -> " in --short; probably
because it was the format used in non-short status. For --porcelain, it
was simply because I used exactly --short. I assumed that --short was
suitable for parsing (which it _is_, it just has some rough edges), and
wanted to provide an option right away that would keep the output
stable, so we didn't run into the usual problem of people wanting to
enhance the human-readable interface, but being blocked by script
compatibility.
IMVHO "git status --porcelain -z" format is not easy to parse either.
(The same can be said for "git diff --raw -z" output format.) You
can't just split on record separator; you have to take into account
status to check if there are two filenames or one.
Yep, I agree. I think the JSON approach is the best solution, as it is
separating syntax from semantics.
[1] A question: we have working area version, index version, and HEAD
version of file. Isn't it possible for *each* of them to have
different filename? What about the case of rename/rename merge
conflict?
Good question. The answer is no, the three different versions can't have
three filenames on the same line, because we don't do rename detection
between the working tree and the index. Which makes sense. Consider
something like this:
mkdir repo && cd repo && git init
echo content >one
git add one && git commit -m one
mv one two && git add -A
mv two three
git status
We will see the movement of "one -> two" between the index and HEAD. In
theory we could see the movement of "three -> two" between the index and
working tree. But "three" isn't tracked, so instead we see "two" deleted
and "three" untracked. We can mark "three" with intent-to-add to note
that we are interested in it, but then it is not a new file any more
(since it has an index entry), and is therefore not eligible for rename
detection.
As for a rename/rename conflict, it gets represented in the index as
both deleting the source and then each side adding its new version with
a conflict. So:
mkdir repo && cd repo && git init
echo content >one
git add one && git commit -m base
git mv one two && git commit -m two
git checkout -b other HEAD^
git mv one three && git commit -m three
git merge master
git status
generates:
# On branch other
# Unmerged paths:
# both deleted: one
# added by us: three
# added by them: two
and an equivalent short-status form.
Although if possible I'd like to have it wrapped in utility macros,
like parseopt, so one does not need to write output_str / output_int
etc.... but currently it is very, very vague sketch of an idea, rather
than realized concept.
I'm not sure I understand what utility macros you would want.
-Peff
From: Jakub Narebski <hidden> Date: 2016-06-15 22:48:39
On Sat, 17 Apr 2010, Jeff King wrote:
On Thu, Apr 15, 2010 at 11:07:32AM +0200, Jakub Narebski wrote:
quoted
[1] A question: we have working area version, index version, and HEAD
version of file. Isn't it possible for *each* of them to have
different filename? What about the case of rename/rename merge
conflict?
[cut]
Thanks for detailed explanation.
quoted
Although if possible I'd like to have it wrapped in utility macros,
like parseopt, so one does not need to write output_str / output_int
etc.... but currently it is very, very vague sketch of an idea, rather
than realized concept.
I'm not sure I understand what utility macros you would want.
Something like that (please remember that it is still in vague beginnings
of an idea stage:
OUT_OBJECT(
OUT_FIELD("mode", OUT_MODE, tree.mode), SP,
OUT_FIELD("type", "%s", tree.object.type), SP,
OUT_FIELD("object", OUT_SHA1, tree.object.sha1), TAB,
OUT_FIELD("file", OUT_FILE(sep), tree.filename),
sep
);
--
Jakub Narebski
Poland
From: Jeff King <hidden> Date: 2016-06-15 22:48:39
On Sat, Apr 17, 2010 at 03:02:39PM +0200, Jakub Narebski wrote:
Something like that (please remember that it is still in vague beginnings
of an idea stage:
OUT_OBJECT(
OUT_FIELD("mode", OUT_MODE, tree.mode), SP,
OUT_FIELD("type", "%s", tree.object.type), SP,
OUT_FIELD("object", OUT_SHA1, tree.object.sha1), TAB,
OUT_FIELD("file", OUT_FILE(sep), tree.filename),
sep
);
Doing that would require variadic macros, which are a C99-ism. So you
would have to do:
OUT_OBJECT_START();
OUT_FIELD("mode", OUT_MODE, tree.mode); OUT_SP;
...
OUT_OBJECT_END();
which is not all that different from what Julian has now. I do think
some type-specific conversions might be handy. They don't even need to
be macros. E.g.,:
void output_mode(struct output_context *oc, int mode)
{
output_strf(oc, "mode", "%06o", mode);
}
OTOH, looking over Julian's last patch series, there really aren't that
many that would be generally applicable, and as you can see they only
save a few characters, not even a line. A few bigger objects could be
factored out, but he has already done that (e.g., see
wt_porcelain_unmerged in his v2 3/4).
-Peff
On Sat, 17 Apr 2010 10:00:53 -0400, Jeff King [off-list ref] wrote:
On Sat, Apr 17, 2010 at 03:02:39PM +0200, Jakub Narebski wrote:
quoted
Something like that (please remember that it is still in vague
beginnings
quoted
of an idea stage:
OUT_OBJECT(
OUT_FIELD("mode", OUT_MODE, tree.mode), SP,
OUT_FIELD("type", "%s", tree.object.type), SP,
OUT_FIELD("object", OUT_SHA1, tree.object.sha1), TAB,
OUT_FIELD("file", OUT_FILE(sep), tree.filename),
sep
);
Doing that would require variadic macros, which are a C99-ism. So you
would have to do:
OUT_OBJECT_START();
OUT_FIELD("mode", OUT_MODE, tree.mode); OUT_SP;
...
OUT_OBJECT_END();
Also, backends such as JSON want to know which things are strings, and
which are numbers - as they print differently. An XML backend may want to
distinguish even more (though I guess that depends on the design).
which is not all that different from what Julian has now. I do think
some type-specific conversions might be handy. They don't even need to
be macros. E.g.,:
void output_mode(struct output_context *oc, int mode)
{
output_strf(oc, "mode", "%06o", mode);
}
OTOH, looking over Julian's last patch series, there really aren't that
many that would be generally applicable, and as you can see they only
save a few characters, not even a line. A few bigger objects could be
factored out, but he has already done that (e.g., see
wt_porcelain_unmerged in his v2 3/4).
It might help standardise the output between commands if there were helper
functions for some of the larger structures - e.g. commits. Though I don't
think that those functions would be able to do legacy output, due to the
current lack of cross-command output compatibility. I'm starting to see
this with blame and diff-tree (and family), where they both want to output
information about commits.
I think that maybe I need to design and document the output structure for
common concepts - so that it would be possible to pass the output from any
command to a common parser, with matching utility functions in the code.
Though, I'm not sure if there actually are any common concepts that need
outputting apart from commits.
Current Status
--------------
I had been planning to post an updated series this weekend, but I'm too
tired to attempt tidying things up for posting at the moment ... If you
want to see the current state then my current mess is available at
http://git.q42.co.uk/w/output.git.
A quick summary of main changes since v2:
- backends are now in a subdirectory
- blame, diff-tree, have --ouptut=... support for plumbing output
- log has some support for --ouput=...
- output library has extended API, including quoted strings and
is_structured_output function
- backend API includes explicit functions for top-level items
--
Julian
From: Jeff King <hidden> Date: 2016-06-15 22:48:40
On Sun, Apr 18, 2010 at 10:46:18PM +0100, Julian Phillips wrote:
It might help standardise the output between commands if there were helper
functions for some of the larger structures - e.g. commits. Though I don't
think that those functions would be able to do legacy output, due to the
current lack of cross-command output compatibility. I'm starting to see
this with blame and diff-tree (and family), where they both want to output
information about commits.
Yeah, that was what I saw on looking at the code. And we have to support
those old formats, obviously. For the most part, I found the level of
verbosity in the patches you posted (and I just peeked at your repo) to
be fine. Sure, it's more lines, but they're IMHO very easy to read.
If we have to tradeoff between either duplicating output entirely (for
both the output form and traditional form) or having a more flexible but
slightly more verbose output library, I think I would rather go with the
latter. It will be more maintainable in the long run.
-Peff