* Re: git.c option parsing
From: Junio C Hamano @ 2007-12-17 8:48 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: git
In-Reply-To: <F6F3247E-4E71-4977-9626-F0571278E1E6@wincent.com>
Wincent Colaiuta <win@wincent.com> writes:
> Of course, the above plan will only work for builtins, not for
> scripts. An additional step would be needed to enable scripts to
> handle these options; perhaps teaching "git rev-parse" something...
As long as special options stay special and we make a rule not to allow
any subcommand to assign its own meaning to them, the git wrapper can
lookahead and reorder, when seeing a command line:
git scripted-command --special
into
git --special scripted-command
And that approach would work well for built-ins as well, I would
imagine.
There is one minor detail, though. There could be an option-parameter
that is literally --special. E.g.
git grep -e --no-paginate
should not be reordered to
git --no-paginate grep -e
^ permalink raw reply
* Re: [PATCH] (squashme) gitcli documentation fixups
From: Pierre Habouzit @ 2007-12-17 8:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7v7ijdeq4w.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1185 bytes --]
On Mon, Dec 17, 2007 at 07:28:47AM +0000, Junio C Hamano wrote:
> This comes directly on top of gitcli documentation patch and is intended
> to be squashed into it.
I obviously ack.
> -Another things to keep in mind is that long options can be negated. For
> +Boolean options with long option names can be negated by prefixing `"--no-"`. For
^^^^^^^
Though this isn't correct: you can negate any kind of option, even one
with strings arguments, and it does makes sense. E.g. if you have some:
foo.stringOpt = "value"
in your gitconfig file, then it's very handy to be able to write:
$ git foo --no-string-opt
to be sure the gitconfig from the user won't mess with what you intend
to do. The negation of commands can be disabled (in the recent
iterations of parseopt) using a flag I don't recall the name, but it's
on by default even for non boolean options. It may make sense to do a
re-read pass of all options and see which ones it makes sense to negate
and which not.
--
·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] (squashme) gitcli documentation fixups
From: Pierre Habouzit @ 2007-12-17 8:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7v7ijdeq4w.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 796 bytes --]
On Mon, Dec 17, 2007 at 07:28:47AM +0000, Junio C Hamano wrote:
> * avoid reinventing the wheel.
>
> but it needs a bit more explanation. Quite a few people seem to try to
> reinvent "git rev-parse --verify HEAD" in their scripts using much
> higher level "git show -s -1 --pretty=format:xxx", which is unfortunate
> and disgusting at the same time.
Oh and about that, the point is, users don't always know the wheel
exists because they don't know where to look in the first place. Maybe
gitcli(5) will become the right place to explain this kind of usual
tricks under a "git scripting idioms" section.
--
·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.c option parsing
From: Wincent Colaiuta @ 2007-12-17 9:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsl21aeqw.fsf@gitster.siamese.dyndns.org>
El 17/12/2007, a las 9:48, Junio C Hamano escribió:
> Wincent Colaiuta <win@wincent.com> writes:
>
>> Of course, the above plan will only work for builtins, not for
>> scripts. An additional step would be needed to enable scripts to
>> handle these options; perhaps teaching "git rev-parse" something...
>
> As long as special options stay special and we make a rule not to
> allow
> any subcommand to assign its own meaning to them, the git wrapper can
> lookahead and reorder, when seeing a command line:
>
> git scripted-command --special
>
> into
>
> git --special scripted-command
>
> And that approach would work well for built-ins as well, I would
> imagine.
Yes, and it would be simpler to implement also. The only downside is
that without all the other proposed changes things like "git-dashed --
special" wouldn't work; only teaching the builtins to actually handle
the special options would work in that case. And in the interests of
consistency I think it's pretty important that "git subcommand --
special" and "git-subcommand --special" both work the same as the user
would (reasonably) expect them to...
> There is one minor detail, though. There could be an option-parameter
> that is literally --special. E.g.
>
> git grep -e --no-paginate
>
> should not be reordered to
>
> git --no-paginate grep -e
Yes, that's definitely one that would need to be treated as a special
case.
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] builtin-tag: fix fallouts from recent parsopt restriction.
From: Pierre Habouzit @ 2007-12-17 9:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vd4t5eq52.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]
On Mon, Dec 17, 2007 at 07:28:41AM +0000, Junio C Hamano wrote:
> The command freely used optional option-argments for its -l and -n options.
> I think allowing "git tag -n xxx" without barfing was an error to begin with,
> but not supporting "git tag -l pattern" form is a serious regression.
>
> So this fixes the handling of -l to reinstate the original behaviour while
> detecting a user error "git tag -l pattern garbage", and adjusts tests that
> use "-n param" form to use "-nparam".
> - if (list)
> + if (list) {
> + /*
> + * This is unfortunate but requiring "git tag -lpattern" and not
> + * allowing "git tag -l pattern" is a serious regression.
> + */
> + if (argc && list == no_pattern) {
> + list = argv[0];
> + argc--;
> + }
> + if (argc)
> + die("extra argument after -l[pattern]: %s", argv[0]);
> return list_tags(list == no_pattern ? NULL : list, lines);
> + }
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, and I find it
nowhere near acceptable. I believe the fix is worse than the disease.
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.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH] Clean up documentation that references deprecated 'git peek-remote'.
From: Johannes Sixt @ 2007-12-17 9:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Sixt
Now that 'git peek-remote' is deprecated and only an alias for
'git ls-remote', it should not be referenced from other manual pages.
This also removes the description of the --exec option, which is no
longer present.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
Documentation/git-daemon.txt | 6 +++---
Documentation/git-ls-remote.txt | 2 +-
Documentation/git-peek-remote.txt | 3 ---
Documentation/git-show-ref.txt | 2 +-
4 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 99e47c9..f1e48dd 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -31,8 +31,8 @@ pass some directory paths as 'git-daemon' arguments, you can further restrict
the offers to a whitelist comprising of those.
By default, only `upload-pack` service is enabled, which serves
-`git-fetch-pack` and `git-peek-remote` clients that are invoked
-from `git-fetch`, `git-ls-remote`, and `git-clone`.
+`git-fetch-pack` and `git-ls-remote` clients, which are invoked
+from `git-fetch`, `git-pull`, and `git-clone`.
This is ideally suited for read-only updates, i.e., pulling from
git repositories.
@@ -166,7 +166,7 @@ the per-repository configuration file can be used to enable or
disable them.
upload-pack::
- This serves `git-fetch-pack` and `git-peek-remote`
+ This serves `git-fetch-pack` and `git-ls-remote`
clients. It is enabled by default, but a repository can
disable it by setting `daemon.uploadpack` configuration
item to `false`.
diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 93e9a60..445beda 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -30,7 +30,7 @@ OPTIONS
Specify the full path of gitlink:git-upload-pack[1] on the remote
host. This allows listing references from repositories accessed via
SSH and where the SSH daemon does not use the PATH configured by the
- user. Also see the '--exec' option for gitlink:git-peek-remote[1].
+ user.
<repository>::
Location of the repository. The shorthand defined in
diff --git a/Documentation/git-peek-remote.txt b/Documentation/git-peek-remote.txt
index 38a5325..09b005b 100644
--- a/Documentation/git-peek-remote.txt
+++ b/Documentation/git-peek-remote.txt
@@ -28,9 +28,6 @@ OPTIONS
shells, but prefer having a lean .bashrc file (they set most of
the things up in .bash_profile).
-\--exec=<git-upload-pack>::
- Same \--upload-pack=<git-upload-pack>.
-
<host>::
A remote host that houses the repository. When this
part is specified, 'git-upload-pack' is invoked via
diff --git a/Documentation/git-show-ref.txt b/Documentation/git-show-ref.txt
index 2355aa5..7893a50 100644
--- a/Documentation/git-show-ref.txt
+++ b/Documentation/git-show-ref.txt
@@ -160,7 +160,7 @@ to get a listing of all tags together with what they dereference.
SEE ALSO
--------
-gitlink:git-ls-remote[1], gitlink:git-peek-remote[1]
+gitlink:git-ls-remote[1]
AUTHORS
-------
--
1.5.4.rc0.73.gcaffa
^ permalink raw reply related
* Re: git.c option parsing
From: Pierre Habouzit @ 2007-12-17 9:20 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Junio C Hamano, git
In-Reply-To: <25018761-80E8-410C-BB18-DD799F58308A@wincent.com>
[-- Attachment #1: Type: text/plain, Size: 2703 bytes --]
On Mon, Dec 17, 2007 at 09:01:56AM +0000, Wincent Colaiuta wrote:
> El 17/12/2007, a las 9:48, Junio C Hamano escribió:
>
> >Wincent Colaiuta <win@wincent.com> writes:
> >
> >>Of course, the above plan will only work for builtins, not for
> >>scripts. An additional step would be needed to enable scripts to
> >>handle these options; perhaps teaching "git rev-parse" something...
> >
> >As long as special options stay special and we make a rule not to allow
> >any subcommand to assign its own meaning to them, the git wrapper can
> >lookahead and reorder, when seeing a command line:
> >
> > git scripted-command --special
> >
> >into
> >
> > git --special scripted-command
> >
> >And that approach would work well for built-ins as well, I would
> >imagine.
>
> Yes, and it would be simpler to implement also. The only downside is that
> without all the other proposed changes things like "git-dashed --special"
> wouldn't work; only teaching the builtins to actually handle the special
> options would work in that case. And in the interests of consistency I
> think it's pretty important that "git subcommand --special" and
> "git-subcommand --special" both work the same as the user would
> (reasonably) expect them to...
There is a simple way to do that, that wouldn't conflict with the git
grep -e one, that would be to define an array of "Special" flags, in a
macro, and have every builtin using parseopt adding that macro at the
end.
Then in git.c, you would have to scan for the command name when called
through `git --opt1 --opt2 foo`, and pass it to parseopt with as argc
the position of `foo` in the argument array. Parseopt will trust you for
it, and if it returns a non zero count, you have to barf, then you just
need to work on the rest of the array. That would mean in pseudo code
that this would work like:
// ...
/* 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"} */
Of course this only works on builtins that do support parseopt other
ones will need the massage you describe. For them the sole thing to
change would be to replace the OPT_END() with a GIT_SPECIAL_OPTS() or
sth alike.
--
·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.c option parsing
From: Pierre Habouzit @ 2007-12-17 9:26 UTC (permalink / raw)
To: Wincent Colaiuta, Junio C Hamano, git
In-Reply-To: <20071217092013.GD7453@artemis.madism.org>
[-- Attachment #1: Type: text/plain, Size: 1946 bytes --]
On Mon, Dec 17, 2007 at 09:20:13AM +0000, Pierre Habouzit wrote:
> There is a simple way to do that, that wouldn't conflict with the git
> grep -e one, that would be to define an array of "Special" flags, in a
> macro, and have every builtin using parseopt adding that macro at the
> end.
>
> Then in git.c, you would have to scan for the command name when called
> through `git --opt1 --opt2 foo`, and pass it to parseopt with as argc
> the position of `foo` in the argument array. Parseopt will trust you for
> it, and if it returns a non zero count, you have to barf, then you just
> need to work on the rest of the array. That would mean in pseudo code
> that this would work like:
>
> // ...
> /* 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"} */
>
> Of course this only works on builtins that do support parseopt other
> ones will need the massage you describe. For them the sole thing to
> change would be to replace the OPT_END() with a GIT_SPECIAL_OPTS() or
> sth alike.
Oh and I forgot to say that I find this is a brilliant idea, because
there is quite a _lot_ of options that are very pervasive: --quiet,
--verbose, … are pretty obvious candidates, but things like:
`--abbrev=12` does make a lot of sense to me, and it would factor quite
a lot of option structures :) Okay `git --abbrev=12 status` probably
makes no real sense, but who cares ?
--
·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: [RFH] convert shortlog to use parse_options
From: Andreas Ericsson @ 2007-12-17 9:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pierre Habouzit, Jeff King, Kristian Høgsberg, git
In-Reply-To: <7v3au1eopr.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Andreas Ericsson <ae@op5.se> writes:
>
>> Junio C Hamano wrote:
>>
>>> ... The
>>> "must stick" restriction feels Ok on paper but in practice it looks
>>> rather draconian and very user unfriendly.
>> Usually, optional arguments warrant adding a second parameter. This can
>> often even improve usability, as it's never unclear or ambiguous what's
>> happening. For the 'git tag -l' case, I'd use something like
>> 'git tag -l --match="regex"' or some such,...
>
> That is essentially arguing for POSIXly correct "do not allow optional
> option-arguments" (utility syntax guidelines #7). That position might
> be politically correct, but I am already discussing beyond that:
> usability.
>
> For "git tag -l", the fix was rather simple, as the option would either
> have taken a zero pattern (list all) or a single pattern (list matching
> this pattern), and the command itself did not take any extra arguments,
> so that was what I did in the patch. Compare your POSIXly correct
> version:
>
> git tag -l (ok)
> git tag -l pattern (not ok)
> git tag -l --match=pattern (ok)
>
> with the traditional (and fixed):
>
> git tag -l (ok)
> git tag -l pattern (ok)
> git tag -l pattern garbage (not ok)
>
> Which one is easier for the user?
git tag -l pattern, although I suspect newcomers often write it as
"git tag -l | grep pattern" anyways.
If -l was a short-hand for "git tag list", and "list" was a subcommand
to git tag, the whole business would be explainable. The fact that
"git tag" lists all tag but doesn't take a patter, while "git tag -l"
does exactly the same thing, but *does* take an (optional) pattern
means the --match functionality could just as well be written as
"git tag -m pattern" (and let '-m' imply '-l', which is sensible).
"-l" can then be deprecated, as its syntax doesn't match that of
the other way to list tags.
Otoh, I don't care all that deeply about it. It's just nicer to explain
the UI if there are no optional arguments, primarily because it involves
exceptions, and secondarily because many programs follow the posixly
correct thing of "no optional arguments", so they're a definitive
minority in the program argument jungle.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* [PATCH] Have a flag to stop the option parsing at the first argument.
From: Pierre Habouzit @ 2007-12-17 9:50 UTC (permalink / raw)
To: Wincent Colaiuta, Junio C Hamano, git
In-Reply-To: <20071217092013.GD7453@artemis.madism.org>
[-- Attachment #1: Type: text/plain, Size: 1737 bytes --]
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 :)
parse-options.c | 2 ++
parse-options.h | 1 +
2 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 7a08a0c..4f5c55e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -229,6 +229,8 @@ int parse_options(int argc, const char **argv, const struct option *options,
const char *arg = args.argv[0];
if (*arg != '-' || !arg[1]) {
+ if (flags & PARSE_OPT_STOP_AT_ARG)
+ break;
argv[j++] = args.argv[0];
continue;
}
diff --git a/parse-options.h b/parse-options.h
index 102ac31..7c636b9 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -18,6 +18,7 @@ enum parse_opt_type {
enum parse_opt_flags {
PARSE_OPT_KEEP_DASHDASH = 1,
+ PARSE_OPT_STOP_AT_ARG = 2,
};
enum parse_opt_option_flags {
--
1.5.4.rc0.1147.gfd22d
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply related
* Re: Windows binaries for qgit 2.0
From: Abdelrazak Younes @ 2007-12-17 9:51 UTC (permalink / raw)
To: msysgit-/JYPxA39Uh5TLH3MbocFFw; +Cc: git-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <e5bfff550712161426y101c77efl4f5321d3440fed3f-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Marco Costalba wrote:
> On Dec 16, 2007 12:11 PM, Abdelrazak Younes <younes.a-GANU6spQydw@public.gmane.org> 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.
^ permalink raw reply
* Re: [PATCH] Have a flag to stop the option parsing at the first argument.
From: Johannes Schindelin @ 2007-12-17 10:34 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Wincent Colaiuta, Junio C Hamano, git
In-Reply-To: <20071217095014.GF7453@artemis.madism.org>
Hi,
On Mon, 17 Dec 2007, Pierre Habouzit wrote:
> diff --git a/parse-options.c b/parse-options.c
> index 7a08a0c..4f5c55e 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -229,6 +229,8 @@ int parse_options(int argc, const char **argv, const struct option *options,
> const char *arg = args.argv[0];
>
> if (*arg != '-' || !arg[1]) {
> + if (flags & PARSE_OPT_STOP_AT_ARG)
> + break;
> argv[j++] = args.argv[0];
> continue;
> }
> diff --git a/parse-options.h b/parse-options.h
> index 102ac31..7c636b9 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -18,6 +18,7 @@ enum parse_opt_type {
>
> enum parse_opt_flags {
> PARSE_OPT_KEEP_DASHDASH = 1,
> + PARSE_OPT_STOP_AT_ARG = 2,
> };
>
> enum parse_opt_option_flags {
Funny. I already posted this:
http://repo.or.cz/w/git/dscho.git?a=commitdiff;h=504f763a28b3109fce258b36f9e94e7c54be6f3d
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Re-re-re-fix common tail optimization
From: Johannes Schindelin @ 2007-12-17 10:39 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Jeff King, Junio C Hamano, Linus Torvalds, git
In-Reply-To: <EBD73F46-810F-4605-972C-54EED0EF9A63@wincent.com>
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".
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.
Ciao,
Dscho
^ permalink raw reply
* Re: [msysGit] Re: Windows binaries for qgit 2.0
From: Johannes Schindelin @ 2007-12-17 10:44 UTC (permalink / raw)
To: Abdelrazak Younes; +Cc: msysgit, git
In-Reply-To: <fk5grp$7il$2@ger.gmane.org>
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".
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.
Ciao,
Dscho
^ 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
* 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: 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
* 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
* 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: [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
* 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: [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: [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] 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] 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
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