Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Add a simple option parser.
From: Pierre Habouzit @ 2007-10-14  7:02 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git, Junio C Hamano
In-Reply-To: <20071013221450.GC2875@steel.home>

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

On Sat, Oct 13, 2007 at 10:14:50PM +0000, Alex Riesen wrote:
> Pierre Habouzit, Sat, Oct 13, 2007 22:54:04 +0200:
> > On Sat, Oct 13, 2007 at 07:16:55PM +0000, Alex Riesen wrote:
> > > Pierre Habouzit, Sat, Oct 13, 2007 15:29:03 +0200:
> > > BTW, if you just printed the usage message out (it is about usage of a
> > > program, isn't it?) and called exit() everyone would be just as happy.
> > > And you wouldn't have to include strbuf (it is the only use of it),
> > > less code, too. It'd make simplier to stea^Wcopy your implementation,
> > > which I like :)
> > 
> >   the reason is that usage() is a wrapper around a callback, and I
> > suppose it's used by some GUI's or anything like that.
> 
> It is not. Not yet. What could they use a usage text for?
> Besides, you could just export the callback (call_usage_callback or
> something) from usage.c and call it.

  Okay makes sense.

> >   FWIW you can rework the .c like this:
> 
> on top of yours:

  Added (reworked a bit for the current state of parse_options), and pushed.

-- 
·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 User's Survey 2007 unfinished summary continued
From: Linus Torvalds @ 2007-10-14  3:55 UTC (permalink / raw)
  To: david
  Cc: Shawn O. Pearce, Johannes Schindelin, J. Bruce Fields,
	Jakub Narebski, git
In-Reply-To: <Pine.LNX.4.64.0710132037290.30704@asgard.lang.hm>



On Sat, 13 Oct 2007, david@lang.hm wrote:
>
> I'll also point out that being a 'productive member of society' may have a
> wider definition then you may think initially.

I actually meant it in the absolutely most narrow possible meaning: you 
take the least productive person imaginable, who is certainly not going to 
do anything at all, and in the end, who cares? It's not like nonproductive 
people really hurt.

Some people in the open source / free software world get really upset 
about "freeloaders". I think that's silly. First off, I agree with you 
that a lot of people don't even end up being freeloaders - even if you 
never code a single line of code, there are ton of ways to be usefully 
involved (and some of them will be entirely invisible to any developer - 
helping random people outside the development lists, for example).

But more importantly, even somebody who really isn't productive at all 
generally can't be messing things up either - so it's a nonissue. Unless 
it results in tons of flaming ...

		Linus

^ permalink raw reply

* Re: Git User's Survey 2007 unfinished summary continued
From: david @ 2007-10-14  3:43 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Shawn O. Pearce, Johannes Schindelin, J. Bruce Fields,
	Jakub Narebski, git
In-Reply-To: <alpine.LFD.0.999.0710132011270.6887@woody.linux-foundation.org>

On Sat, 13 Oct 2007, Linus Torvalds wrote:

> But at the same time, just accepting that there are people who will
> potentially never really be productive members of society (whether
> "society" is git or something bigger), is probably a good idea. They
> aren't worth complaining about: they don't generally tend to take anything
> away from the community unless the community itself reacts negatively to
> them.

I'll also point out that being a 'productive member of society' may have a 
wider definition then you may think initially.

is a sysadmin who never contribures a line of code, but switches hundreds 
of servers to linux and assists friends in migrating to Linux a productive 
member? he doesn't contribute any code, so some people would say no, but 
in spreading the use he is increasing the number of potential contributers 
so others would say yes.

keep this in mind before you assume that someone isn't worth anything.

David Lang

^ permalink raw reply

* Re: Git User's Survey 2007 unfinished summary continued
From: Linus Torvalds @ 2007-10-14  3:15 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Johannes Schindelin, J. Bruce Fields, Jakub Narebski, git
In-Reply-To: <20071014014445.GN27899@spearce.org>



On Sat, 13 Oct 2007, Shawn O. Pearce wrote:
> 
> But just saying "MY GOD FIX THE UI" is not a wishlist item (yes,
> that was a real survey answer).  It provides the community no
> chance to understand what parts of the UI we need to work on, and
> what parts the end-user is OK with or just hasn't even tried to use.

Heh. I do agree that some people just ask for unreasonable or stupid 
things (or maybe they are just really bad at explaining them, and may have 
something non-stupid in mind but just cannot articulate it).

And I also agree that there are tons of people who are just lazy and don't 
even bother to try to explain themselves.

And I'll flame people myself. I can't even say that's a rare event. So I 
shouldn't throw _too_ many stones, or one of them might bounce back. 

But at the same time, just accepting that there are people who will 
potentially never really be productive members of society (whether 
"society" is git or something bigger), is probably a good idea. They 
aren't worth complaining about: they don't generally tend to take anything 
away from the community unless the community itself reacts negatively to 
them.

			Linus

^ permalink raw reply

* Re: [PATCH 14/14] Use the asyncronous function infrastructure to run the content filter.
From: Johannes Schindelin @ 2007-10-14  3:07 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: gitster, git
In-Reply-To: <1192305984-22594-15-git-send-email-johannes.sixt@telecom.at>

Hi,

On Sat, 13 Oct 2007, Johannes Sixt wrote:

>  	status = finish_command(&child_process);
>  	if (status)
> -		error("external filter %s failed %d", cmd, -status);
> +		error("external filter %s failed", params->cmd);

Did you mean to remove the status from the output (it should probably read 
"(exit status %d)" instead of just "%d", but an exit status can help 
identify problems, right?


> -	child_process.pid = fork();
> -	if (child_process.pid < 0) {
> -		error("cannot fork to run external filter %s", cmd);
> -		close(pipe_feed[0]);
> -		close(pipe_feed[1]);
> -		return NULL;
> -	}
> -	if (!child_process.pid) {
> -		close(pipe_feed[0]);
> -		exit(filter_buffer(pipe_feed[1], src, *sizep, cmd));
> -	}
> -	close(pipe_feed[1]);
> +	if (start_async(&async))
> +		return 0;	/* error was already reported */

Please write "return NULL;"

Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH 0/14] fork/exec removal series
From: Shawn O. Pearce @ 2007-10-14  2:58 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Johannes Sixt, gitster, git
In-Reply-To: <Pine.LNX.4.64.0710140348550.25221@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Sat, 13 Oct 2007, Shawn O. Pearce wrote:
> 
> > Since builtin-pack-objects now accepts (limited) pthread support, 
> > perhaps this should be implemented in terms of pthread support when 
> > pthreads are available?
> 
> Falling back to fork() when no pthreads are available?  Yes, that makes 
> sense.
> 
> It might also (marginally) speed up operations, since the switches between 
> threads are cheaper than those between processes, right?

Usually.  If we have a large virtual address space (say due to
opening a bunch of packfiles and reading commits out of them into
struct commit* thingies) and the OS does a giant copy of the page
tables during fork() then the pthread creation should be a heck of
a lot cheaper.

But we most definately *must* continue to support fork() for the
async functions.  Its the most common interface available on one
of our biggest platforms (UNIX).

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 0/14] fork/exec removal series
From: Johannes Schindelin @ 2007-10-14  2:50 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Johannes Sixt, gitster, git
In-Reply-To: <20071014021149.GO27899@spearce.org>

Hi,

On Sat, 13 Oct 2007, Shawn O. Pearce wrote:

> Since builtin-pack-objects now accepts (limited) pthread support, 
> perhaps this should be implemented in terms of pthread support when 
> pthreads are available?

Falling back to fork() when no pthreads are available?  Yes, that makes 
sense.

It might also (marginally) speed up operations, since the switches between 
threads are cheaper than those between processes, right?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 0/14] fork/exec removal series
From: Shawn O. Pearce @ 2007-10-14  2:11 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: gitster, git
In-Reply-To: <1192305984-22594-1-git-send-email-johannes.sixt@telecom.at>

Johannes Sixt <johannes.sixt@telecom.at> wrote:
> here is a series of patches that removes a number fork/exec pairs.
...
> The series consists of 2 parts:
> 
> - The first half replaces a number of fork/exec pairs by start_command/
>   finish_command or run_command.
> 
> - The second half introduces a new framework that runs a function
>   asynchronously. New functions start_async and finish_async are implemented
>   similarly to start_command and run_command. They are used to replace
>   occurrences of fork() that does not exec() in the child. Such code
>   could in principle be run in a thread, and on MinGW port we will go this
>   route, but on Posix we stay with fork().

This series looks pretty good to me.  I like seeing huge blocks
go away only to be replaced with the simple API offered by
run-command.h.  Makes the result much easier to follow.

The async interface is also quite simple.  Unfortunately there
is some risk with the canonical fork() implementation in that the
async routine might attempt to alter global data that the parent
is also using, and folks on a good UNIX that is using the fork()
implementation will not even notice as they are in totally separated
address spaces.  But you'll see it in MSYS Git.

Since builtin-pack-objects now accepts (limited) pthread support,
perhaps this should be implemented in terms of pthread support
when pthreads are available?  Most Linux/BSD/Mac OS X systems do
have pthreads these days and that's the majority of git users and
developers.  This would make it more likely that bugs in this sort
of code would be detected early.  Just a thought.
 
>  13 files changed, 334 insertions(+), 369 deletions(-)

Hard to argue with that final state.  You killed 35 lines and
also made Git easier to port to "that OS unfortunately named after
transparent glass thingies".

-- 
Shawn.

^ permalink raw reply

* Re: Git User's Survey 2007 unfinished summary continued
From: Johannes Schindelin @ 2007-10-14  2:06 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: J. Bruce Fields, Jakub Narebski, git
In-Reply-To: <alpine.LFD.0.999.0710131810550.6887@woody.linux-foundation.org>

Hi,

On Sat, 13 Oct 2007, Linus Torvalds wrote:

> On Sun, 14 Oct 2007, Johannes Schindelin wrote:
> >
> > My main point is -- and always was -- that I'd like people to realise 
> > how much it depends on _them_ if (and when) their wishes come true.
> 
> Dscho, that's just not fair.
> 
> The fact is, stating what you wish for *is* taking an action. Starting 
> to complain about people stating their wishes (which you have done 
> several times) is simply unreasonable.

Well, maybe I overreacted.

> You don't have to *do* what they wish for, but I really wish you stopped 
> complaining about people bringing up their hopes for improvement.

Fair enough, I'll shut up about these issues.

At least as long as I can hold my breath ;-)

> Complain about it when somebody asks for something *stupid*. Explain why 
> it would be wrong to do something like that. But don't complain about 
> people having wish-lists, even if those people may not work on them.
> 
> Not everybody is a "doer". It's important to get input from people who are 
> just plain users, or hope to be.

A pity, but you're probably right.

Ciao,
Dscho

^ permalink raw reply

* Re: Git User's Survey 2007 unfinished summary continued
From: Shawn O. Pearce @ 2007-10-14  1:44 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Johannes Schindelin, J. Bruce Fields, Jakub Narebski, git
In-Reply-To: <alpine.LFD.0.999.0710131810550.6887@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Sun, 14 Oct 2007, Johannes Schindelin wrote:
> >
> > My main point is -- and always was -- that I'd like people to realise how 
> > much it depends on _them_ if (and when) their wishes come true.
> 
> Dscho, that's just not fair.
...
> Complain about it when somebody asks for something *stupid*. Explain why 
> it would be wrong to do something like that. But don't complain about 
> people having wish-lists, even if those people may not work on them.
> 
> Not everybody is a "doer". It's important to get input from people who are 
> just plain users, or hope to be.

I agree with both of you.  My understanding of Dscho's original
comment was that people weren't saying *what* specifically their
wish-list was, which means we have no hope as a community of meeting
their requests.

Carl and Andy both had submitted a long list of very specific issues
that they had with Git.  The result of those lists being posted was
a number of people contributed improvements that lead us to 1.5.
Nobody can argue with that.

But just saying "MY GOD FIX THE UI" is not a wishlist item (yes,
that was a real survey answer).  It provides the community no
chance to understand what parts of the UI we need to work on, and
what parts the end-user is OK with or just hasn't even tried to use.

-- 
Shawn.

^ permalink raw reply

* Re: Git User's Survey 2007 unfinished summary continued
From: Linus Torvalds @ 2007-10-14  1:13 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: J. Bruce Fields, Jakub Narebski, git
In-Reply-To: <Pine.LNX.4.64.0710140135020.25221@racer.site>



On Sun, 14 Oct 2007, Johannes Schindelin wrote:
>
> My main point is -- and always was -- that I'd like people to realise how 
> much it depends on _them_ if (and when) their wishes come true.

Dscho, that's just not fair.

The fact is, stating what you wish for *is* taking an action. Starting to 
complain about people stating their wishes (which you have done several 
times) is simply unreasonable.

You don't have to *do* what they wish for, but I really wish you stopped 
complaining about people bringing up their hopes for improvement.

Complain about it when somebody asks for something *stupid*. Explain why 
it would be wrong to do something like that. But don't complain about 
people having wish-lists, even if those people may not work on them.

Not everybody is a "doer". It's important to get input from people who are 
just plain users, or hope to be.

		Linus

^ permalink raw reply

* Re: [PATCH 01/14] Change git_connect() to return a struct child_process instead of a pid_t.
From: Johannes Schindelin @ 2007-10-14  0:57 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: gitster, git
In-Reply-To: <1192305984-22594-2-git-send-email-johannes.sixt@telecom.at>

Hi,

On Sat, 13 Oct 2007, Johannes Sixt wrote:

> -int finish_connect(pid_t pid)
> +int finish_connect(struct child_process *conn)
>  {
> -	if (pid == 0)
> +	if (conn == NULL)
>  		return 0;
>  
> -	while (waitpid(pid, NULL, 0) < 0) {
> +	while (waitpid(conn->pid, NULL, 0) < 0) {
>  		if (errno != EINTR)
>  			return -1;

Just for completeness' sake: could you do a free(conn); before return -1;?

Thanks,
Dscho

^ permalink raw reply

* Re: Addition of "xmlto" to install documentation
From: Johannes Schindelin @ 2007-10-14  0:43 UTC (permalink / raw)
  To: Markus Elfring; +Cc: git
In-Reply-To: <47112DAA.5080701@web.de>

Hi,

On Sat, 13 Oct 2007, Markus Elfring wrote:

> I have cloned the current Git release to my computer. I resolved all 
> dependencies that were mentioned in the file "INSTALL". But when I've 
> tried "make install install-doc" I got the message that "xmlto" was not 
> found on my openSUSE 10.3 system. (I have installed it now.) Would you 
> like to add this tool to the system requirements in the documentation?

Well, it is not strictly necessary to build git, and not even to install 
it, if you have the "man" branch.

Ciao,
Dscho

^ permalink raw reply

* Re: Git User's Survey 2007 unfinished summary continued
From: Johannes Schindelin @ 2007-10-14  0:36 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Jakub Narebski, git
In-Reply-To: <20071013202713.GA2467@fieldses.org>

Hi,

On Sat, 13 Oct 2007, J. Bruce Fields wrote:

> On Sat, Oct 13, 2007 at 09:59:41PM +0200, David Kastrup wrote:
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > 
> ...survey quote:
> > >>    Figure out why people find git hard to learn and eliminate those
> > >>    barriers to entry.  Make git more task-oriented rather than
> > >>    data-model-oriented the way it is now.
> > >
> > > Frankly, expectations like these make me want to bang somebody's
> > > head on the wall.
> > 
> > And you wonder that people are unwilling to ask for things on the
> > list?

That is utter rubbish.

> Well, he does have a point that they could have been more specific.
> 
> But, yes, "I wish we could get people to be more specific" might be the 
> better way to put it.

Yes, there are politer ways to phrase it.

My main point is -- and always was -- that I'd like people to realise how 
much it depends on _them_ if (and when) their wishes come true.

Ciao,
Dscho

^ permalink raw reply

* Re: Imports without Tariffs
From: Michael Witten @ 2007-10-13 23:04 UTC (permalink / raw)
  To: git; +Cc: Jeff King
In-Reply-To: <20071013075723.GA27533@coredump.intra.peff.net>


On 13 Oct 2007, at 3:57:23 AM, Jeff King wrote:

> On Sat, Oct 13, 2007 at 12:30:09AM -0400, Michael Witten wrote:
>
>>> except that git-rebase is smart enough to realize that C == C'  
>>> and skip
>>> it (so it's a "safe" way of moving forward).
>>
>> This is good to know! The documentation should mention this case!
>
> Yes, it probably should. Can you submit a patch describing the  
> behavior
> where you think it ought to go?

I can make a patch, but at the moment I'm swamped and I don't want to  
think
about doing that.

I'll get around to it eventually, I hope.

Do I just submit the patch to this list? How do I know it will be used?

>> Basically, the imported cvs history should be treated like
>> a remote that's being tracked. It seems like the solution
>> I proposed kind of does this and would work for other SCM
>> imports too.
>
> I think it's an interesting avenue to pursue, though I would worry a
> little about robustness. I like the fact that after rebasing, the
> commits have done a complete git->cvs->git loop and look identical

Frankly, I don't know how robust my idea is either, but it's simple
enough not to have many problems lurking in the shadows.

It would certainly be more useful than not.

> As an alternate idea, why not try to have the CVS commit contain all
> information necessary to create a particular git commit. IOW, describe
> all of the data that goes into the commit hash as textual comments in
> the CVS commit (committer name/time, author name/time). And then
> theoretically git-cvsimport can reconstruct the exact same commits
> again, and your git->cvs->git merge really _will_ be a fastforward.

I considered this too, but this exposes what we're doing. We don't
want the old farts to wonder what all these hash thingies are.

Michael Witten

^ permalink raw reply

* Re: [PATCH] Fixing path quoting issues
From: Andreas Ericsson @ 2007-10-13 22:36 UTC (permalink / raw)
  To: Jonathan del Strother; +Cc: Johannes Sixt, git, Junio C Hamano
In-Reply-To: <92879AC5-2927-439B-8EB0-AC20AAEE412E@steelskies.com>

Jonathan del Strother wrote:
> On 11 Oct 2007, at 07:19, Johannes Sixt wrote:
> 
>>> -     git-commit -F msg -m amending ."
>>> +    git-commit -F msg -m amending ."
>>
>> You fix whitespace...
>>
>>>  test_expect_success \
>>> -    "using message from other commit" \
>>> -    "git-commit -C HEAD^ ."
>>> +     "using message from other commit" \
>>> +     "git-commit -C HEAD^ ."
>>
>> ... and you break it. More of these follow. Don't do that, it makes 
>> patch review unnecessarily hard.
> 
> 
> I'm just preparing to release this patch... was that "don't break 
> whitespace", or "don't try to fix whitespace in a patch that's has 
> nothing to do with whitespacing-fixing" ?
> 

Both, I think ;-)

> And while I'm here - tabs are preferred, are they?  There seem to be a 
> mixture of tabs & 4 space indentation.

1 hard tab / level of indent, but use spaces for continuation alignment.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: Addition of "xmlto" to install documentation
From: Alex Riesen @ 2007-10-13 22:29 UTC (permalink / raw)
  To: Markus Elfring; +Cc: git
In-Reply-To: <47112DAA.5080701@web.de>

Markus Elfring, Sat, Oct 13, 2007 22:42:18 +0200:
> 
> I have cloned the current Git release to my computer. I resolved all
> dependencies that were mentioned in the file "INSTALL". But when I've
> tried "make install install-doc" I got the message that "xmlto" was not
> found on my openSUSE 10.3 system. (I have installed it now.)
> Would you like to add this tool to the system requirements in the
> documentation?
> 

It is already there, in the same INSTALL file:

 - To build and install documentation suite, you need to have
   the asciidoc/xmlto toolchain.  Because not many people are
   inclined to install the tools, the default build target
   ("make all") does _not_ build them.

^ permalink raw reply

* Re: [PATCH] Color support added to git-add--interactive.
From: Jean-Luc Herren @ 2007-10-13 22:23 UTC (permalink / raw)
  To: Dan Z, git
In-Reply-To: <cff973550710131450r3b54a328k8db97488f4b50e2a@mail.gmail.com>

Dan Z wrote:
> I think color.add is better, because git-add--interactive goes beyond
> coloring diffs. When this is complete, it should probably use
> color.diff.<slot> for the actual diff output, and color.add.<slot> for
> colored prompts/commands.

Or maybe rather color.interactive.<slot>, where <slot> could be
'prompt', 'header', etc.  It's better to give it a name that
describes what it is for, instead of one that describes which tool
uses it.  This way it could possibly be used for other potential
interactive tools in the future.

jlh

^ permalink raw reply

* Re: [PATCH] Add a simple option parser.
From: Alex Riesen @ 2007-10-13 22:14 UTC (permalink / raw)
  To: Pierre Habouzit, git, Junio C Hamano
In-Reply-To: <20071013205404.GK7110@artemis.corp>

Pierre Habouzit, Sat, Oct 13, 2007 22:54:04 +0200:
> On Sat, Oct 13, 2007 at 07:16:55PM +0000, Alex Riesen wrote:
> > Pierre Habouzit, Sat, Oct 13, 2007 15:29:03 +0200:
> > BTW, if you just printed the usage message out (it is about usage of a
> > program, isn't it?) and called exit() everyone would be just as happy.
> > And you wouldn't have to include strbuf (it is the only use of it),
> > less code, too. It'd make simplier to stea^Wcopy your implementation,
> > which I like :)
> 
>   the reason is that usage() is a wrapper around a callback, and I
> suppose it's used by some GUI's or anything like that.

It is not. Not yet. What could they use a usage text for?
Besides, you could just export the callback (call_usage_callback or
something) from usage.c and call it.

>   FWIW you can rework the .c like this:

on top of yours:

From: Alex Riesen <raa.lkml@gmail.com>
Date: Sun, 14 Oct 2007 00:10:51 +0200
Subject: [PATCH] Rework make_usage to print the usage message immediately

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 parse-options.c |   60 ++++++++++++++++++++++++------------------------------
 1 files changed, 27 insertions(+), 33 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 07abb50..1e3940f 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1,6 +1,5 @@
 #include "git-compat-util.h"
 #include "parse-options.h"
-#include "strbuf.h"
 
 #define OPT_SHORT 1
 #define OPT_UNSET 2
@@ -171,57 +170,52 @@ int parse_options(int argc, const char **argv,
 
 void make_usage(const char * const usagestr[], struct option *opts, int cnt)
 {
-	struct strbuf sb;
-
-	strbuf_init(&sb, 4096);
-	do {
-		strbuf_addstr(&sb, *usagestr++);
-		strbuf_addch(&sb, '\n');
-	} while (*usagestr);
+	fprintf(stderr, "usage: ");
+	while (*usagestr)
+		fprintf(stderr, "%s\n", *usagestr++);
 
 	if (cnt && opts->type != OPTION_GROUP)
-		strbuf_addch(&sb, '\n');
+		fputc('\n', stderr);
 
 	for (; cnt-- > 0; opts++) {
 		size_t pos;
 
 		if (opts->type == OPTION_GROUP) {
-			strbuf_addch(&sb, '\n');
+			fputc('\n', stderr);
 			if (*opts->help)
-				strbuf_addf(&sb, "%s\n", opts->help);
+				fprintf(stderr, "%s\n", opts->help);
 			continue;
 		}
 
-		pos = sb.len;
-		strbuf_addstr(&sb, "    ");
-		if (opts->short_name) {
-			strbuf_addf(&sb, "-%c", opts->short_name);
-		}
-		if (opts->long_name) {
-			strbuf_addf(&sb, opts->short_name ? ", --%s" : "--%s",
-						opts->long_name);
-		}
+		pos = fprintf(stderr, "    ");
+		if (opts->short_name)
+			pos += fprintf(stderr, "-%c", opts->short_name);
+		if (opts->long_name)
+			pos += fprintf(stderr,
+				       opts->short_name ? ", --%s" : "--%s",
+				       opts->long_name);
 		switch (opts->type) {
 		case OPTION_INTEGER:
-			strbuf_addstr(&sb, " <n>");
+			fputs(" <n>", stderr);
+			pos += 4;
 			break;
 		case OPTION_STRING:
-			if (opts->argh) {
-				strbuf_addf(&sb, " <%s>", opts->argh);
-			} else {
-				strbuf_addstr(&sb, " ...");
+			if (opts->argh)
+				pos += fprintf(stderr, " <%s>", opts->argh);
+			else {
+				fputs(" ...", stderr);
+				pos += 4;
 			}
 			break;
 		default:
 			break;
 		}
-		if (sb.len - pos <= USAGE_OPTS_WIDTH) {
-			int pad = USAGE_OPTS_WIDTH - (sb.len - pos) + USAGE_GAP;
-			strbuf_addf(&sb, "%*s%s\n", pad, "", opts->help);
-		} else {
-			strbuf_addf(&sb, "\n%*s%s\n", USAGE_OPTS_WIDTH + USAGE_GAP, "",
-						opts->help);
-		}
+		if (pos <= USAGE_OPTS_WIDTH) {
+			int pad = USAGE_OPTS_WIDTH - pos + USAGE_GAP;
+			fprintf(stderr, "%*s%s\n", pad, "", opts->help);
+		} else
+			fprintf(stderr, "\n%*s%s\n",
+				USAGE_OPTS_WIDTH + USAGE_GAP, "", opts->help);
 	}
-	usage(sb.buf);
+	exit(129);
 }
-- 
1.5.3.4.232.ga843

^ permalink raw reply related

* Re: [PATCH] Color support added to git-add--interactive.
From: Dan Z @ 2007-10-13 21:50 UTC (permalink / raw)
  To: Wincent Colaiuta
  Cc: Jeff King, Git Mailing List, Jonathan del Strother,
	Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <8DDFBF9A-2C68-404B-843C-BE63C52F0DAF@wincent.com>

On 10/13/07, Wincent Colaiuta <win@wincent.com> wrote:
> Or could you just piggy-back on the settings for color.diff.<slot>?
>
> And if a separate group for git-add is necessary, perhaps "add" would
> be enough, rather than "add-interactive".
>
> Wincent
>

I think color.add is better, because git-add--interactive goes beyond
coloring diffs. When this is complete, it should probably use
color.diff.<slot> for the actual diff output, and color.add.<slot> for
colored prompts/commands.

Dan

^ permalink raw reply

* Re: Git User's Survey 2007 unfinished summary continued
From: David Kastrup @ 2007-10-13 20:57 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Johannes Schindelin, Jakub Narebski, git
In-Reply-To: <20071013202713.GA2467@fieldses.org>

"J. Bruce Fields" <bfields@fieldses.org> writes:

> On Sat, Oct 13, 2007 at 09:59:41PM +0200, David Kastrup wrote:
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
> ...survey quote:
>> >>    Figure out why people find git hard to learn and eliminate those
>> >>    barriers to entry.  Make git more task-oriented rather than
>> >>    data-model-oriented the way it is now.
>> >
>> > Frankly, expectations like these make me want to bang somebody's
>> > head on the wall.
>> 
>> And you wonder that people are unwilling to ask for things on the
>> list?  When even mentioning something in a _survey_ makes core
>> developers want to bang their heads against a wall?
>
> Well, he does have a point that they could have been more specific.

He did not write "vague phrasings like this".  He wrote
"_expectations_ like these make [him] want to bang somebody's head on
the wall".

> But, yes, "I wish we could get people to be more specific" might be
> the better way to put it.

It is something entirely different.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* Re: [PATCH] Add a simple option parser.
From: Pierre Habouzit @ 2007-10-13 20:54 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git, Junio C Hamano
In-Reply-To: <20071013191655.GA2875@steel.home>

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

On Sat, Oct 13, 2007 at 07:16:55PM +0000, Alex Riesen wrote:
> Pierre Habouzit, Sat, Oct 13, 2007 15:29:03 +0200:
> > [...]
> 
> "const struct option *opts"?
> 
> Why not "const char *const *usagestr"? Especially if you change
> "usagestr" (the pointer itself) later. "[]" is sometimes a hint that
> the pointer itself should not be changed, being an array.
> 
> And you want make opts const.

  Ok.

> BTW, it does not "make" usage. It calls the usage() or prints a usage
> description. "make" implies it creates the "usage", which according to
> the prototype is later nowhere to be found.

  Yes this has been spotted and fixed already.

> 
> > +{
> > +	struct strbuf sb;
> > +
> > +	strbuf_init(&sb, 4096);
> > +	do {
> > +		strbuf_addstr(&sb, *usagestr++);
> > +		strbuf_addch(&sb, '\n');
> > +	} while (*usagestr);
> 
> This will crash for empty usagestr, like  "{ NULL }". Was it
> deliberately? (I'd make it deliberately, if I were you. I'd even used
> cnt of opts, to force people to document all options).

  Yes this is intentional, there should be at least on string in the
usagestr array.


> > +     strbuf_addf(&sb, "\n%*s%s\n", USAGE_OPTS_WIDTH + USAGE_GAP, "",
> > +		    opts->help);
> ....
> > +	usage(sb.buf);
> 
> BTW, if you just printed the usage message out (it is about usage of a
> program, isn't it?) and called exit() everyone would be just as happy.
> And you wouldn't have to include strbuf (it is the only use of it),
> less code, too. It'd make simplier to stea^Wcopy your implementation,
> which I like :)

  the reason is that usage() is a wrapper around a callback, and I
suppose it's used by some GUI's or anything like that.

  FWIW you can rework the .c like this:

  pos = 0; /* and not pos = sb.len */

  replace the strbuf_add* by the equivalents:
  pos += printf("....");

  and tada, you're done.


  Note that in the most recent version, I also deal with a
OPTION_CALLBACK that passes the value to a callback.

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

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

^ permalink raw reply

* Addition of "xmlto" to install documentation
From: Markus Elfring @ 2007-10-13 20:42 UTC (permalink / raw)
  To: git

Hello,

I have cloned the current Git release to my computer. I resolved all
dependencies that were mentioned in the file "INSTALL". But when I've
tried "make install install-doc" I got the message that "xmlto" was not
found on my openSUSE 10.3 system. (I have installed it now.)
Would you like to add this tool to the system requirements in the
documentation?

Regards,
Markus

^ permalink raw reply

* Re: [PATCH] Color support added to git-add--interactive.
From: Wincent Colaiuta @ 2007-10-13 20:36 UTC (permalink / raw)
  To: Dan Zwell
  Cc: Jeff King, Git Mailing List, Jonathan del Strother,
	Johannes Schindelin, Frank Lichtenheld
In-Reply-To: <47112491.8070309@gmail.com>

El 13/10/2007, a las 22:03, Dan Zwell escribió:

> The importance of the diff coloring pales in comparison to the  
> prompt coloring. Diff coloring is useful, but prompt coloring is a  
> basic usability concern (if people can't easily tell where a hunk  
> begins, the tool becomes annoying). Perhaps we could split this  
> into two patches, merging the first after a few small changes can  
> be taken care of, while the second may need more discussion and  
> testing. The coloring of the prompts is relatively low risk. It  
> just needs to be modified to take color settings from .git/config.  
> I was thinking that this might be the example that I would take  
> settings from:
>
> [color]
>         add-interactive = auto
> [color "add-interactive"]
>         prompt = bold blue
>         header = bold
>         help = blue

Or could you just piggy-back on the settings for color.diff.<slot>?

And if a separate group for git-add is necessary, perhaps "add" would  
be enough, rather than "add-interactive".

Wincent

^ permalink raw reply

* Re: Git User's Survey 2007 unfinished summary continued
From: J. Bruce Fields @ 2007-10-13 20:27 UTC (permalink / raw)
  To: David Kastrup; +Cc: Johannes Schindelin, Jakub Narebski, git
In-Reply-To: <853awepyz6.fsf@lola.goethe.zz>

On Sat, Oct 13, 2007 at 09:59:41PM +0200, David Kastrup wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
...survey quote:
> >>    Figure out why people find git hard to learn and eliminate those
> >>    barriers to entry.  Make git more task-oriented rather than
> >>    data-model-oriented the way it is now.
> >
> > Frankly, expectations like these make me want to bang somebody's
> > head on the wall.
> 
> And you wonder that people are unwilling to ask for things on the
> list?  When even mentioning something in a _survey_ makes core
> developers want to bang their heads against a wall?

Well, he does have a point that they could have been more specific.

But, yes, "I wish we could get people to be more specific" might be the
better way to put it.

--b.

^ 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