* Re: [PATCH] Have a flag to stop the option parsing at the first argument.
From: Johannes Schindelin @ 2007-12-17 11:50 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Wincent Colaiuta, Junio C Hamano, git
In-Reply-To: <20071217114703.GH7453@artemis.madism.org>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2208 bytes --]
Hi,
On Mon, 17 Dec 2007, Pierre Habouzit wrote:
> On Mon, Dec 17, 2007 at 11:10:00AM +0000, Wincent Colaiuta wrote:
> > El 17/12/2007, a las 10:50, Pierre Habouzit escribió:
> >
> > >Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> > >---
> > >> // ...
> > >> /* when in `git --opt1 --opt2 foo -a -b -c` mode: */
> > >> int cmd_pos = git_find_builtin_command_name(argc, argv);
> > >> int count = parse_options(cmd_pos, argv, git_generic_options,
> > >> "git [special-options] cmd [options]", 0);
> > >> if (count)
> > >> die("unknown git command: `%s`", argv[0]);
> > >> argv += cmd_pos;
> > >> argc -= cmd_pos;
> > >> /* here we simulate an argv of {"foo", "-a", "-b", "-c"} */
> > >
> > > Or even simpler, with the following specifically tailored patch you
> > > can directly write:
> > >
> > > argc = parse_options(argc, argv, git_generic_options,
> > > "git [generic-options] <command> [cmd-options]",
> > > PARSE_OPT_STOP_AT_ARG);
> > >
> > > and then {argc, argv} will exactly be the NULL-terminated array
> > > starting with the builtin command. Kind of nice :)
> >
> > Indeed, nice ideas. I think all this will lead to a nice UI improvement
> > post-1.5.4.
> >
> > About the only thing that I think would merit action *prior* to 1.5.4 is
> > marking the "-p" switch to git (synonym for --paginate) as deprecated,
> > see as it clashes with other commands' uses of that switch ("git log -p"
> > for example). Are there any other conflicting specials that a currently
> > parsed in git.c?
>
> You don't need to, and I'd see that as a regression. With my proposal,
> there isn't any kind of need that git commands do not clash with git
> ones. The parse-option mechanism will properly hide options that are
> masked this way, dscho wrote the patch for that.
>
> git -p log -p ...
>
> just makes sense to me. CVS or SVN e.g. (don't hit me !) have the same
> kind of "issues", and I never found that weird.
>
> In fact I see this the other way around: git status -p that is in fact
> the same as git -p status, is the conveniency, git -p status is the
> canonical form.
I would even go further: "git status -p" looks utterly wrong to me.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.
From: Pierre Habouzit @ 2007-12-17 11:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7v7ijda81h.fsf@gitster.siamese.dyndns.org>
On Mon, Dec 17, 2007 at 11:13:14AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
>
> > Okay this is kind of disgusting, and I'm absolutely not pleased with
> > it (I mean I'm not pleased that parse_opt forces us to write things like
> > that). This hack allows:
> >
> > git tag -l -n10 <pattern>
> >
> > and will then attach the <pattern> to the '-l' switch,...
>
> Heh, it turns out that we were both stupid and blind.
[...]
indeed, but then this happens to be a better patch than yours IMHO:
>From 5a3cdd255f17c7d7bc9245881f0d50146413113f Mon Sep 17 00:00:00 2001
From: Pierre Habouzit <madcoder@debian.org>
Date: Mon, 17 Dec 2007 12:54:55 +0100
Subject: [PATCH] git-tag: fix -l switch handling regression.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
builtin-tag.c | 12 +++++-------
1 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/builtin-tag.c b/builtin-tag.c
index 274901a..219633d 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -16,7 +16,7 @@
static const char * const git_tag_usage[] = {
"git-tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
"git-tag -d <tagname>...",
- "git-tag [-n [<num>]] -l [<pattern>]",
+ "git-tag -l [-n [<num>]] [<pattern>]",
"git-tag -v <tagname>...",
NULL
};
@@ -370,13 +370,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct ref_lock *lock;
int annotate = 0, sign = 0, force = 0, lines = 0,
- delete = 0, verify = 0;
- char *list = NULL, *msgfile = NULL, *keyid = NULL;
- const char *no_pattern = "NO_PATTERN";
+ list = 0, delete = 0, verify = 0;
+ char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct option options[] = {
- { OPTION_STRING, 'l', NULL, &list, "pattern", "list tag names",
- PARSE_OPT_OPTARG, NULL, (intptr_t) no_pattern },
+ OPT_INTEGER('l', NULL, &list, "list tag names"),
{ OPTION_INTEGER, 'n', NULL, &lines, NULL,
"print n lines of each tag message",
PARSE_OPT_OPTARG, NULL, 1 },
@@ -408,7 +406,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
annotate = 1;
if (list)
- return list_tags(list == no_pattern ? NULL : list, lines);
+ return list_tags(argv[0], lines);
if (delete)
return for_each_tag_name(argv, delete_tag);
if (verify)
--
debian.1.5.3.7.1-dirty
^ permalink raw reply related
* Re: [PATCH] Re-re-re-fix common tail optimization
From: Johannes Schindelin @ 2007-12-17 11:57 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Jeff King, Junio C Hamano, Linus Torvalds, git
In-Reply-To: <36E62F9B-26FF-4DC0-99B8-D6DC2B960E67@wincent.com>
Hi,
On Mon, 17 Dec 2007, Wincent Colaiuta wrote:
> El 17/12/2007, a las 11:39, Johannes Schindelin escribi?:
>
> > On Mon, 17 Dec 2007, Wincent Colaiuta wrote:
> >
> > > El 16/12/2007, a las 23:29, Jeff King escribi?:
> > >
> > > > On Sun, Dec 16, 2007 at 02:23:27PM -0800, Junio C Hamano wrote:
> > > >
> > > > > > Aren't we using "git diff" for the second diff there nowadays?
> > > > >
> > > > > Some people seem to think that is a good idea, but I generally
> > > > > do not like using "git diff" between expect and actual (both
> > > > > untracked) inside tests. The last "diff" is about validating
> > > > > what git does and using "git diff" there would make the test
> > > > > meaningless when "git diff" itself is broken.
> > > >
> > > > I think that is a valid concern. But ISTR that were some issues
> > > > with using GNU diff. Commit 5bd74506 mentions getting rid of the
> > > > dependency in all existing tests, but gives no reason.
> > >
> > > I'd say it's safe and sensible to use "git diff" in all tests
> > > *except* for tests of "git diff" itself.
> >
> > To the contrary. It has to test "git diff", so it must use "git
> > diff".
>
> Obviously, you can only test "git diff" by actually running it.
Sorry, I should have made clear that I meant this as funny:
;-)
> > As for the reference output: we include the expected diffs as texts,
> > and therefore do not really have to rely on having GNU diff installed.
> >
> > Besides, we cannot even test the goodies like "rename from" by
> > comparing to GNU diff's output.
>
> Sorry, I didn't make myself clear. That's not what I was proposing at
> all. I was talking about this kind of example:
>
> > + git diff -U0 | sed -e "/^index/d" -e "s/$z2047/Z/g" >actual &&
> > + diff -u expect actual
>
> First line uses "git diff", if the second line uses "git diff" as well
> and "git diff" happens to be broken then you're using a broken tool to
> test a broken tool, as Junio already pointed out.
Hmm. There is some chicken-and-egg problem here (I read the thread, but
did not really see a problem, as I assumed that _other_ tests would assure
that "git diff --no-index" works as expected).
But as at least one released version of GNU diff has a pretty serious bug,
I would rather not rely too much on diff. (BTW this was the reason I
wanted --no-index so badly.)
So yeah, the second "diff" cannot be "git diff". Maybe "cmp", but not
"git diff".
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.
From: Pierre Habouzit @ 2007-12-17 11:59 UTC (permalink / raw)
To: Junio C Hamano, Jeff King, git
In-Reply-To: <20071217115648.GI7453@artemis.madism.org>
[-- Attachment #1: Type: text/plain, Size: 2896 bytes --]
On Mon, Dec 17, 2007 at 11:56:48AM +0000, Pierre Habouzit wrote:
> On Mon, Dec 17, 2007 at 11:13:14AM +0000, Junio C Hamano wrote:
> > Pierre Habouzit <madcoder@debian.org> writes:
> >
> > > Okay this is kind of disgusting, and I'm absolutely not pleased with
> > > it (I mean I'm not pleased that parse_opt forces us to write things like
> > > that). This hack allows:
> > >
> > > git tag -l -n10 <pattern>
> > >
> > > and will then attach the <pattern> to the '-l' switch,...
> >
> > Heh, it turns out that we were both stupid and blind.
>
> [...]
>
> indeed, but then this happens to be a better patch than yours IMHO:
>
>
> From 5a3cdd255f17c7d7bc9245881f0d50146413113f Mon Sep 17 00:00:00 2001
> From: Pierre Habouzit <madcoder@debian.org>
> Date: Mon, 17 Dec 2007 12:54:55 +0100
> Subject: [PATCH] git-tag: fix -l switch handling regression.
>
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
> builtin-tag.c | 12 +++++-------
> 1 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/builtin-tag.c b/builtin-tag.c
> index 274901a..219633d 100644
> --- a/builtin-tag.c
> +++ b/builtin-tag.c
> @@ -16,7 +16,7 @@
> static const char * const git_tag_usage[] = {
> "git-tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
> "git-tag -d <tagname>...",
> - "git-tag [-n [<num>]] -l [<pattern>]",
> + "git-tag -l [-n [<num>]] [<pattern>]",
> "git-tag -v <tagname>...",
> NULL
> };
> @@ -370,13 +370,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> struct ref_lock *lock;
>
> int annotate = 0, sign = 0, force = 0, lines = 0,
> - delete = 0, verify = 0;
> - char *list = NULL, *msgfile = NULL, *keyid = NULL;
> - const char *no_pattern = "NO_PATTERN";
> + list = 0, delete = 0, verify = 0;
> + char *msgfile = NULL, *keyid = NULL;
> struct msg_arg msg = { 0, STRBUF_INIT };
> struct option options[] = {
> - { OPTION_STRING, 'l', NULL, &list, "pattern", "list tag names",
> - PARSE_OPT_OPTARG, NULL, (intptr_t) no_pattern },
> + OPT_INTEGER('l', NULL, &list, "list tag names"),
^^^^^^^^^^^
that should obviously be OPT_BOOLEAN
> { OPTION_INTEGER, 'n', NULL, &lines, NULL,
> "print n lines of each tag message",
> PARSE_OPT_OPTARG, NULL, 1 },
> @@ -408,7 +406,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> annotate = 1;
>
> if (list)
> - return list_tags(list == no_pattern ? NULL : list, lines);
> + return list_tags(argv[0], lines);
> if (delete)
> return for_each_tag_name(argv, delete_tag);
> if (verify)
> --
> debian.1.5.3.7.1-dirty
>
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] Have a flag to stop the option parsing at the first argument.
From: Wincent Colaiuta @ 2007-12-17 12:06 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0712171149540.9446@racer.site>
El 17/12/2007, a las 12:50, Johannes Schindelin escribió:
> On Mon, 17 Dec 2007, Pierre Habouzit wrote:
>
>> On Mon, Dec 17, 2007 at 11:10:00AM +0000, Wincent Colaiuta wrote:
>>> El 17/12/2007, a las 10:50, Pierre Habouzit escribió:
>>>
>>>> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
>>>> ---
>>>>> // ...
>>>>> /* when in `git --opt1 --opt2 foo -a -b -c` mode: */
>>>>> int cmd_pos = git_find_builtin_command_name(argc, argv);
>>>>> int count = parse_options(cmd_pos, argv, git_generic_options,
>>>>> "git [special-options] cmd [options]", 0);
>>>>> if (count)
>>>>> die("unknown git command: `%s`", argv[0]);
>>>>> argv += cmd_pos;
>>>>> argc -= cmd_pos;
>>>>> /* here we simulate an argv of {"foo", "-a", "-b", "-c"} */
>>>>
>>>> Or even simpler, with the following specifically tailored patch
>>>> you
>>>> can directly write:
>>>>
>>>> argc = parse_options(argc, argv, git_generic_options,
>>>> "git [generic-options] <command> [cmd-options]",
>>>> PARSE_OPT_STOP_AT_ARG);
>>>>
>>>> and then {argc, argv} will exactly be the NULL-terminated array
>>>> starting with the builtin command. Kind of nice :)
>>>
>>> Indeed, nice ideas. I think all this will lead to a nice UI
>>> improvement
>>> post-1.5.4.
>>>
>>> About the only thing that I think would merit action *prior* to
>>> 1.5.4 is
>>> marking the "-p" switch to git (synonym for --paginate) as
>>> deprecated,
>>> see as it clashes with other commands' uses of that switch ("git
>>> log -p"
>>> for example). Are there any other conflicting specials that a
>>> currently
>>> parsed in git.c?
>>
>> You don't need to, and I'd see that as a regression. With my
>> proposal,
>> there isn't any kind of need that git commands do not clash with git
>> ones. The parse-option mechanism will properly hide options that are
>> masked this way, dscho wrote the patch for that.
>>
>> git -p log -p ...
>>
>> just makes sense to me. CVS or SVN e.g. (don't hit me !) have the
>> same
>> kind of "issues", and I never found that weird.
Yes, we know what it does because we know that "git ... log ..." is
actually two commands and each one handles one of the -p switches, but
it is much easier to present git as a single tool to the newcomer (and
I guess I don't need to argue that case here seeing as the decision
has already been taken long ago to talk using dashless forms), and it
is much easier to explain to a newcomer something like:
git log --paginate -p
Than:
git -p log -p
(Incidentally, who actually uses "git -p log -p"? Doesn't everybody
have a pager set-up by default?)
>> In fact I see this the other way around: git status -p that is in
>> fact
>> the same as git -p status, is the conveniency, git -p status is the
>> canonical form.
>
> I would even go further: "git status -p" looks utterly wrong to me.
This is exactly the kind of insider knowledge that will baffle
newcomers. The canonical form is canonical only because it relies on
an historical (and continuing) implementation detail, but we shouldn't
expect newcomers to have to learn such details in order to know where
to stick their command line arguments; they already have enough things
to learn about, I think.
But it doesn't really matter. The proposed changes allow old-timers to
continue putting their special options between the "git" and the
"command". If you don't want to deprecate the -p special because of
the confusion it might cause, I think we should at least not give it a
very prominent place in the documentation, nor use it any examples.
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] Re-re-re-fix common tail optimization
From: Wincent Colaiuta @ 2007-12-17 12:08 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, Junio C Hamano, Linus Torvalds, git
In-Reply-To: <Pine.LNX.4.64.0712171151490.9446@racer.site>
El 17/12/2007, a las 12:57, Johannes Schindelin escribió:
> Hmm. There is some chicken-and-egg problem here (I read the thread,
> but
> did not really see a problem, as I assumed that _other_ tests would
> assure
> that "git diff --no-index" works as expected).
>
> But as at least one released version of GNU diff has a pretty
> serious bug,
> I would rather not rely too much on diff. (BTW this was the reason I
> wanted --no-index so badly.)
>
> So yeah, the second "diff" cannot be "git diff". Maybe "cmp", but not
> "git diff".
Well cmp would be fine as well, seeing all we want is a boolean "is
this the same or not" answer. (I'm not familiar with the GNU diff bug
you speak of, but was it so bad that it couldn't even get *that*
answer right?)
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] Re-re-re-fix common tail optimization
From: Jeff King @ 2007-12-17 12:12 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Johannes Schindelin, Junio C Hamano, Linus Torvalds, git
In-Reply-To: <57245FA1-361B-4333-B490-A2CC99ED4F9C@wincent.com>
On Mon, Dec 17, 2007 at 01:08:45PM +0100, Wincent Colaiuta wrote:
>> So yeah, the second "diff" cannot be "git diff". Maybe "cmp", but not
>> "git diff".
>
> Well cmp would be fine as well, seeing all we want is a boolean "is this the
> same or not" answer. (I'm not familiar with the GNU diff bug you speak of,
> but was it so bad that it couldn't even get *that* answer right?)
Personally I find it useful to generate the diff output when viewing
with "-v". But if there is a real reason to use cmp over diff in the
script, one can always manually go into the trash directory and run
diff.
-Peff
^ permalink raw reply
* [PATCH] Document diff.external and mergetool.<tool>.path
From: Johannes Schindelin @ 2007-12-17 12:21 UTC (permalink / raw)
To: Schuberth, Sebastian; +Cc: git, gitster
In-Reply-To: <E6DFE65BB5ADFE44BE13CCC976124447D5BB8D@fue-email2.ad.mc.com>
There was no documentation for the config variables diff.external
and mergetool.<tool>.path.
Noticed by Sebastian Schuberth.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Documentation/config.txt | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ce16fc7..8a0df57 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -448,6 +448,13 @@ diff.autorefreshindex::
affects only `git diff` Porcelain, and not lower level
`diff` commands, such as `git diff-files`.
+diff.external::
+ If this config variable is set, diff generation is not
+ performed using the internal diff machinery, but using the
+ given command. Note: if you want to use an external diff
+ program only on a subset of your files, you might want to
+ use gitlink:gitattributes[5] instead.
+
diff.renameLimit::
The number of files to consider when performing the copy/rename
detection; equivalent to the git diff option '-l'.
@@ -671,6 +678,10 @@ merge.<driver>.recursive::
performing an internal merge between common ancestors.
See gitlink:gitattributes[5] for details.
+mergetool.<tool>.path::
+ Override the path for the given tool. This is useful in case
+ your tool is not in the PATH.
+
pack.window::
The size of the window used by gitlink:git-pack-objects[1] when no
window size is given on the command line. Defaults to 10.
--
1.5.4.rc0.36.g7680
^ permalink raw reply related
* Re: [PATCH] Re-re-re-fix common tail optimization
From: Johannes Sixt @ 2007-12-17 12:20 UTC (permalink / raw)
To: Wincent Colaiuta
Cc: Johannes Schindelin, Jeff King, Junio C Hamano, Linus Torvalds,
git
In-Reply-To: <57245FA1-361B-4333-B490-A2CC99ED4F9C@wincent.com>
Wincent Colaiuta schrieb:
> El 17/12/2007, a las 12:57, Johannes Schindelin escribió:
>
>> Hmm. There is some chicken-and-egg problem here (I read the thread, but
>> did not really see a problem, as I assumed that _other_ tests would
>> assure
>> that "git diff --no-index" works as expected).
>>
>> But as at least one released version of GNU diff has a pretty serious
>> bug,
>> I would rather not rely too much on diff. (BTW this was the reason I
>> wanted --no-index so badly.)
>>
>> So yeah, the second "diff" cannot be "git diff". Maybe "cmp", but not
>> "git diff".
>
> Well cmp would be fine as well, seeing all we want is a boolean "is this
> the same or not" answer. (I'm not familiar with the GNU diff bug you
> speak of, but was it so bad that it couldn't even get *that* answer right?)
Heh, there's at least one distribution out there (Suse 10.1) that comes with
a *cmp* that doesn't get that answer right if its output is connected to
/dev/null, which is the case when you simply 'make test'.
-- Hannes
^ permalink raw reply
* Re: [PATCH] Have a flag to stop the option parsing at the first argument.
From: Johannes Schindelin @ 2007-12-17 12:26 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <3CF3CEA5-72F1-47D1-ADB9-37F5C2E292A8@wincent.com>
Hi,
On Mon, 17 Dec 2007, Wincent Colaiuta wrote:
> Yes, we know what it does because we know that "git ... log ..." is
> actually two commands and each one handles one of the -p switches, but
> it is much easier to present git as a single tool to the newcomer (and I
> guess I don't need to argue that case here seeing as the decision has
> already been taken long ago to talk using dashless forms), and it is
> much easier to explain to a newcomer something like:
>
> git log --paginate -p
>
> Than:
>
> git -p log -p
How about
git log -p
Hmm?
Fact is: you make the tool easier by having sane defaults. Not by moving
around command line options. The option "-p" for git is an option that
holds for _all_ subcommands. That's why it belongs _before_ the
subcommand.
> But it doesn't really matter. The proposed changes allow old-timers to
> continue putting their special options between the "git" and the
> "command". If you don't want to deprecate the -p special because of the
> confusion it might cause, I think we should at least not give it a very
> prominent place in the documentation, nor use it any examples.
I think it is wrong to go out of our way to support "git status -p" as a
synonym to "git -p status". I simply do not believe that newcomers are
not intelligent enough to understand that "git -p <subcommand>" means that
the output goes into their pager.
Ciao,
Dscho
^ permalink raw reply
* Re: The Reposithon! Take 2 [URL FIX]
From: Sam Vilain @ 2007-12-17 12:00 UTC (permalink / raw)
To: Nicholas Clark; +Cc: Junio C Hamano, Perl 5 Porters, Git Mailing List
In-Reply-To: <20071217115450.GR23703@plum.flirble.org>
Nicholas Clark wrote:
> On Tue, Dec 18, 2007 at 12:48:26AM +1300, Sam Vilain wrote:
>> Nicholas Clark wrote:
>>> error: Could not interpret The page was, like, not found and stuff. as something to pull
>>>
>>>
>>> ^ random capitalisation or missing punctuation
>>> ^ slang interjection
>>> ^ slang interjection
>>> ^ missing capitalisation
>>>
>> I've never had my server 404 message
>>
>> http://utsl.gen.nz/err/404.html
>>
>> So well critiqued! Thanks! :-)
>>
>> Hopefully all the pull problems should be sorted now.
>
> I remain unhappy that git does not tell me that it's reporting a 404 error.
> 404 makes sense in any language. What the server admin chooses as the
> accompanying page may well not, although usually it would be because he or
> she chooses to make it report errors in the local language.
meh. look, actually tell you what my little web server is returning the
wrong error HTTP response code. Which is a pretty stupid
misconfiguration and not one you'd see very often. So the data gets a
little further in before it is noticed to be wrong.
The error message isn't the best, no.
Junio, any chance of this going in?
Subject: Clarify error response from 'git fetch' for bad responses
This error message prints the reponse from the server at this point.
Label it as such in the output.
---
walker.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/walker.c b/walker.c
index 397b80d..adc3e80 100644
--- a/walker.c
+++ b/walker.c
@@ -274,7 +274,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,
for (i = 0; i < targets; i++) {
if (interpret_target(walker, target[i], &sha1[20 * i])) {
- error("Could not interpret %s as something to pull", target[i]);
+ error("Could not interpret response from server '%s' as something to pull", target[i]);
goto unlock_and_fail;
}
if (process(walker, lookup_unknown_object(&sha1[20 * i])))
^ permalink raw reply related
* Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.
From: Pierre Habouzit @ 2007-12-17 12:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7v1w9la7o8.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1931 bytes --]
On Mon, Dec 17, 2007 at 11:21:11AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
>
> > On Mon, Dec 17, 2007 at 10:53:00AM +0000, Junio C Hamano wrote:
> > ...
> >> This is just a quick idea before I go back to sleep, but your earlier
> >> comment on "--no-<an-option-that-is-not-even-boolean>" made me realize
> >> that the alternative I was suggesting earlier would actually work much
> >> nicer, if you introduce "--<an-option-that-take-optional-arg>-default"
> >> magic.
> >
> > meeeow I love the idea !
>
> There is a bit more serious issue than coding, actually.
>
> Short options.
>
> A script wants to use default rename detection threshold for unknown
> commit $foo whose name might look like a number. IOW, this
>
> git diff -M $foo
>
> could be ambiguous. Obviously, "git diff -M-default $foo" would not fly
> very well.
Yes, I thought about that too actually.
After having written this mail 4 time already, I came up with an idea I
kind of like: like find, we could make {} be a placeholder for the
"default" argument. For example:
$ git foo --abbrev {} 10
$ git log -M {} 1
...
{} would have the same semantics as your --long-opt-default. It tells the
option parser that "no there isn't anything to grok for that command thank you
very much". Of course if for some reason you really want to pass "{}" to the
command, the stuck form holds:
$ git foo --long-opt={}
$ git foo -o{}
What do you think ?
PS: I know that in some shells {} needs escaping, which isn't nice. I chose it
because it's the same as find(1) but we could e.g. use '_' that is less
"conventional" (if it even makes sense) but is a bit easier to type than \{}.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: git-clone: Unobvious error messages when update-server-info has not been run
From: Jeff King @ 2007-12-17 12:43 UTC (permalink / raw)
To: Sebastian Harl; +Cc: Junio C Hamano, Gerrit Pape, git
In-Reply-To: <20071217105541.GG14889@albany.tokkee.org>
On Mon, Dec 17, 2007 at 11:55:41AM +0100, Sebastian Harl wrote:
> I was just trying to clone a repository using http but missed to run
> git-update-server-info on the server side. git-clone aborted with the
> following error messages:
>
> % git clone http://some/repo.git
> Initialized empty Git repository in /path/repo/.git/
> cat: /path/repo/.git/refs/remotes/origin/master: No such file or directory
> cd: 482: can't cd to /path/repo/.git/refs/remotes/origin
> fatal: : not a valid SHA1
> fatal: Not a valid object name HEAD
>
> It's kind of hard to guess where the error comes from in this case (I blamed
> Git at first). Is there some way to improve the error message in a case like
> this?
git-clone is supposed to detect this condition, but there was a bug in
the error checking code. Can you confirm that this patch fixes it?
Gerrit, I think was caused by your f28dd477 (it is a funny shell
interaction that the non-followed case branch resets $?, but it behaves
the same with bash and dash).
-- >8 --
clone: correctly report http_fetch errors
The exit status from curl was accidentally lost by the
'case' statement. We need to explicitly save it so that $?
doesn't get overwritten.
This improves the error message when fetching from an http
repository which has never had update-server-info run.
Previously, it would fail to note the fetch error and
produce multiple errors about the lack of origin branches.
It now correctly suggests running git-update-server-info.
Signed-off-by: Jeff King <peff@peff.net>
---
git-clone.sh | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/git-clone.sh b/git-clone.sh
index 68085a3..9a160ee 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -56,11 +56,12 @@ fi
http_fetch () {
# $1 = Remote, $2 = Local
- curl -nsfL $curl_extra_args "$1" >"$2" ||
- case $? in
- 126|127) exit ;;
- *) return $? ;;
- esac
+ curl -nsfL $curl_extra_args "$1" >"$2"
+ curl_exit_status=$?
+ case $curl_exit_status in
+ 126|127) exit ;;
+ *) return $curl_exit_status ;;
+ esac
}
clone_dumb_http () {
--
1.5.4.rc0.1145.gef733-dirty
^ permalink raw reply related
* Re: [PATCH] Have a flag to stop the option parsing at the first argument.
From: Wincent Colaiuta @ 2007-12-17 12:40 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0712171223210.9446@racer.site>
El 17/12/2007, a las 13:26, Johannes Schindelin escribió:
> Hi,
>
> On Mon, 17 Dec 2007, Wincent Colaiuta wrote:
>
>> Yes, we know what it does because we know that "git ... log ..." is
>> actually two commands and each one handles one of the -p switches,
>> but
>> it is much easier to present git as a single tool to the newcomer
>> (and I
>> guess I don't need to argue that case here seeing as the decision has
>> already been taken long ago to talk using dashless forms), and it is
>> much easier to explain to a newcomer something like:
>>
>> git log --paginate -p
>>
>> Than:
>>
>> git -p log -p
>
> How about
>
> git log -p
>
> Hmm?
>
> Fact is: you make the tool easier by having sane defaults. Not by
> moving
> around command line options. The option "-p" for git is an option
> that
> holds for _all_ subcommands. That's why it belongs _before_ the
> subcommand.
>
>> But it doesn't really matter. The proposed changes allow old-timers
>> to
>> continue putting their special options between the "git" and the
>> "command". If you don't want to deprecate the -p special because of
>> the
>> confusion it might cause, I think we should at least not give it a
>> very
>> prominent place in the documentation, nor use it any examples.
>
> I think it is wrong to go out of our way to support "git status -p"
> as a
> synonym to "git -p status". I simply do not believe that newcomers
> are
> not intelligent enough to understand that "git -p <subcommand>"
> means that
> the output goes into their pager.
But the point is, of all the special options, -p is the *only* that
can't unambiguously go after the subcommand.
I'm arguing that the world would be a simpler place, friendlier to
newcomers if *all* git commands looked like "git subcommand opts...",
and at the moment -p is the only obstacle to making this so. And just
in case it's necessary to restate this, I am not proposing removing
support for the git specials subcommand opts..." form; I'm just trying
to make it so that we don't have to advertise it so prominently. This
seems to be the natural complement to the move to dashless forms.
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] Re-re-re-fix common tail optimization
From: Johannes Schindelin @ 2007-12-17 12:51 UTC (permalink / raw)
To: Johannes Sixt
Cc: Wincent Colaiuta, Jeff King, Junio C Hamano, Linus Torvalds, git
In-Reply-To: <476669A7.1050407@viscovery.net>
Hi,
On Mon, 17 Dec 2007, Johannes Sixt wrote:
> Wincent Colaiuta schrieb:
> > El 17/12/2007, a las 12:57, Johannes Schindelin escribi?:
> >
> >> Hmm. There is some chicken-and-egg problem here (I read the thread, but
> >> did not really see a problem, as I assumed that _other_ tests would
> >> assure
> >> that "git diff --no-index" works as expected).
> >>
> >> But as at least one released version of GNU diff has a pretty serious
> >> bug,
> >> I would rather not rely too much on diff. (BTW this was the reason I
> >> wanted --no-index so badly.)
> >>
> >> So yeah, the second "diff" cannot be "git diff". Maybe "cmp", but not
> >> "git diff".
> >
> > Well cmp would be fine as well, seeing all we want is a boolean "is
> > this the same or not" answer. (I'm not familiar with the GNU diff bug
> > you speak of, but was it so bad that it couldn't even get *that*
> > answer right?)
>
> Heh, there's at least one distribution out there (Suse 10.1) that comes
> with a *cmp* that doesn't get that answer right if its output is
> connected to /dev/null, which is the case when you simply 'make test'.
Yeah. That's what it was. I even posted a patch to GNU diff, only to
find out that it was already fixed in CVS. Sigh.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Have a flag to stop the option parsing at the first argument.
From: Johannes Schindelin @ 2007-12-17 12:55 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <26962818-F702-44D2-BD26-95D74CE21F0D@wincent.com>
Hi,
On Mon, 17 Dec 2007, Wincent Colaiuta wrote:
> El 17/12/2007, a las 13:26, Johannes Schindelin escribi?:
>
> > I think it is wrong to go out of our way to support "git status -p" as
> > a synonym to "git -p status". I simply do not believe that newcomers
> > are not intelligent enough to understand that "git -p <subcommand>"
> > means that the output goes into their pager.
>
> But the point is, of all the special options, -p is the *only* that
> can't unambiguously go after the subcommand.
It should not be put after the subcommand. That's my point. Exactly
because it is -- even conceptually -- no subcommand option.
CVS has many shortcomings, but one lesson here is that people had no
problems with "cvs -z3 update -d -P". See, the "-z3" is an option that
has nothing to do with the subcommand.
Exactly the same situation here. I never had any problems explaining why
"-p" goes before the subcommand here. Never.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Have a flag to stop the option parsing at the first argument.
From: Wincent Colaiuta @ 2007-12-17 13:12 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0712171253290.9446@racer.site>
El 17/12/2007, a las 13:55, Johannes Schindelin escribió:
> On Mon, 17 Dec 2007, Wincent Colaiuta wrote:
>
>> El 17/12/2007, a las 13:26, Johannes Schindelin escribi?:
>>
>>> I think it is wrong to go out of our way to support "git status -
>>> p" as
>>> a synonym to "git -p status". I simply do not believe that
>>> newcomers
>>> are not intelligent enough to understand that "git -p <subcommand>"
>>> means that the output goes into their pager.
>>
>> But the point is, of all the special options, -p is the *only* that
>> can't unambiguously go after the subcommand.
>
> It should not be put after the subcommand. That's my point. Exactly
> because it is -- even conceptually -- no subcommand option.
>
> CVS has many shortcomings, but one lesson here is that people had no
> problems with "cvs -z3 update -d -P". See, the "-z3" is an option
> that
> has nothing to do with the subcommand.
>
> Exactly the same situation here. I never had any problems
> explaining why
> "-p" goes before the subcommand here. Never.
Would be even better if there was nothing to explain and it all just
worked, which is what I'm proposing here. As I said, -p is the only
special option which clashes with any non-special uses.
But leaving -p aside, will you oppose any patches that make it
possible for people to write stuff like:
git init --bare
Personally, I think this is an obvious usability improvement worth
striving for. Given that "git --bare init" will continue to work under
what I'm proposing, I really can't see any worthwhile argument against
it. Because we're talking about a UI improvement for newcomers at no
cost to old timers.
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] Have a flag to stop the option parsing at the first argument.
From: Johannes Schindelin @ 2007-12-17 13:30 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <223E3B44-92DA-4861-83D6-67E56F70E784@wincent.com>
Hi,
On Mon, 17 Dec 2007, Wincent Colaiuta wrote:
> El 17/12/2007, a las 13:55, Johannes Schindelin escribi?:
>
> > I never had any problems explaining why "-p" goes before the
> > subcommand here. Never.
>
> Would be even better if there was nothing to explain and it all just
> worked, which is what I'm proposing here. As I said, -p is the only
> special option which clashes with any non-special uses.
Even in the best of circumstances, you have to teach people a little.
> But leaving -p aside, will you oppose any patches that make it possible
> for people to write stuff like:
>
> git init --bare
>
> Personally, I think this is an obvious usability improvement worth
> striving for. Given that "git --bare init" will continue to work under
> what I'm proposing, I really can't see any worthwhile argument against
> it. Because we're talking about a UI improvement for newcomers at no
> cost to old timers.
My reasoning is like as for "-p". "--bare" is not special to "init". It
makes git not guess, but work on the current directory as a bare
repository.
In my experience, it is easier to give people a clear-cut distinction
between different concepts. Then way fewer surprises will hassle them
later.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] Actually add config variable diff.external
From: Johannes Schindelin @ 2007-12-17 13:42 UTC (permalink / raw)
To: Schuberth, Sebastian; +Cc: git, gitster
In-Reply-To: <Pine.LNX.4.64.0712171220540.9446@racer.site>
We had the diff.external variable in the documentation of the config
file since its conception, but failed to respect it.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
On Mon, 17 Dec 2007, Johannes Schindelin wrote:
> Subject: [PATCH] Document diff.external and mergetool.<tool>.path
>
> There was no documentation for the config variables
> diff.external and mergetool.<tool>.path.
Heh. Missed that diff.external was actually never implemented...
Also noticed by Sebastian.
diff.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/diff.c b/diff.c
index 7d17754..a49c36a 100644
--- a/diff.c
+++ b/diff.c
@@ -20,6 +20,7 @@
static int diff_detect_rename_default;
static int diff_rename_limit_default = 100;
static int diff_use_color_default;
+static const char *external_diff_cmd_cfg;
int diff_auto_refresh_index = 1;
static char diff_colors[][COLOR_MAXLEN] = {
@@ -163,6 +164,10 @@ int git_diff_ui_config(const char *var, const char *value)
diff_auto_refresh_index = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, "diff.external")) {
+ external_diff_cmd_cfg = xstrdup(value);
+ return 0;
+ }
if (!prefixcmp(var, "diff.")) {
const char *ep = strrchr(var, '.');
@@ -209,6 +214,8 @@ static const char *external_diff(void)
if (done_preparing)
return external_diff_cmd;
external_diff_cmd = getenv("GIT_EXTERNAL_DIFF");
+ if (!external_diff_cmd)
+ external_diff_cmd = external_diff_cmd_cfg;
done_preparing = 1;
return external_diff_cmd;
}
--
1.5.4.rc0.59.g1d10d
^ permalink raw reply related
* Re: [PATCH] Have a flag to stop the option parsing at the first argument.
From: Wincent Colaiuta @ 2007-12-17 13:57 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0712171328020.9446@racer.site>
El 17/12/2007, a las 14:30, Johannes Schindelin escribió:
>> But leaving -p aside, will you oppose any patches that make it
>> possible
>> for people to write stuff like:
>>
>> git init --bare
>>
>> Personally, I think this is an obvious usability improvement worth
>> striving for. Given that "git --bare init" will continue to work
>> under
>> what I'm proposing, I really can't see any worthwhile argument
>> against
>> it. Because we're talking about a UI improvement for newcomers at no
>> cost to old timers.
>
> My reasoning is like as for "-p". "--bare" is not special to
> "init". It
> makes git not guess, but work on the current directory as a bare
> repository.
Yes, I know what --bare does. It seems to me your argument is as
follows:
- "git subcommand" is actually "git" (which does setup) followed by
"git-subcommand" (which performs an action)[*]
- the newcomer should be taught this, and know which options go
between "git" and "subcommand" and which options go after
- even though there is a straightforward way to have "git subcommand
specials args" work without breaking compatibility, you oppose it on
the grounds that users should know how Git works
- the fact that CVS also has some options that go before the
subcommand makes it OK for git to do the same
- even though we could have a better UI that didn't require any of
this explanation at all, we should keep the UI we have seeing as it is
easy to explain
And possibly some combination of these two as well:
- requiring arguments to be inserted between "git" and "subcommand"
doesn't undermine the effort to make git appear like a single tool to
users
- the effort to move to dashless forms has nothing to do with
presenting git as a single tool to users
Well, I basically disagree with you on most of these points; all but
the first one(*), really. I think it's fairly clear what both you and
I think on this and I'm not very interested in debating it further; I
am more interested in hearing other people's points on this to get a
feeling for whether there is a consensus (or lack thereof).
Cheers,
Wincent
^ permalink raw reply
* add -i not able to find get_colorbool
From: Johannes Schindelin @ 2007-12-17 14:04 UTC (permalink / raw)
To: git
Hi,
I did not use "git add -i" in something like two weeks, and now it stopped
working:
$ git add -i
Can't locate object method "get_colorbool" via package "Git" at
/home/gitte/bin/git-add--interactive line 15.
The second line of git-add--interactive reads thusly:
use lib (split(/:/, $ENV{GITPERLLIB} ||
"/home/gitte/lib/perl5/site_perl/5.8.8"));
and this puzzles me:
$ grep -n get_colorbool /home/gitte/lib/perl5/site_perl/5.8.8/Git.pm
584:=item get_colorbool ( NAME )
591:sub get_colorbool {
So it has it, but cannot locate get_colorbool?
Help,
Dscho
^ permalink raw reply
* Re: add -i not able to find get_colorbool
From: Jeff King @ 2007-12-17 14:31 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0712171400250.9446@racer.site>
On Mon, Dec 17, 2007 at 02:04:21PM +0000, Johannes Schindelin wrote:
> The second line of git-add--interactive reads thusly:
>
> use lib (split(/:/, $ENV{GITPERLLIB} ||
> "/home/gitte/lib/perl5/site_perl/5.8.8"));
>
> and this puzzles me:
>
> $ grep -n get_colorbool /home/gitte/lib/perl5/site_perl/5.8.8/Git.pm
> 584:=item get_colorbool ( NAME )
> 591:sub get_colorbool {
>
> So it has it, but cannot locate get_colorbool?
That does seem odd. Can you try running git-add--interactive in the perl
debugger and doing this:
x \&Git::get_colorbool
x \&Git::repository
It should tell you where it is getting the functions from (in the first
case, it will presumably not find it at all, but you seem to have a
Git::repository, so it should point to the file that has been loaded).
-Peff
^ permalink raw reply
* Solaris install error: didn't get git-init link was [Fwd: Re: The Reposithon! Take 2]
From: Sam Vilain @ 2007-12-17 14:42 UTC (permalink / raw)
To: Git Mailing List
[-- Attachment #1: Type: text/plain, Size: 85 bytes --]
Refer attached ... 1.5.3.6 install, didn't get git-init link to git.
Ring any bells?
[-- Attachment #2: Re: The Reposithon! Take 2.eml --]
[-- Type: message/rfc822, Size: 3339 bytes --]
From: Andrew Dougherty <doughera@lafayette.edu>
To: "H.Merijn Brand" <h.m.brand@xs4all.nl>
Cc: perl5-porters@perl.org
Subject: Re: The Reposithon! Take 2
Date: Mon, 17 Dec 2007 09:32:39 -0500 (EST)
Message-ID: <Pine.LNX.4.64.0712170910420.22480@fractal.phys.lafayette.edu>
On Mon, 17 Dec 2007, H.Merijn Brand wrote:
> I don't say porting git to HP-UX was/is easy, but at least I got it building
> in less than four hours without having to depend on other projects. I gave
> up on SVN after three days and not having any runnable part of svn at all.
My experiences on Solaris are about equally painful, but reversed. I ended
up hand-editing svn's configure scripts to get them to work at all. It
took about a day of on-and-off effort, but it does now seem to work.
Git took about half a day to get to build and install, but alas it
doesn't seem to work:
$ git-clone git://git.utsl.gen.nz/p4-metadata
/home/doughera/git/bin/git-clone: git-init: command not found
Usage: /home/doughera/git/bin/git-clone [--template=<template_directory>] [--reference <reference-repo>] [--bare] [-l [-s]] [-q] [-u <upload-pack>] [--origin <name>] [--depth <n>] [-n] <repo> [<dir>]
$ git --version
git version 1.5.3.6
Indeed, I have no 'git-init' command anywhere to be found. I have no
idea if this is normal. It's probably something simple, but I probably
won't have time to investigate until January sometime.
Sigh.
On the other hand, I certainly agree that the discussion has been
constructive and positive. Sam's done a wonderful job working on
converting the perforce repository and sharing his work with the broader
community.
--
Andy Dougherty doughera@lafayette.edu
^ permalink raw reply
* Re: Solaris install error: didn't get git-init link was [Fwd: Re: The Reposithon! Take 2]
From: Jeff King @ 2007-12-17 15:13 UTC (permalink / raw)
To: Sam Vilain; +Cc: Git Mailing List
In-Reply-To: <47668ABB.9000504@vilain.net>
On Tue, Dec 18, 2007 at 03:42:03AM +1300, Sam Vilain wrote:
> Refer attached ... 1.5.3.6 install, didn't get git-init link to git.
> Ring any bells?
I just tried it and it worked OK. Note that the hardlinks are the _last_
part of the install procedure, even after doing the perl and git-gui
subdirectories. Are we sure the "make install" actually made it to
completion?
-Peff
^ permalink raw reply
* Re: [PATCH] Have a flag to stop the option parsing at the first argument.
From: Johannes Sixt @ 2007-12-17 15:13 UTC (permalink / raw)
To: Wincent Colaiuta
Cc: Johannes Schindelin, Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <223E3B44-92DA-4861-83D6-67E56F70E784@wincent.com>
Wincent Colaiuta schrieb:
> But leaving -p aside, will you oppose any patches that make it possible
> for people to write stuff like:
>
> git init --bare
>
> Personally, I think this is an obvious usability improvement worth
> striving for. Given that "git --bare init" will continue to work under
> what I'm proposing, I really can't see any worthwhile argument against
> it. Because we're talking about a UI improvement for newcomers at no
> cost to old timers.
Your point. I hate to have to think hard each time whether it's "git --bare
init" or "git init --bare" and "git clone --bare" or "git --bare clone" and
wouldn't mind if I no longer needed to.
-- Hannes
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox