Git development
 help / color / mirror / Atom feed
* 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: 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] 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: 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] 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: [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

* [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: 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

* 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] 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] 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] 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: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] 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] Have a flag to stop the option parsing at the first argument.
From: Pierre Habouzit @ 2007-12-17 11:47 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: Junio C Hamano, git
In-Reply-To: <30351C09-8BED-4D81-ABDD-2E079B4D54D2@wincent.com>

[-- Attachment #1: Type: text/plain, Size: 2205 bytes --]

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.
-- 
·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: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.
From: Junio C Hamano @ 2007-12-17 11:21 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Jeff King, git
In-Reply-To: <20071217105834.GG7453@artemis.madism.org>

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.

^ permalink raw reply

* Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.
From: Junio C Hamano @ 2007-12-17 11:13 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Jeff King, git
In-Reply-To: <20071217090749.GC7453@artemis.madism.org>

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.

Look at contrib/examples/git-tag.sh again.

The original addition of "-n <count>" was suboptimal and did not allow
"git tag -l -n10 <glob>", but I would actually call that a bug of the -n
<count> implementation (it wanted to affect working of other option, so
at that point it should have restructured the loop and made parsing of
options and carrying out of actions separate steps).

The -l option tells "git tag" to work in "list tags" mode, and in that
mode, the command itself takes zero or one (we could have taken more but
we didn't) glob to limit the listing.  The <glob> is not even an option
argument to option -l, but the arguments to "git tag" command itself.

So "git-tag -l" was a bad example of an option that takes optional
option-argument, and you can see that the conversion to parse_options()
done in 396865859918e9c7bf8ce74aae137c57da134610 (Make builtin-tag.c use
parse_options.) broke it.

IOW, the fixup I posted was not a workaround but happens to be a bugfix
to the above commit that did parse_options() conversion.

^ permalink raw reply

* Re: [PATCH] Have a flag to stop the option parsing at the first argument.
From: Wincent Colaiuta @ 2007-12-17 11:10 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Junio C Hamano, git
In-Reply-To: <20071217095014.GF7453@artemis.madism.org>

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?

Cheers,
Wincent

^ permalink raw reply

* Re: [StGit PATCH 00/17] Series short description
From: Catalin Marinas @ 2007-12-17 11:09 UTC (permalink / raw)
  To: David Kågedal; +Cc: git
In-Reply-To: <20071214105238.18066.23281.stgit@krank>

On 14/12/2007, David Kågedal <davidk@lysator.liu.se> wrote:
> The following series an emacs interface to stgit patch stacks. It
> shows a buffer with the the output of "stg series" and allows you to
> do some common operations on it, such as push/pop, commit/uncommit,
> edit, rename, repair, and coalesce.

That's really cool stuff! Thanks.

> The coalesce command obviosly requires the kha/experimental branch.
> The edit command requires the edit fixes in kha/safe.

I'll start this week merging patches from kha/experimental (I'm a bit
slow because of the Christmas activities).

-- 
Catalin

^ permalink raw reply

* Re: [msysGit] Re: Windows binaries for qgit 2.0
From: Abdelrazak Younes @ 2007-12-17 11:07 UTC (permalink / raw)
  To: git; +Cc: msysgit
In-Reply-To: <Pine.LNX.4.64.0712171042520.9446@racer.site>

Johannes Schindelin wrote:
> Hi,
> 
> On Mon, 17 Dec 2007, Abdelrazak Younes wrote:
> 
>> Marco Costalba wrote:
>>> On Dec 16, 2007 12:11 PM, Abdelrazak Younes <younes.a@free.fr> wrote:
>>>> Actually you might prefer to just use the LyX dependencies package that
>>>> we provide for Windows developers, it contains Qt. I paste here the
>>>> relevant part of our 'INSTALL.Win32':
>>>>
>>> Thanks, I've tried it but without success because I need MSVC 2005
>>> installed, and currently is not.
>> Right.
>>
>>> Anyhow for now I have produced a version with mingw that seems more or
>>> less to work, when I have a bit of time I will install MSVC 2005 and
>>> try if with that compiler is better...
>> I would like to help you with that but I can't retrieve the repository:
>>
>> $ git clone git://git.kernel.org/pub/scm/qgit/qgit4.git qgit4.git
>> Initialized empty Git repository in d:/devel/git/qgit4/qgit4.git/.git/
>> git.kernel.org[0: 130.239.17.7]: errno=Invalid argument
>> git.kernel.org[1: 199.6.1.166]: errno=Bad file descriptor
>> git.kernel.org[2: 204.152.191.8]: errno=Bad file descriptor
>> git.kernel.org[3: 204.152.191.40]: errno=Bad file descriptor
>> fatal: unable to connect a socket (Bad file descriptor)
>> fetch-pack from 'git://git.kernel.org/pub/scm/qgit/qgit4.git' failed.
>>
>> $ git clone http://git.kernel.org/pub/scm/qgit/qgit4.git qgit4.git
>> Initialized empty Git repository in d:/devel/git/qgit4/qgit4.git/.git/
>> Cannot get remote repository information.
>> Perhaps git-update-server-info needs to be run there?
>>
>> $ git --version
>> git version 1.5.3.6.1791.gfd264
>>
>> 'git clone ssh://...' seems to work but I guess I need a login and password?
>>
>> Maybe I am doing something wrong here? Sorry, my first time ever with git...
>>
>> Abdel.
>>
>> PS: Sorry for the cross posting but I guess this issue is maybe of
>> interest to msysgit people.
> 
> Why would anything that has to do with MSVC2005 be interesting to msysGit?  
> Note the "msys" part of "msysGit".

Sorry, I meant the error I am facing trying to clone the qgit 
repository. MSVC2005 has of course nothing to do with MSYS (while one 
could imagine that MSys and for the matter git could be compiled with it 
instead of mingw).

> 
> FWIW a member of our team works on compiling/including qgit into msysGit.  
> But definitely not using closed-source compilers.  I would violently 
> oppose that.

Noted. I didn't meant to start a debate here.

Abdel.

^ permalink raw reply

* git-stash: RFC: Adopt the default behavior to other commands
From: Sebastian Harl @ 2007-12-17 11:03 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 790 bytes --]

Hi,

By default, git-stash (when called without any other arguments) creates a new
stash. This is quite different to the behavior of most other Git commands
(e.g. git-tag, git-branch, etc. do "list" by default). In order to improve
consistency git-stash should imho adopt this as well.

The creation of a new stash should not do any harm. However, I think that
consistency is more important (iirc this has been mentioned in the current
survey a couple of times) and doing "list" is (in general) the best default.

Comments?

Cheers,
Sebastian

-- 
Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/

Those who would give up Essential Liberty to purchase a little Temporary
Safety, deserve neither Liberty nor Safety.         -- Benjamin Franklin


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH] Re-re-re-fix common tail optimization
From: Wincent Colaiuta @ 2007-12-17 10:59 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jeff King, Junio C Hamano, Linus Torvalds, git
In-Reply-To: <Pine.LNX.4.64.0712171038130.9446@racer.site>

El 17/12/2007, a las 11:39, Johannes Schindelin escribió:

> Hi,
>
> 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.

> 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. I presumed that if  
you had read the whole thread then that would be obvious (look at the  
quoted section from Junio above).

In the example you're not interested in the details of the output  
format, only in the exit status, so it is appropriate to use diff  
instead of "git diff".

Wincent

^ permalink raw reply

* Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.
From: Pierre Habouzit @ 2007-12-17 10:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vir2xa8z7.fsf@gitster.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 2213 bytes --]

On Mon, Dec 17, 2007 at 10:53:00AM +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).
> > ...
> > I'll try to think harder about what we can do about it. Though for now,
> > we will have to go for it for a while.
> 
> 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 !

> Then, normal users who know what the value of $foo is (iow, not scripts)
> can say:
> 
> 	git cmd --abbrev 10
>         git cmd --abbrev HEAD
>         git cmd --abbrev=10 HEAD
> 
> and scripts that want to have $foo to be treated as rev, even when it
> consists entirely of digits, can say:
> 
> 	git cmd --abbrev-default $foo
> 
> to disambiguate (i.e.  like "--no-" magic, "-default" is a magic, and it
> tells the parser that "there is no option-argument given to this").
> 
> To make sure $foo is treated as the precision, the script can say:
> 
> 	git cmd --abbrev=$foo
> 
> If the script wants DWIM just like human users want, it can do:
> 
> 	git cmd --abbrev $foo
> 
> There of course is a little details called coding, but I think this is
> probably the most user friendly to the users and the scripts alike.  It
> certainly is nicer than what the current parse_options() does, and/or
> the git-tag workaround does.

I like the idea, this way we can do the "let's pass the argument as an
option to the callback and let it say if it likes it or not, and have a
quite not so bad way to help the guy scripting this disambiguate. I like
it a lot, and it shouldn't be that hard to deal with. I'll work on it,
and propose new patches ASAP.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* git-clone: Unobvious error messages when update-server-info has not been run
From: Sebastian Harl @ 2007-12-17 10:55 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 891 bytes --]

Hi,

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?

TIA,
Sebastian

-- 
Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/

Those who would give up Essential Liberty to purchase a little Temporary
Safety, deserve neither Liberty nor Safety.         -- Benjamin Franklin


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.
From: Junio C Hamano @ 2007-12-17 10:53 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Jeff King, git
In-Reply-To: <20071217090749.GC7453@artemis.madism.org>

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).
> ...
> I'll try to think harder about what we can do about it. Though for now,
> we will have to go for it for a while.

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.

Then, normal users who know what the value of $foo is (iow, not scripts)
can say:

	git cmd --abbrev 10
        git cmd --abbrev HEAD
        git cmd --abbrev=10 HEAD

and scripts that want to have $foo to be treated as rev, even when it
consists entirely of digits, can say:

	git cmd --abbrev-default $foo

to disambiguate (i.e.  like "--no-" magic, "-default" is a magic, and it
tells the parser that "there is no option-argument given to this").

To make sure $foo is treated as the precision, the script can say:

	git cmd --abbrev=$foo

If the script wants DWIM just like human users want, it can do:

	git cmd --abbrev $foo

There of course is a little details called coding, but I think this is
probably the most user friendly to the users and the scripts alike.  It
certainly is nicer than what the current parse_options() does, and/or
the git-tag workaround does.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox