Git development
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Olivier Marin <dkr+ml.git@free.fr>
Cc: Miklos Vajna <vmiklos@frugalware.org>,
	Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Olivier Marin <dkr@freesurf.fr>
Subject: Re: [PATCH 01/15] Move split_cmdline() to alias.c
Date: Sun, 29 Jun 2008 15:43:54 +0100 (BST)	[thread overview]
Message-ID: <alpine.DEB.1.00.0806291542070.9925@racer> (raw)
In-Reply-To: <48679C44.30702@free.fr>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1815 bytes --]

Hi,

On Sun, 29 Jun 2008, Olivier Marin wrote:

> Johannes Schindelin a écrit :
> > 
> > As the string comes either from the config (where it is trimmed), or 
> > from the command line (where the user can be stup^W^Wask for 
> > whitespace explicitely), I do not see much merit in this patch.
> 
> You are right, today it is not a problem because the usage is really 
> limited but Miklos's intention seems to make the function usable by 
> everyone. As we do not know how it will be used in the future, I think 
> it is safer with my patch.

I am generally not a fan of crossing bridges miles before you reach them.

But if you want to keep running with this, you should add at least a few 
tests to show what you patch solves, and that it solves it.  Preferably 
with a "test_expect_failure" patch, and then a patch fixing it and 
changing that _failure to _success.

Here's a starter:

-- snipsnap --
diff --git a/Makefile b/Makefile
index d3e339a..83a9a30 100644
--- a/Makefile
+++ b/Makefile
@@ -1229,7 +1229,7 @@ endif
 
 ### Testing rules
 
-TEST_PROGRAMS = test-chmtime$X test-genrandom$X test-date$X test-delta$X test-sha1$X test-match-trees$X test-absolute-path$X test-parse-options$X test-strbuf$X
+TEST_PROGRAMS = test-chmtime$X test-genrandom$X test-date$X test-delta$X test-sha1$X test-match-trees$X test-absolute-path$X test-parse-options$X test-strbuf$X test-cmdline$X
 
 all:: $(TEST_PROGRAMS)
 
diff --git a/test-cmdline.c b/test-cmdline.c
new file mode 100644
index 0000000..2f6d9eb
--- /dev/null
+++ b/test-cmdline.c
@@ -0,0 +1,15 @@
+#include "cache.h"
+
+int main(int argc, char **argv)
+{
+	int i = 0;
+	const char **new_argv;
+
+	if (argc < 2 || (argc = split_cmdline(argv[1], &new_argv)) < 0)
+		return 1;
+
+	while (i < argc)
+		printf("arg %d: '%s'\n", i++, *new_argv++);
+
+	return 0;
+}

  reply	other threads:[~2008-06-29 14:46 UTC|newest]

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-27 16:21 [PATCH 00/15] Build in merge Miklos Vajna
2008-06-27 16:21 ` [PATCH 01/15] Move split_cmdline() to alias.c Miklos Vajna
2008-06-29 14:05   ` Olivier Marin
2008-06-29 14:15     ` Johannes Schindelin
2008-06-29 14:29       ` Olivier Marin
2008-06-29 14:43         ` Johannes Schindelin [this message]
2008-06-30 22:51           ` Olivier Marin
2008-06-27 16:21 ` [PATCH 02/15] Move commit_list_count() to commit.c Miklos Vajna
2008-06-27 16:21 ` [PATCH 03/15] Move parse-options's skip_prefix() to git-compat-util.h Miklos Vajna
2008-06-27 16:21 ` [PATCH 04/15] Add new test to ensure git-merge handles pull.twohead and pull.octopus Miklos Vajna
2008-06-29 13:30   ` Olivier Marin
2008-06-29 14:51     ` [PATCH] " Miklos Vajna
2008-06-29 15:11       ` Miklos Vajna
2008-07-04 16:34   ` [PATCH 04/15] " Mike Ralphson
2008-07-05  0:26     ` Miklos Vajna
2008-07-05  0:32       ` [PATCH] Fix t7601-merge-pull-config.sh on AIX Miklos Vajna
2008-07-05  1:49         ` Junio C Hamano
2008-06-27 16:21 ` [PATCH 05/15] Move read_cache_unmerged() to read-cache.c Miklos Vajna
2008-06-27 16:21 ` [PATCH 06/15] git-fmt-merge-msg: make it usable from other builtins Miklos Vajna
2008-06-27 16:22 ` [PATCH 07/15] Introduce get_octopus_merge_bases() in commit.c Miklos Vajna
2008-06-27 16:22 ` [PATCH 08/15] Add new test to ensure git-merge handles more than 25 refs Miklos Vajna
2008-06-27 17:06   ` Miklos Vajna
2008-06-27 16:22 ` [PATCH 09/15] Introduce get_merge_bases_many() Miklos Vajna
2008-06-27 16:22 ` [PATCH 10/15] Introduce reduce_heads() Miklos Vajna
2008-06-27 16:22 ` [PATCH 11/15] Add strbuf_vaddf(), use it in strbuf_addf(), and add strbuf_initf() Miklos Vajna
2008-06-28  2:00   ` Junio C Hamano
2008-06-28  2:33     ` Miklos Vajna
2008-06-28  2:38       ` [PATCH 13/13] Build in merge Miklos Vajna
2008-06-29  7:46         ` Junio C Hamano
2008-06-29  8:11           ` Jakub Narebski
2008-06-30  1:36           ` Miklos Vajna
2008-06-30  1:39             ` Miklos Vajna
2008-06-30  5:44               ` Junio C Hamano
2008-06-30 17:41                 ` Alex Riesen
2008-07-01  2:13                 ` Miklos Vajna
2008-07-01  2:22                   ` [PATCH 13/14] git-commit-tree: make it usable from other builtins Miklos Vajna
2008-07-01  5:07                     ` Johannes Schindelin
2008-07-01  5:50                     ` Junio C Hamano
2008-07-01 12:09                       ` Miklos Vajna
2008-07-01  2:22                   ` [PATCH 14/14] Build in merge Miklos Vajna
2008-07-01  2:37                     ` [PATCH 00/14] " Miklos Vajna
2008-07-01  2:37                       ` [PATCH 08/14] Add new test to ensure git-merge handles more than 25 refs Miklos Vajna
2008-07-01  2:37                       ` [PATCH 13/14] git-commit-tree: make it usable from other builtins Miklos Vajna
2008-07-01  2:37                       ` [PATCH 14/14] Build in merge Miklos Vajna
2008-07-01  6:23                         ` Junio C Hamano
2008-07-01 12:50                           ` Miklos Vajna
2008-07-01 13:18                             ` Miklos Vajna
2008-07-01 23:55                               ` Junio C Hamano
2008-07-02  7:43                                 ` Miklos Vajna
2008-07-06  8:50                               ` Junio C Hamano
2008-07-06  9:43                                 ` Junio C Hamano
2008-07-07 17:17                                   ` Miklos Vajna
2008-07-07 18:15                                     ` Junio C Hamano
2008-07-07 23:42                                       ` [PATCH] " Miklos Vajna
2008-07-08  0:32                                         ` Junio C Hamano
2008-07-08  0:53                                           ` Junio C Hamano
2008-07-08  1:18                                             ` Miklos Vajna
2008-07-08  1:00                                           ` Miklos Vajna
2008-07-08  1:05                                             ` Junio C Hamano
2008-07-08  1:41                                               ` Miklos Vajna
2008-07-06 12:38                                 ` [PATCH 14/14] " Johannes Schindelin
2008-07-06 19:39                                   ` Junio C Hamano
2008-07-07 17:24                                 ` [PATCH] " Miklos Vajna
2008-07-07 17:35                                   ` Miklos Vajna
2008-07-01  7:27                         ` [PATCH 14/14] " Junio C Hamano
2008-07-01 12:55                           ` Miklos Vajna
2008-06-30 22:44               ` [PATCH 13/13] " Olivier Marin
2008-06-30 22:58                 ` Miklos Vajna
2008-06-30  5:40             ` Junio C Hamano
2008-06-30 22:48               ` Miklos Vajna
2008-06-28 17:33     ` [PATCH 11/15] Add strbuf_vaddf(), use it in strbuf_addf(), and add strbuf_initf() Johannes Schindelin
2008-06-29  8:07       ` Junio C Hamano
2008-06-29 13:40         ` Johannes Schindelin
2008-06-29 20:17           ` Alex Riesen
2008-06-29 20:24             ` Junio C Hamano
2008-06-29 20:30               ` Sverre Rabbelier
2008-06-27 16:22 ` [PATCH 12/15] strbuf_vaddf(): support %*s, too Miklos Vajna
2008-06-27 16:22 ` [PATCH 13/15] Add new test case to ensure git-merge reduces octopus parents when possible Miklos Vajna
2008-06-27 16:22 ` [PATCH 14/15] Add new test case to ensure git-merge prepends the custom merge message Miklos Vajna
2008-06-27 16:22 ` [PATCH 15/15] Build in merge Miklos Vajna
2008-06-27 17:09   ` Miklos Vajna

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.1.00.0806291542070.9925@racer \
    --to=johannes.schindelin@gmx.de \
    --cc=dkr+ml.git@free.fr \
    --cc=dkr@freesurf.fr \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=vmiklos@frugalware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox