Git development
 help / color / mirror / Atom feed
* [PATCH DIFF-CLEANUP 2/2] Reorder diff_opt_parse options more logically per topics.
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-7-git-send-email-madcoder@debian.org>

This is a line reordering patch _only_.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
    the 10 added lines are five times a blank line and a header for the each
    of the 5 options groups.

    This patch is very useful as it will help checking that I didn't missed
    any option while converting to parseopt, it's not coquetry.

 diff.c |  116 ++++++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 63 insertions(+), 53 deletions(-)

diff --git a/diff.c b/diff.c
index 97c9a59..7a89959 100644
--- a/diff.c
+++ b/diff.c
@@ -2198,6 +2198,8 @@ static int diff_scoreopt_parse(const char *opt);
 int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 {
 	const char *arg = av[0];
+
+	/* Output format options */
 	if (!strcmp(arg, "-p") || !strcmp(arg, "-u"))
 		options->output_format |= DIFF_FORMAT_PATCH;
 	else if (opt_arg(arg, 'U', "unified", &options->context))
@@ -2210,6 +2212,18 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->output_format |= DIFF_FORMAT_NUMSTAT;
 	else if (!strcmp(arg, "--shortstat"))
 		options->output_format |= DIFF_FORMAT_SHORTSTAT;
+	else if (!strcmp(arg, "--check"))
+		options->output_format |= DIFF_FORMAT_CHECKDIFF;
+	else if (!strcmp(arg, "--summary"))
+		options->output_format |= DIFF_FORMAT_SUMMARY;
+	else if (!strcmp(arg, "--patch-with-stat"))
+		options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT;
+	else if (!strcmp(arg, "--name-only"))
+		options->output_format |= DIFF_FORMAT_NAME;
+	else if (!strcmp(arg, "--name-status"))
+		options->output_format |= DIFF_FORMAT_NAME_STATUS;
+	else if (!strcmp(arg, "-s"))
+		options->output_format |= DIFF_FORMAT_NO_OUTPUT;
 	else if (!prefixcmp(arg, "--stat")) {
 		char *end;
 		int width = options->stat_width;
@@ -2237,42 +2251,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->stat_name_width = name_width;
 		options->stat_width = width;
 	}
-	else if (!strcmp(arg, "--check"))
-		options->output_format |= DIFF_FORMAT_CHECKDIFF;
-	else if (!strcmp(arg, "--summary"))
-		options->output_format |= DIFF_FORMAT_SUMMARY;
-	else if (!strcmp(arg, "--patch-with-stat"))
-		options->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT;
-	else if (!strcmp(arg, "-z"))
-		options->line_termination = 0;
-	else if (!prefixcmp(arg, "-l"))
-		options->rename_limit = strtoul(arg+2, NULL, 10);
-	else if (!strcmp(arg, "--full-index"))
-		DIFF_OPT_SET(options, FULL_INDEX);
-	else if (!strcmp(arg, "--binary")) {
-		options->output_format |= DIFF_FORMAT_PATCH;
-		DIFF_OPT_SET(options, BINARY);
-	}
-	else if (!strcmp(arg, "-a") || !strcmp(arg, "--text"))
-		DIFF_OPT_SET(options, TEXT);
-	else if (!strcmp(arg, "--name-only"))
-		options->output_format |= DIFF_FORMAT_NAME;
-	else if (!strcmp(arg, "--name-status"))
-		options->output_format |= DIFF_FORMAT_NAME_STATUS;
-	else if (!strcmp(arg, "-R"))
-		DIFF_OPT_SET(options, REVERSE_DIFF);
-	else if (!prefixcmp(arg, "-S"))
-		options->pickaxe = arg + 2;
-	else if (!strcmp(arg, "-s"))
-		options->output_format |= DIFF_FORMAT_NO_OUTPUT;
-	else if (!prefixcmp(arg, "-O"))
-		options->orderfile = arg + 2;
-	else if (!prefixcmp(arg, "--diff-filter="))
-		options->filter = arg + 14;
-	else if (!strcmp(arg, "--pickaxe-all"))
-		options->pickaxe_opts = DIFF_PICKAXE_ALL;
-	else if (!strcmp(arg, "--pickaxe-regex"))
-		options->pickaxe_opts = DIFF_PICKAXE_REGEX;
+
+	/* renames options */
 	else if (!prefixcmp(arg, "-B")) {
 		if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)
 			return -1;
@@ -2289,33 +2269,38 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 			return -1;
 		options->detect_rename = DIFF_DETECT_COPY;
 	}
+	else if (!strcmp(arg, "--no-renames"))
+		options->detect_rename = 0;
+
+	/* xdiff options */
+	else if (!strcmp(arg, "-w") || !strcmp(arg, "--ignore-all-space"))
+		options->xdl_opts |= XDF_IGNORE_WHITESPACE;
+	else if (!strcmp(arg, "-b") || !strcmp(arg, "--ignore-space-change"))
+		options->xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;
+	else if (!strcmp(arg, "--ignore-space-at-eol"))
+		options->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;
+
+	/* flags options */
+	else if (!strcmp(arg, "--binary")) {
+		options->output_format |= DIFF_FORMAT_PATCH;
+		DIFF_OPT_SET(options, BINARY);
+	}
+	else if (!strcmp(arg, "--full-index"))
+		DIFF_OPT_SET(options, FULL_INDEX);
+	else if (!strcmp(arg, "-a") || !strcmp(arg, "--text"))
+		DIFF_OPT_SET(options, TEXT);
+	else if (!strcmp(arg, "-R"))
+		DIFF_OPT_SET(options, REVERSE_DIFF);
 	else if (!strcmp(arg, "--find-copies-harder"))
 		DIFF_OPT_SET(options, FIND_COPIES_HARDER);
 	else if (!strcmp(arg, "--follow"))
 		DIFF_OPT_SET(options, FOLLOW_RENAMES);
-	else if (!strcmp(arg, "--abbrev"))
-		options->abbrev = DEFAULT_ABBREV;
-	else if (!prefixcmp(arg, "--abbrev=")) {
-		options->abbrev = strtoul(arg + 9, NULL, 10);
-		if (options->abbrev < MINIMUM_ABBREV)
-			options->abbrev = MINIMUM_ABBREV;
-		else if (40 < options->abbrev)
-			options->abbrev = 40;
-	}
 	else if (!strcmp(arg, "--color"))
 		DIFF_OPT_SET(options, COLOR_DIFF);
 	else if (!strcmp(arg, "--no-color"))
 		DIFF_OPT_CLR(options, COLOR_DIFF);
-	else if (!strcmp(arg, "-w") || !strcmp(arg, "--ignore-all-space"))
-		options->xdl_opts |= XDF_IGNORE_WHITESPACE;
-	else if (!strcmp(arg, "-b") || !strcmp(arg, "--ignore-space-change"))
-		options->xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;
-	else if (!strcmp(arg, "--ignore-space-at-eol"))
-		options->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;
 	else if (!strcmp(arg, "--color-words"))
 		options->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;
-	else if (!strcmp(arg, "--no-renames"))
-		options->detect_rename = 0;
 	else if (!strcmp(arg, "--exit-code"))
 		DIFF_OPT_SET(options, EXIT_WITH_STATUS);
 	else if (!strcmp(arg, "--quiet"))
@@ -2324,6 +2309,31 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		DIFF_OPT_SET(options, ALLOW_EXTERNAL);
 	else if (!strcmp(arg, "--no-ext-diff"))
 		DIFF_OPT_CLR(options, ALLOW_EXTERNAL);
+
+	/* misc options */
+	else if (!strcmp(arg, "-z"))
+		options->line_termination = 0;
+	else if (!prefixcmp(arg, "-l"))
+		options->rename_limit = strtoul(arg+2, NULL, 10);
+	else if (!prefixcmp(arg, "-S"))
+		options->pickaxe = arg + 2;
+	else if (!strcmp(arg, "--pickaxe-all"))
+		options->pickaxe_opts = DIFF_PICKAXE_ALL;
+	else if (!strcmp(arg, "--pickaxe-regex"))
+		options->pickaxe_opts = DIFF_PICKAXE_REGEX;
+	else if (!prefixcmp(arg, "-O"))
+		options->orderfile = arg + 2;
+	else if (!prefixcmp(arg, "--diff-filter="))
+		options->filter = arg + 14;
+	else if (!strcmp(arg, "--abbrev"))
+		options->abbrev = DEFAULT_ABBREV;
+	else if (!prefixcmp(arg, "--abbrev=")) {
+		options->abbrev = strtoul(arg + 9, NULL, 10);
+		if (options->abbrev < MINIMUM_ABBREV)
+			options->abbrev = MINIMUM_ABBREV;
+		else if (40 < options->abbrev)
+			options->abbrev = 40;
+	}
 	else
 		return 0;
 	return 1;
-- 
1.5.3.5.1598.gdef4e

^ permalink raw reply related

* [PATCH PARSEOPT 4/4] Use OPT_BIT in builtin-pack-refs
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-5-git-send-email-madcoder@debian.org>

---
 builtin-pack-refs.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index a62f06b..1923fb1 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -122,19 +122,13 @@ static char const * const pack_refs_usage[] = {
 
 int cmd_pack_refs(int argc, const char **argv, const char *prefix)
 {
-	int all = 0, prune = 1;
-	unsigned int flags = 0;
+	unsigned int flags = PACK_REFS_PRUNE;
 	struct option opts[] = {
-		OPT_BOOLEAN(0, "all", &all, "pack everything"),
-		OPT_BOOLEAN(0, "prune", &prune, "prune loose refs (default)"),
+		OPT_BIT(0, "all",   &flags, "pack everything", PACK_REFS_ALL),
+		OPT_BIT(0, "prune", &flags, "prune loose refs (default)", PACK_REFS_PRUNE),
 		OPT_END(),
 	};
-
 	if (parse_options(argc, argv, opts, pack_refs_usage, 0))
 		usage_with_options(pack_refs_usage, opts);
-	if (prune)
-		flags |= PACK_REFS_PRUNE;
-	if (all)
-		flags |= PACK_REFS_ALL;
 	return pack_refs(flags);
 }
-- 
1.5.3.5.1598.gdef4e

^ permalink raw reply related

* Preliminary patches to the diff.[hc] parseoptification
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
  To: gitster; +Cc: git

I'm currently working on the very involved task of migrating diff
options to parseopts. It's quite involved because for many reasons I
won't detail here, it needs to migrate those _and_ revisions options at
the same time (most of the diff options users _also_ use the revisions
ones).

So I'm trying to see some preliminary patches that I need to make this
the less disruptive possible: my goal is that people can use those
preliminary series bit by bit, and not a whole 10 to 20 patches (I
believe it will really take this amount in the end) at once with
breakages hard to find in the mess.

So in the coming days I'll probably send my work bit by bit when I feel
it won't change anymore.


Summary:
~~~~~~~

The 3 series are completely independent.

  + this is a patch to fix an issue that begin to irritate me and
    prevents me to build with -Werror.

    [1/1] Make gcc warning about empty if body go away.

  + this series add new features that are needed for the diff options.
    commit 1 implements them (see commit log,
    commits 2 to 4 use them in some commands that benefit from those.

    [1/4] parse-options new features.
    [2/4] Use OPT_SET_INT and OPT_BIT in builtin-branch
    [3/4] Use OPT_BIT in builtin-for-each-ref
    [4/4] Use OPT_BIT in builtin-pack-refs

  + this series does some modifications on the diff_options structure
    and the diff_opt_parse functions. Those are not fundamentally needed
    without parseopt, but parseopt will need them.

    Basically, diff_options had bitfields (the C :1 kind). parseopt cannot
    grok that portably, so I've converted these into a single
    `unsigned flags` with explicit masks to use (and helper macros to
    avoid overlong lines).

    The obvious goal is that I can use the OPTION_BIT feature from the
    previous series to fill them from parse_options.

    [1/2] Make the diff_options bitfields be an unsigned with explicit masks.
    [2/2] Reorder diff_opt_parse options more logically per topics.

Those patches are available on my public repository in the
ph/parseopt-stable branch.

ph/parseopt is my WIP and has many many horrible commits right now, that
I rebase -i to cleanse them once the big picture pleases me enough,
though people may want to look at diff.h in that branch to see where it
goes. I'm pleased to say that it seems I'll only need 3 diff-specific
callbacks, which is not a lot:

    http://git.madism.org/?p=git.git;a=blobdiff;f=diff.h;h=0e44e5;hp=6ff2b0;hb=def4eb;hpb=bdc1ab

I suppose that the split of this big macro is explanatory enough for the
reason behind commit 2/2 of the third series...


Diff stats:
~~~~~~~~~~

  + gcc issue:
     builtin-diff.c |    2 +-
     1 file changed, 1 insertion(+), 1 deletion(-)

  + parseopt features:
     builtin-branch.c       |   44 ++++++++++++-----------------------
     builtin-for-each-ref.c |   19 ++++++++-------
     builtin-pack-refs.c    |   12 ++-------
     git-compat-util.h      |    1 
     parse-options.c        |   61 +++++++++++++++++++++++++++++++++++--------------
     parse-options.h        |   15 +++++++++++-
     6 files changed, 88 insertions(+), 64 deletions(-)

  + diff-cleanup:
     builtin-blame.c      |   10 +-
     builtin-diff-files.c |    4 +-
     builtin-diff-index.c |    4 +-
     builtin-diff-tree.c  |    9 +-
     builtin-diff.c       |   12 ++--
     builtin-log.c        |   24 +++---
     combine-diff.c       |   10 +-
     diff-lib.c           |   23 +++---
     diff.c               |  221 ++++++++++++++++++++++++++------------------------
     diff.h               |   40 ++++++----
     log-tree.c           |    6 +-
     merge-recursive.c    |    2 +-
     patch-ids.c          |    2 +-
     revision.c           |   22 +++---
     tree-diff.c          |   14 ++--
     15 files changed, 209 insertions(+), 194 deletions(-)

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

^ permalink raw reply

* [PATCH PARSEOPT 3/4] Use OPT_BIT in builtin-for-each-ref
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-4-git-send-email-madcoder@debian.org>

---
 builtin-for-each-ref.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index da8c794..1a23ea6 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -833,16 +833,19 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	int i, num_refs;
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
 	struct ref_sort *sort = NULL, **sort_tail = &sort;
-	int maxcount = 0, quote_style;
-	int quote_shell = 0, quote_perl = 0, quote_python = 0, quote_tcl = 0;
+	int maxcount = 0, quote_style = 0;
 	struct refinfo **refs;
 	struct grab_ref_cbdata cbdata;
 
 	struct option opts[] = {
-		OPT_BOOLEAN('s', "shell", &quote_shell, "quote placeholders suitably for shells"),
-		OPT_BOOLEAN('p', "perl",  &quote_perl, "quote placeholders suitably for perl"),
-		OPT_BOOLEAN( 0 , "python", &quote_python, "quote placeholders suitably for python"),
-		OPT_BOOLEAN( 0 , "tcl",  &quote_tcl, "quote placeholders suitably for tcl"),
+		OPT_BIT('s', "shell", &quote_style,
+		        "quote placeholders suitably for shells", QUOTE_SHELL),
+		OPT_BIT('p', "perl",  &quote_style,
+		        "quote placeholders suitably for perl", QUOTE_PERL),
+		OPT_BIT(0 , "python", &quote_style,
+		        "quote placeholders suitably for python", QUOTE_PYTHON),
+		OPT_BIT(0 , "tcl",  &quote_style,
+		        "quote placeholders suitably for tcl", QUOTE_TCL),
 
 		OPT_GROUP(""),
 		OPT_INTEGER( 0 , "count", &maxcount, "show only <n> matched refs"),
@@ -857,15 +860,13 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		error("invalid --count argument: `%d'", maxcount);
 		usage_with_options(for_each_ref_usage, opts);
 	}
-	if (quote_shell + quote_perl + quote_python + quote_tcl > 1) {
+	if (HAS_MULTI_BITS(quote_style)) {
 		error("more than one quoting style ?");
 		usage_with_options(for_each_ref_usage, opts);
 	}
 	if (verify_format(format))
 		usage_with_options(for_each_ref_usage, opts);
 
-	quote_style = QUOTE_SHELL * quote_shell + QUOTE_PERL * quote_perl +
-		QUOTE_PYTHON * quote_python + QUOTE_TCL * quote_tcl;
 	if (!sort)
 		sort = default_sort();
 	sort_atom_limit = used_atom_cnt;
-- 
1.5.3.5.1598.gdef4e

^ permalink raw reply related

* [PATCH PARSEOPT 2/4] Use OPT_SET_INT and OPT_BIT in builtin-branch
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-3-git-send-email-madcoder@debian.org>

Also remove a spurious after-check on --abbrev (OPT__ABBREV already takes
care of that)
---
 builtin-branch.c |   44 ++++++++++++++++----------------------------
 1 files changed, 16 insertions(+), 28 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index 3bf40f1..2694c9c 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -507,48 +507,36 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
-	int delete = 0, force_delete = 0, force_create = 0;
-	int rename = 0, force_rename = 0;
+	int delete = 0, rename = 0, force_create = 0;
 	int verbose = 0, abbrev = DEFAULT_ABBREV, detached = 0;
 	int reflog = 0, track;
-	int kinds = REF_LOCAL_BRANCH, kind_remote = 0, kind_any = 0;
+	int kinds = REF_LOCAL_BRANCH;
 
 	struct option options[] = {
 		OPT_GROUP("Generic options"),
 		OPT__VERBOSE(&verbose),
 		OPT_BOOLEAN( 0 , "track",  &track, "set up tracking mode (see git-pull(1))"),
 		OPT_BOOLEAN( 0 , "color",  &branch_use_color, "use colored output"),
-		OPT_BOOLEAN('r', NULL,     &kind_remote, "act on remote-tracking branches"),
+		OPT_SET_INT('r', NULL,     &kinds, "act on remote-tracking branches",
+			REF_REMOTE_BRANCH),
 		OPT__ABBREV(&abbrev),
 
 		OPT_GROUP("Specific git-branch actions:"),
-		OPT_BOOLEAN('a', NULL,     &kind_any, "list both remote-tracking and local branches"),
-		OPT_BOOLEAN('d', NULL,     &delete, "delete fully merged branch"),
-		OPT_BOOLEAN('D', NULL,     &force_delete, "delete branch (even if not merged)"),
-		OPT_BOOLEAN('l', NULL,     &reflog, "create the branch's reflog"),
-		OPT_BOOLEAN('f', NULL,     &force_create, "force creation (when already exists)"),
-		OPT_BOOLEAN('m', NULL,     &rename, "move/rename a branch and its reflog"),
-		OPT_BOOLEAN('M', NULL,     &force_rename, "move/rename a branch, even if target exists"),
+		OPT_SET_INT('a', NULL, &kinds, "list both remote-tracking and local branches",
+			REF_REMOTE_BRANCH | REF_LOCAL_BRANCH),
+		OPT_BIT('d', NULL, &delete, "delete fully merged branch", 1),
+		OPT_BIT('D', NULL, &delete, "delete branch (even if not merged)", 2),
+		OPT_BIT('m', NULL, &rename, "move/rename a branch and its reflog", 1),
+		OPT_BIT('M', NULL, &rename, "move/rename a branch, even if target exists", 2),
+		OPT_BOOLEAN('l', NULL, &reflog, "create the branch's reflog"),
+		OPT_BOOLEAN('f', NULL, &force_create, "force creation (when already exists)"),
 		OPT_END(),
 	};
 
 	git_config(git_branch_config);
 	track = branch_track;
 	argc = parse_options(argc, argv, options, builtin_branch_usage, 0);
-
-	delete |= force_delete;
-	rename |= force_rename;
-	if (kind_remote)
-		kinds = REF_REMOTE_BRANCH;
-	if (kind_any)
-		kinds = REF_REMOTE_BRANCH | REF_LOCAL_BRANCH;
-	if (abbrev && abbrev < MINIMUM_ABBREV)
-		abbrev = MINIMUM_ABBREV;
-	else if (abbrev > 40)
-		abbrev = 40;
-
-	if ((delete && rename) || (delete && force_create) ||
-	    (rename && force_create))
+	if (!!delete + !!rename + !!force_create > 1)
 		usage_with_options(builtin_branch_usage, options);
 
 	head = resolve_ref("HEAD", head_sha1, 0, NULL);
@@ -564,13 +552,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	}
 
 	if (delete)
-		return delete_branches(argc, argv, force_delete, kinds);
+		return delete_branches(argc, argv, delete > 1, kinds);
 	else if (argc == 0)
 		print_ref_list(kinds, detached, verbose, abbrev);
 	else if (rename && (argc == 1))
-		rename_branch(head, argv[0], force_rename);
+		rename_branch(head, argv[0], rename > 1);
 	else if (rename && (argc == 2))
-		rename_branch(argv[0], argv[1], force_rename);
+		rename_branch(argv[0], argv[1], rename > 1);
 	else if (argc <= 2)
 		create_branch(argv[0], (argc == 2) ? argv[1] : head,
 			      force_create, reflog, track);
-- 
1.5.3.5.1598.gdef4e

^ permalink raw reply related

* [PATCH PARSEOPT 1/4] parse-options new features.
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-2-git-send-email-madcoder@debian.org>

options flags:
~~~~~~~~~~~~~
  PARSE_OPT_NONEG allow the caller to disallow the negated option to exists.

option types:
~~~~~~~~~~~~
  OPTION_BIT: ORs (or NANDs) a mask.
  OPTION_SET_INT: force the value to be set to this integer.
  OPTION_SET_PTR: force the value to be set to this pointer.

helper:
~~~~~~
  HAS_MULTI_BITS (in git-compat-util.h) is a bit-hack to check if an
  unsigned integer has more than one bit set, useful to check if conflicting
  options have been used.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 git-compat-util.h |    1 +
 parse-options.c   |   61 ++++++++++++++++++++++++++++++++++++++--------------
 parse-options.h   |   15 ++++++++++++-
 3 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 7b29d1b..f86b19f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -20,6 +20,7 @@
 #endif
 
 #define MSB(x, bits) ((x) & TYPEOF(x)(~0ULL << (sizeof(x) * 8 - (bits))))
+#define HAS_MULTI_BITS(i)  ((i) & ((i) - 1))  /* checks if an integer has more than 1 bit set */
 
 /* Approximation of the length of the decimal representation of this type. */
 #define decimal_length(x)	((int)(sizeof(x) * 2.56 + 0.5) + 1)
diff --git a/parse-options.c b/parse-options.c
index 15b32f7..d3e608a 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -40,24 +40,53 @@ static int get_value(struct optparse_t *p,
                      const struct option *opt, int flags)
 {
 	const char *s, *arg;
-	arg = p->opt ? p->opt : (p->argc > 1 ? p->argv[1] : NULL);
+	const int unset = flags & OPT_UNSET;
 
-	if (p->opt && (flags & OPT_UNSET))
+	if (unset && p->opt)
 		return opterror(opt, "takes no value", flags);
+	if (unset && (opt->flags & PARSE_OPT_NONEG))
+		return opterror(opt, "isn't available", flags);
 
-	switch (opt->type) {
-	case OPTION_BOOLEAN:
-		if (!(flags & OPT_SHORT) && p->opt)
+	if (!(flags & OPT_SHORT) && p->opt) {
+		switch (opt->type) {
+		case OPTION_CALLBACK:
+			if (!(opt->flags & PARSE_OPT_NOARG))
+				break;
+			/* FALLTHROUGH */
+		case OPTION_BOOLEAN:
+		case OPTION_BIT:
+		case OPTION_SET_INT:
+		case OPTION_SET_PTR:
 			return opterror(opt, "takes no value", flags);
-		if (flags & OPT_UNSET)
-			*(int *)opt->value = 0;
+		default:
+			break;
+		}
+	}
+
+	arg = p->opt ? p->opt : (p->argc > 1 ? p->argv[1] : NULL);
+	switch (opt->type) {
+	case OPTION_BIT:
+		if (unset)
+			*(int *)opt->value &= ~opt->defval;
 		else
-			(*(int *)opt->value)++;
+			*(int *)opt->value |= opt->defval;
+		return 0;
+
+	case OPTION_BOOLEAN:
+		*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
+		return 0;
+
+	case OPTION_SET_INT:
+		*(int *)opt->value = unset ? 0 : opt->defval;
+		return 0;
+
+	case OPTION_SET_PTR:
+		*(void **)opt->value = unset ? NULL : (void *)opt->defval;
 		return 0;
 
 	case OPTION_STRING:
-		if (flags & OPT_UNSET) {
-			*(const char **)opt->value = (const char *)NULL;
+		if (unset) {
+			*(const char **)opt->value = NULL;
 			return 0;
 		}
 		if (opt->flags & PARSE_OPT_OPTARG && (!arg || *arg == '-')) {
@@ -70,13 +99,10 @@ static int get_value(struct optparse_t *p,
 		return 0;
 
 	case OPTION_CALLBACK:
-		if (flags & OPT_UNSET)
+		if (unset)
 			return (*opt->callback)(opt, NULL, 1);
-		if (opt->flags & PARSE_OPT_NOARG) {
-			if (p->opt && !(flags & OPT_SHORT))
-				return opterror(opt, "takes no value", flags);
+		if (opt->flags & PARSE_OPT_NOARG)
 			return (*opt->callback)(opt, NULL, 0);
-		}
 		if (opt->flags & PARSE_OPT_OPTARG && (!arg || *arg == '-'))
 			return (*opt->callback)(opt, NULL, 0);
 		if (!arg)
@@ -84,7 +110,7 @@ static int get_value(struct optparse_t *p,
 		return (*opt->callback)(opt, get_arg(p), 0);
 
 	case OPTION_INTEGER:
-		if (flags & OPT_UNSET) {
+		if (unset) {
 			*(int *)opt->value = 0;
 			return 0;
 		}
@@ -292,7 +318,7 @@ void usage_with_options(const char * const *usagestr,
 					pos += fprintf(stderr, " ...");
 			}
 			break;
-		default:
+		default: /* OPTION_{BIT,BOOLEAN,SET_INT,SET_PTR} */
 			break;
 		}
 
@@ -311,6 +337,7 @@ void usage_with_options(const char * const *usagestr,
 
 /*----- some often used options -----*/
 #include "cache.h"
+
 int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset)
 {
 	int v;
diff --git a/parse-options.h b/parse-options.h
index 65bce6e..a8760ac 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -2,9 +2,15 @@
 #define PARSE_OPTIONS_H
 
 enum parse_opt_type {
+	/* special types */
 	OPTION_END,
 	OPTION_GROUP,
-	OPTION_BOOLEAN,
+	/* options with no arguments */
+	OPTION_BIT,
+	OPTION_BOOLEAN, /* _INCR would have been a better name */
+	OPTION_SET_INT,
+	OPTION_SET_PTR,
+	/* options with arguments (usually) */
 	OPTION_STRING,
 	OPTION_INTEGER,
 	OPTION_CALLBACK,
@@ -17,6 +23,7 @@ enum parse_opt_flags {
 enum parse_opt_option_flags {
 	PARSE_OPT_OPTARG  = 1,
 	PARSE_OPT_NOARG   = 2,
+	PARSE_OPT_NONEG   = 4,
 };
 
 struct option;
@@ -49,12 +56,15 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
  *   mask of parse_opt_option_flags.
  *   PARSE_OPT_OPTARG: says that the argument is optionnal (not for BOOLEANs)
  *   PARSE_OPT_NOARG: says that this option takes no argument, for CALLBACKs
+ *   PARSE_OPT_NONEG: says that this option cannot be negated
  *
  * `callback`::
  *   pointer to the callback to use for OPTION_CALLBACK.
  *
  * `defval`::
  *   default value to fill (*->value) with for PARSE_OPT_OPTARG.
+ *   OPTION_{BIT,SET_INT,SET_PTR} store the {mask,integer,pointer} to put in
+ *   the value when met.
  *   CALLBACKS can use it like they want.
  */
 struct option {
@@ -72,7 +82,10 @@ struct option {
 
 #define OPT_END()                   { OPTION_END }
 #define OPT_GROUP(h)                { OPTION_GROUP, 0, NULL, NULL, NULL, (h) }
+#define OPT_BIT(s, l, v, h, b)      { OPTION_BIT, (s), (l), (v), NULL, (h), 0, NULL, (b) }
 #define OPT_BOOLEAN(s, l, v, h)     { OPTION_BOOLEAN, (s), (l), (v), NULL, (h) }
+#define OPT_SET_INT(s, l, v, h, i)  { OPTION_SET_INT, (s), (l), (v), NULL, (h), 0, NULL, (i) }
+#define OPT_SET_PTR(s, l, v, h, p)  { OPTION_SET_PTR, (s), (l), (v), NULL, (h), 0, NULL, (p) }
 #define OPT_INTEGER(s, l, v, h)     { OPTION_INTEGER, (s), (l), (v), NULL, (h) }
 #define OPT_STRING(s, l, v, a, h)   { OPTION_STRING,  (s), (l), (v), (a), (h) }
 #define OPT_CALLBACK(s, l, v, a, h, f) \
-- 
1.5.3.5.1598.gdef4e

^ permalink raw reply related

* [PATCH MISC 1/1] Make gcc warning about empty if body go away.
From: Pierre Habouzit @ 2007-11-07 10:20 UTC (permalink / raw)
  To: gitster; +Cc: git, Pierre Habouzit
In-Reply-To: <1194430832-6224-1-git-send-email-madcoder@debian.org>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-diff.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-diff.c b/builtin-diff.c
index f77352b..80392a8 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -204,7 +204,7 @@ static void refresh_index_quietly(void)
 		if (write_cache(fd, active_cache, active_nr) ||
 		    close(fd) ||
 		    commit_locked_index(lock_file))
-			; /*
+			(void)0; /*
 			   * silently ignore it -- we haven't mucked
 			   * with the real index.
 			   */
-- 
1.5.3.5.1598.gdef4e

^ permalink raw reply related

* Re: [PATCH] Documentation: enhanced "git for CVS users" doc about shared repositories
From: David Kastrup @ 2007-11-07  9:09 UTC (permalink / raw)
  To: git
In-Reply-To: <Pine.LNX.4.64.0711070053320.4362@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Remember, those who read "git for CVS users" are _unwilling_ to
> spend the time reading git documentation (at least for the most
> part).  If they encounter something which is not useful to them,
> they will not just ignore it, they will stop reading.

People who read documentation do this only in order to avoid reading
documentation, anyway?  This must be about the most stupid argument to
refrain from augmenting documentation I have heard in a long time.

It may well be possible that some of the proposed additions don't have
a good place in the document.  I have not checked so myself.  But the
above criticism is not constructive for documentation writers since
its logical conclusion would boil down to "don't bother writing any
documentation".

-- 
David Kastrup

^ permalink raw reply

* Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.
From: David Kastrup @ 2007-11-07  9:03 UTC (permalink / raw)
  To: git
In-Reply-To: <Pine.LNX.4.64.0711062225090.4362@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Tue, 6 Nov 2007, Robin Rosenberg wrote:
>
>> tisdag 06 november 2007 skrev Mike Hommey:
>> > Maybe the documentation could emphasise on how to undo things when the
>> > user makes mistakes.
>> > Sometimes, saving your repo can be as simple as git reset --hard HEAD@{1}.
>> > This is not, unfortunately, a works-for-all-cases command.
>> 
>> Yea, git-undo(7). 
>
> In related news, I know a few users who need an un-rm-rf.  Anyone?

Most file systems don't have a reflog or other ways to recover from
shooting yourself in the foot.  git has, and for good reason.

There is no sense in hiding that facility away because of feeling
macho.  Since git already keeps the file space around needed for
recovery (and you really have to exert yourself to make it let go for
good), there is no point in not making it as convenient as feasible to
recover.

-- 
David Kastrup

^ permalink raw reply

* Re: [PATCH] Add Documentation/CodingStyle
From: Andreas Ericsson @ 2007-11-07  8:53 UTC (permalink / raw)
  To: Wincent Colaiuta
  Cc: Johannes Schindelin, Junio C Hamano, Ralf Wildenhues, git
In-Reply-To: <4749D87D-9660-472B-97CF-8E649435AFD7@wincent.com>

Wincent Colaiuta wrote:
> El 7/11/2007, a las 0:17, Johannes Schindelin escribió:
> 
>> Even if our code is quite a good documentation for our coding style,
>> some people seem to prefer a document describing it.
> 
> Great idea, Johannes, especially your nice concise summary of 
> conventions for shell-scripts (which is an area where we most often see 
> list traffic on which way to write things).
> 

That was ripped from a mail Junio sent to the list though ;-)

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

^ permalink raw reply

* Re: [PATCH] Add Documentation/CodingStyle
From: Andreas Ericsson @ 2007-11-07  8:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Ralf Wildenhues, git
In-Reply-To: <7v640ega5q.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
>> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
>> new file mode 100644
>> index 0000000..622b80b
>> --- /dev/null
>> +++ b/Documentation/CodingStyle
>> @@ -0,0 +1,87 @@
>> +As a popular project, we also have some guidelines to keep to the
>> +code.  For git in general, two rough rules are:
>> +
>> + - Most importantly, we never say "It's in POSIX; we'll happily
>> +   screw your system that does not conform."  We live in the
>> +   real world.
>> +
>> + - However, we often say "Let's stay away from that construct,
>> +   it's not even in POSIX".
>> +
> 
> I am not sure if we want to have CodingStyle document, but the
> above are not CodingStyle issues.
> 
> If we are to write this down, I'd like to have the more
> important third rule, which is:
> 
>  - In spite of the above two rules, we sometimes say "Although
>    this is not in POSIX, it (is so convenient | makes the code
>    much more readable | has other good characteristics) and
>    practically all the platforms we care about support it, so
>    let's use it".  Again, we live in the real world, and it is
>    sometimes a judgement call, decided based more on real world
>    constraints people face than what the paper standard says.
> 
>> +For C programs:
>> +
>> + - Use tabs to increment, and interpret tabs as taking up to 8 spaces
> 
> What's the character for decrement?  DEL? ;-)
> 
>> +   Double negation is often harder to understand than no negation at
>> +   all.
>> +
>> +   Some clever tricks, like using the !! operator with arithmetic
>> +   constructs, can be extremely confusing to others.  Avoid them,
>> +   unless there is a compelling reason to use them.
> 
> I actually think (!!var) idiom is already established in our
> codebase.
> 

Not very widely for arithmetic expressions though. I believe this
alludes to the (!!a + !!b + !!c) idiom used earlier, where it's not
exactly clear *why* a, b and c can be > 1 if != 0 is the only thing
we care about.

>> + - Use the API.  No, really.  We have a strbuf (variable length string),
>> +   several arrays with the ALLOC_GROW() macro, a path_list for sorted
>> +   string lists, a hash map (mapping struct objects) named
>> +   "struct decorate", amongst other things.
> 
>  - When you come up with an API, document it.
> 
>> + - if you are planning a new command, consider writing it in shell or
>> +   perl first, so that changes in semantics can be easily changed and
>> +   discussed.  Many git commands started out like that, and a few are
>> +   still scripts.
> 
> No Python allowed?


Perhaps with this addendum?

- Think very, very hard before introducing a new dependency into git. This
  means you should stay away from scripting languages not already used in
  the git core command set unless your command is clearly separate from it,
  such as an importer to convert random-scm-X repositories to git.

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

^ permalink raw reply

* Re: [PATCH] Documentation: enhanced "git for CVS users" doc about shared repositories
From: Andreas Ericsson @ 2007-11-07  8:45 UTC (permalink / raw)
  To: Steffen Prohaska
  Cc: Aghiles, Johannes Schindelin, Junio C Hamano, Francesco Pretto,
	Git Mailing List
In-Reply-To: <DED2A61B-B5AE-4BAA-942A-18A61924611E@zib.de>

Steffen Prohaska wrote:
>  BTW, useradd and such
> would not help me at all, because I need to talk to my admin
> anyway and he adds an account to the LDAP database.
> 

I suspect this is the case for most companies looking to switch
to git. It certainly is here. Personally, I tend to find documents
stooping to toddler-level a bit insulting unless they're marked
as "walkthrough" or "for beginners" or some such. Especially if
they are obviously not correct for all environments.

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

^ permalink raw reply

* Re: [PATCH] Mark 'git stash [message...]' as deprecated
From: Pierre Habouzit @ 2007-11-07  8:23 UTC (permalink / raw)
  To: Brian Downing; +Cc: Junio C Hamano, tsuna, aghilesk, git
In-Reply-To: <1194395205-27905-1-git-send-email-bdowning@lavos.net>

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

On Wed, Nov 07, 2007 at 12:26:44AM +0000, Brian Downing wrote:
> Complain to STDERR unless 'git stash save' is explicitly used.
> This is in preparation for completely disabling the "default save"
> behavior of the command in the future.

  No arguments at all should not IMHO be deprecated, it's very useful,
and is not ambiguous. The issue with git stash <random> is that if you
thought you typed a command that doesn't in fact exists, you stash which
is not what you meant _at all_.

  When you type `git stash` you certainly want to stash, and it's what
it does.

Here is how it should work:

git-stash (list | show [<stash>] | apply [<stash>] | clear)
git-stash [save <message>]


-- 
·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] git-revert is one of the most misunderstood command in git, help users out.
From: Mike Hommey @ 2007-11-07  8:16 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Robin Rosenberg, Junio C Hamano, Steven Grimm, Pierre Habouzit,
	git
In-Reply-To: <Pine.LNX.4.64.0711062225090.4362@racer.site>

On Tue, Nov 06, 2007 at 10:25:48PM +0000, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
> 
> On Tue, 6 Nov 2007, Robin Rosenberg wrote:
> 
> > tisdag 06 november 2007 skrev Mike Hommey:
> > > Maybe the documentation could emphasise on how to undo things when the
> > > user makes mistakes.
> > > Sometimes, saving your repo can be as simple as git reset --hard HEAD@{1}.
> > > This is not, unfortunately, a works-for-all-cases command.
> > 
> > Yea, git-undo(7). 
> 
> In related news, I know a few users who need an un-rm-rf.  Anyone?

The fact is you can do harm to your repo with things you wouldn't expect to
break things, except maybe you gave bad arguments or so. It's quite easy to
fuck up with git-rebase, or to merge the wrong commits, etc.

Mike

^ permalink raw reply

* Re: [PATCH] git-fetch: avoid local fetching from alternate (again)
From: Johannes Sixt @ 2007-11-07  8:12 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20071107024118.GA11043@spearce.org>

Shawn O. Pearce schrieb:
> +	err = start_command(&revlist);
> +	if (!err)
> +		err |= finish_command(&revlist);

There's a short-hand:

	err = run_command(&revlist);

-- Hannes

^ permalink raw reply

* Re: [PATCH] Mark 'git stash [message...]' as deprecated
From: Wincent Colaiuta @ 2007-11-07  8:12 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Brian Downing, Junio C Hamano, tsuna, aghilesk, git
In-Reply-To: <473170AB.7030104@viscovery.net>

El 7/11/2007, a las 9:00, Johannes Sixt escribió:

> Brian Downing schrieb:
>> Complain to STDERR unless 'git stash save' is explicitly used.
>> This is in preparation for completely disabling the "default save"
>> behavior of the command in the future.
>> ...
>> -'git-stash' [save] [message...]
>> +'git-stash' save [message...]
>
> Can't we have these two?
>
> 	git-stash
> 	git-stash save [message...]
>
> 'git stash' without a message as an equivalent of 'git stash save'  
> is still very handy.

Agreed.

Wincent

^ permalink raw reply

* Re: [PATCH] Documentation: enhanced "git for CVS users" doc about shared repositories
From: Steffen Prohaska @ 2007-11-07  8:07 UTC (permalink / raw)
  To: Francesco Pretto; +Cc: Junio C Hamano, git
In-Reply-To: <47310ACF.4030103@gmail.com>

On Nov 7, 2007, at 1:46 AM, Francesco Pretto wrote:

> Junio C Hamano ha scritto:
>>
>> Honestly speaking, I am not too thrilled about making the
>> cvs-migration document much longer than what it currently is.
>>

Maybe the description of setting up a shared repository should
go to the user-manual and cvs-migration should refer to the
user-manual, instead of the other way round. I don't like the
idea that the user-manual is referring to a CVS specific guide.
The user manual should be as self-contained as possible.


> Honestly speaking, you've spent too much time in looking for every  
> possible
> objections against these simple additions. At least it should be  
> less than the
> time I've spent in measuring every single word of this patch,  
> hoping you could
> consider them for inclusion. You gave me lot of attentions (I am  
> grateful of this,
> really) so I should probably be surprised of the cleanliness of git  
> code, of the
> rigor of the code style, of the clarity of the documentation. But  
> unfortunately,
> I am not. I simply tried to make this document more useful and  
> helpful for a
> wider audience of people that could ever consider of using git in  
> their life.
> And yes, I decided to so because I had trouble myself during  
> initial configurations.
> What's the problem if a document called "git for CVS users" is more  
> explicated?
> What's the problem if it contains as many as possible informations  
> to set up
> git in a viable way and, hopefully, to learn something on how it  
> does work?
>
> I'm sad. Not only because you refused a documentation patch, but  
> because i could
> have sent a "Bug: Documentation Sucks!" to the ml and i would have  
> obtained the
> same thing: nothing.

Don't be unfair. Junio made clear that the documentation
should not be cluttered with an introduction to Unix commands.
But at least two points (git-shell, git-init.txt) would be
accepted if you sent an cleaned-up patch.

I have no good idea how to reconcile your idea of giving more
guidance to Unix commands with the idea of having a concise
document that assumes a reader with decent Unix knowledge

Maybe you could just add some references to the distribution
specific information, or just refer to the man pages.

Maybe you could move the introductory comments to a FAQ-like
appendix. It could give brief hints on
"How to set up a user account?",
"How to setup a world-writable directory?"
I'm a bit reluctant to this because we'd need to maintain such
information. But I suspect that some users would find them
helpful.

One last comment: discussing patches is how the world works on
the git mailing list. It happend to me, too, that patches were
rejected after a brief or after a lengthy discussion. So, yes,
finally sometimes there is no change. But often the discussions
reveal a better way of achieving the original goal. Nonetheless,
it can be frustrating to the original author.

Thanks for you effort of improving the documentation.

	Steffen

^ permalink raw reply

* Re: [PATCH] Documentation: enhanced "git for CVS users" doc about shared repositories
From: Wincent Colaiuta @ 2007-11-07  8:03 UTC (permalink / raw)
  To: Francesco Pretto; +Cc: Junio C Hamano, git
In-Reply-To: <47310ACF.4030103@gmail.com>

El 7/11/2007, a las 1:46, Francesco Pretto escribió:

> Junio C Hamano ha scritto:
>>
>> Honestly speaking, I am not too thrilled about making the
>> cvs-migration document much longer than what it currently is.
>>
>
> Honestly speaking, you've spent too much time in looking for every  
> possible
> objections against these simple additions. At least it should be  
> less than the
> time I've spent in measuring every single word of this patch, hoping  
> you could
> consider them for inclusion. You gave me lot of attentions (I am  
> grateful of this,
> really) so I should probably be surprised of the cleanliness of git  
> code, of the
> rigor of the code style, of the clarity of the documentation. But  
> unfortunately,
> I am not. I simply tried to make this document more useful and  
> helpful for a
> wider audience of people that could ever consider of using git in  
> their life.
> And yes, I decided to so because I had trouble myself during initial  
> configurations.
> What's the problem if a document called "git for CVS users" is more  
> explicated?
> What's the problem if it contains as many as possible informations  
> to set up
> git in a viable way and, hopefully, to learn something on how it  
> does work?
>
> I'm sad. Not only because you refused a documentation patch, but  
> because i could
> have sent a "Bug: Documentation Sucks!" to the ml and i would have  
> obtained the
> same thing: nothing.

On the contrary, I think you received some excellent, high-quality  
feedback. The process that you've been participating in over the last  
few days is exactly why the Git codebase is as good as it is; only the  
very best patches get accepted, and those which aren't "the very best"  
receive detailed feedback that help the submitter to turn weak patches  
into strong ones. The process works very well and the proof is in the  
pudding (the quality of the product).

Cheers,
Wincent

^ permalink raw reply

* Re: [PATCH] Mark 'git stash [message...]' as deprecated
From: Junio C Hamano @ 2007-11-07  8:02 UTC (permalink / raw)
  To: Brian Downing; +Cc: tsuna, aghilesk, git
In-Reply-To: <1194395205-27905-1-git-send-email-bdowning@lavos.net>

Brian Downing <bdowning@lavos.net> writes:

> Complain to STDERR unless 'git stash save' is explicitly used.
> This is in preparation for completely disabling the "default save"
> behavior of the command in the future.

Ok, but I would prefer to see this made into at least a
three-step process to ease the migration on users.  I do not
have any issue with a deprecation warning before the next big
release (1.5.4?).

The next step after this patch should not be the removal of
"defalut save".  Instead, introduce a boolean configuration,
stash.defaultsave, that defaults to false.  Without the
configuration, disable the "default save" (and do not even
mention the configuration variable, but do give the usage
message listing the commands).  But allow people to use the
"default save" behaviour with the configuration to help existing
users.  You can do this in the same release as above if you
want.

Then you would finally drop the "default save" in the next
big release after that "deprecation release".  But not before
that.

BTW, I've been quietly rewriting git-stash in C.  Be warned ;-)

^ permalink raw reply

* Re: [PATCH] Mark 'git stash [message...]' as deprecated
From: Johannes Sixt @ 2007-11-07  8:00 UTC (permalink / raw)
  To: Brian Downing; +Cc: Junio C Hamano, tsuna, aghilesk, git
In-Reply-To: <1194395205-27905-1-git-send-email-bdowning@lavos.net>

Brian Downing schrieb:
> Complain to STDERR unless 'git stash save' is explicitly used.
> This is in preparation for completely disabling the "default save"
> behavior of the command in the future.
> 
> ...
> -'git-stash' [save] [message...]
> +'git-stash' save [message...]

Can't we have these two?

	git-stash
	git-stash save [message...]

'git stash' without a message as an equivalent of 'git stash save' is still 
very handy.

-- Hannes

^ permalink raw reply

* Re: [PATCH] Add Documentation/CodingStyle
From: Wincent Colaiuta @ 2007-11-07  7:53 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Ralf Wildenhues, git
In-Reply-To: <Pine.LNX.4.64.0711062317330.4362@racer.site>

El 7/11/2007, a las 0:17, Johannes Schindelin escribió:

> Even if our code is quite a good documentation for our coding style,
> some people seem to prefer a document describing it.

Great idea, Johannes, especially your nice concise summary of  
conventions for shell-scripts (which is an area where we most often  
see list traffic on which way to write things).

Cheers,
Wincent

^ permalink raw reply

* Re: [PATCH] git-fetch: avoid local fetching from alternate (again)
From: Junio C Hamano @ 2007-11-07  7:45 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <7vsl3iefoj.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
>> Back in e3c6f240fd9c5bdeb33f2d47adc859f37935e2df Junio taught
>> git-fetch to avoid copying objects when we are fetching from
>> a repository that is already registered as an alternate object
>> database.  In such a case there is no reason to copy any objects
>> as we can already obtain them through the alternate.
>
> Well spotted.  It would be a good idea to commit the big comment
> from contrib/examples/git-fetch.sh to fetch_local_nocopy()
> function, which would have made us realize that the patch does
> not refrain from applying this optimization even when shallow
> is in effect.  But I think that is actually a good change.

Having thought about this further by writing that comment myself
(patch attached), I suspect that the test at the beginning of
the function to see if we are talking to another local
repository is not necessary.  Even if we are _not_ fetching from
the remote, we could have all the necessary objects connected,
albeit the chance of that is rather slim.  But that means the
rev-list test will error out rather quickly because objects near
the tips are more likely to be missing, and if we are talking
about a true remote connection, networking cost will most likely
outweigh the cost to do this checking.

---

 transport.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/transport.c b/transport.c
index 0604dc6..a887491 100644
--- a/transport.c
+++ b/transport.c
@@ -614,6 +614,20 @@ static struct ref *get_refs_via_connect(const struct transport *transport)
 	return refs;
 }
 
+/*
+ * We would want to bypass the object transfer altogether if
+ * everything we are going to fetch already exists and connected
+ * locally.
+ *
+ * The refs we are going to fetch are in to_fetch (nr_heads in
+ * total).  If running
+ *
+ *  $ git-rev-list --objects to_fetch[0] to_fetch[1] ... --not --all
+ *
+ * does not error out, that means everything reachable from the
+ * refs we are going to fetch exists and is connected to some of
+ * our existing refs.
+ */
 static int fetch_local_nocopy(struct transport *transport,
 			       int nr_heads, struct ref **to_fetch)
 {

^ permalink raw reply related

* Re: git pull opinion
From: Pascal Obry @ 2007-11-07  7:40 UTC (permalink / raw)
  To: Uwe Kleine-König, Pascal Obry, Aghiles, git
In-Reply-To: <20071107070646.GA3417@informatik.uni-freiburg.de>

Uwe Kleine-König a écrit :
> Hello,
> 
>> I'm using:
>>
>> $ git config --global alias.update '!git stash && git pull && git stash apply'
> I wonder how this works, if the merge produces conflicts...

If you have conflicts it will not do the "git stash apply" as git pull
will return with an error. So you'll need to fix the conflicts and do
you the final git stash manually.

Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595

^ permalink raw reply

* Re: [PATCH] Documentation: enhanced "git for CVS users" doc about shared repositories
From: Steffen Prohaska @ 2007-11-07  7:35 UTC (permalink / raw)
  To: Aghiles
  Cc: Johannes Schindelin, Junio C Hamano, Francesco Pretto,
	Git Mailing List
In-Reply-To: <3abd05a90711061736r4c969cddj348c006615ffbdd6@mail.gmail.com>


On Nov 7, 2007, at 2:36 AM, Aghiles wrote:

> Hello,
>>
>> Remember, those who read "git for CVS users" are _unwilling_ to  
>> spend the
>> time reading git documentation (at least for the most part).  If they
>> encounter something which is not useful to them, they will not  
>> just ignore
>> it, they will stop reading.
>>
>
> I must disagree with this analysis (although I didn't read the  
> content of the
> patch you are commenting). People that are not really interested in  
> git will
> find many reasons to stop reading the manual anyway. Those who really
> want to migrate (such as we did) are looking for every tiny bit of  
> information.
> (We googled git commands and error messages because we didn't
> find what we needed in the docs)

Could you be a bit more specific? What are the most important
points that you did not found in the documentation?

It would be interesting if you could share some details. Sending
patches that would fix the deficiencies would even be
superior ;)


> The docs are not perfect and somewhat unequal in content but I prefer
> more information than less, at this particular stage of git  
> development.

I agree if it is git specific information. But, personally,
I'd get a bit annoyed if a git specific document tried to
teach me how to manage Unix accounts. BTW, useradd and such
would not help me at all, because I need to talk to my admin
anyway and he adds an account to the LDAP database.

	Steffen

^ permalink raw reply

* Re: git pull opinion
From: Uwe Kleine-König @ 2007-11-07  7:06 UTC (permalink / raw)
  To: Pascal Obry; +Cc: Aghiles, git
In-Reply-To: <4730AD48.2050907@obry.net>

Hello,

> I'm using:
> 
> $ git config --global alias.update '!git stash && git pull && git stash apply'
I wonder how this works, if the merge produces conflicts...

Best regards
Uwe

-- 
Uwe Kleine-König

http://www.google.com/search?q=1+year+in+days

^ 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