Git development
 help / color / mirror / Atom feed
* What's in git.git (stable frozen)
From: Junio C Hamano @ 2007-12-13  2:47 UTC (permalink / raw)
  To: git
In-Reply-To: <7vve78qhtf.fsf@gitster.siamese.dyndns.org>

The tip of 'master' is now tagged as 1.5.4-rc0.  Tarballs are found in
the usual places:

  http://www.kernel.org/pub/software/scm/git/

  git-1.5.4.rc0.tar.{gz,bz2}			(tarball)
  git-htmldocs-1.5.4.rc0.tar.{gz,bz2}		(preformatted docs)
  git-manpages-1.5.4.rc0.tar.{gz,bz2}		(preformatted docs)

I've also built a set of preview RPM packages.  They are found in:

  http://www.kernel.org/pub/software/scm/git/testing/

I am not an RPM person.  It would be really appreciated if experts help
out finding and fixing any bug in the packaging.  I do not want to
repeat the firefighting we needed with 1.5.3.1.

----------------------------------------------------------------
* The 'master' branch has these since the last announcement.

Alex Riesen (1):
  Fix git-fast-export for zero-sized blobs

Alexandre Julliard (1):
  git.el: Added a menu for git-status-mode.

Charles Bailey (1):
  Fix clone not to ignore depth when performing a local clone

Christian Couder (5):
  git-help: add -i|--info option to display info page.
  Documentation: describe -i/--info option to "git-help"
  git-help: add -w|--web option to display html man page in a browser.
  Use {web,instaweb,help}.browser config options.
  Documentation: describe -w/--web option to "git-help".

Daniel Barkalow (1):
  Add more checkout tests

Eric Wong (2):
  git-svn: replace .rev_db with a more space-efficient .rev_map format
  git-svn: reinstate old rev_db optimization in new rev_map

Eyvind Bernhardsen (1):
  Fix mis-markup of the -p, --patch option in git-add(1)

Gerrit Pape (1):
  Don't cache DESTDIR in perl/perl.mak.

Jakub Narebski (1):
  autoconf: Check asciidoc version to automatically set ASCIIDOC8

Jeff King (6):
  don't mention index refreshing side effect in git-status docs
  Add git-browse-help to .gitignore
  Support GIT_PAGER_IN_USE environment variable
  git-svn: get color config from --get-colorbool
  shortlog: document -e option
  git-clone: print an error message when trying to clone empty repo

Johannes Sixt (1):
  Fix a typo in checkout.sh and cleanup one-line help messages

Junio C Hamano (18):
  git-shortlog -e: show e-mail address as well
  Re-fix ls-remote
  Update draft Release Notes for 1.5.4
  Documentation: add gitman.info target
  git-help -i: invoke info with document and node name
  git-help -i: show info documentation from matching version of git
  send-email: do not muck with initial-reply-to when unset.
  commit: do not add extra LF at the end of the summary.
  Support a merge with conflicting gitlink change
  blame: drop blob data after passing blame to the parent
  shortlog: default to HEAD when the standard input is a tty
  git-diff --numstat -z: make it machine readable
  disable t9119 for now.
  Documentation: rename git.texi to user-manual.texi
  git.spec.in: remove python_path
  git-help -w: do not require to be in git repository
  RPM spec: Adjust htmldir
  GIT 1.5.4-rc0

Linus Torvalds (1):
  "git tag -u keyname" broken

Mike Hommey (4):
  git-send-email.perl: Really add angle brackets to In-Reply-To if
    necessary
  Cleanup variables in http.[ch]
  Fix small memory leaks induced by diff_tree_setup_paths
  Fix XML parser leaks in http-push

Nguyễn Thái Ngọc Duy (1):
  Remove repo version check from setup_git_directory

Nicolas Pitre (1):
  pack-objects: more threaded load balancing fix with often changed paths

Pierre Habouzit (1):
  Invert numbers and names in the git-shortlog summary mode.

Wincent Colaiuta (5):
  Interactive editor tests for commit-msg hook
  Style fixes for pre-commit hook tests
  Use "whitespace" consistently
  Documentation: minor grammar fix for "git apply"
  Fix "diff --check" whitespace detection

^ permalink raw reply

* Re: [egit] How-to use egit
From: Jing Xue @ 2007-12-13  2:47 UTC (permalink / raw)
  To: Wink Saville; +Cc: git
In-Reply-To: <476032A2.9010308@saville.com>

On Wed, Dec 12, 2007 at 11:12:34AM -0800, Wink Saville wrote:
> Robin et. al,
>
>
> I cloned egit from git://repo.or.cz/egit and imported it into my workspace
> following the instructions in INSTALL I did successfully compile although
> there were 1115 warnings but no compilation errors.
>
> I then exported to a archive org.spearce.egit.jar and then copied this file
> to /usr/eclipse/plugin/ I then restarted Eclipse. But I don't see any 
> evidence

I have five jars:

/opt/eclipse/plugins$ ll org.spearce*
-rw-r----- 1 jingxue jingxue 2.5K 2007-12-01 20:45 org.spearce.egit_0.3.0.200712020145.jar
-rw-r----- 1 jingxue jingxue  70K 2007-12-01 20:45 org.spearce.egit.core_0.3.0.200712020145.jar
-rw-r----- 1 jingxue jingxue  14K 2007-12-01 20:45 org.spearce.egit.core.test_0.3.0.200712020145.jar
-rw-r----- 1 jingxue jingxue 107K 2007-12-01 20:45 org.spearce.egit.ui_0.3.0.200712020145.jar
-rw-r----- 1 jingxue jingxue 223K 2007-12-01 20:45 org.spearce.jgit_0.3.0.200712020145.jar

Did you select all the org.spearce projects before exporting?

-- 
Jing Xue

^ permalink raw reply

* What's cooking in git.git (topics)
From: Junio C Hamano @ 2007-12-13  2:48 UTC (permalink / raw)
  To: git
In-Reply-To: <7v7ijorwnc.fsf@gitster.siamese.dyndns.org>

Here are the topics that have been cooking.  Commits prefixed
with '-' are only in 'pu' while commits prefixed with '+' are
in 'next'.  Others commits may be stashed in 'offcuts'.

The topics list the commits in reverse chronological order.

----------------------------------------------------------------
[New Topics]

* jc/git-symref (Tue Dec 11 16:42:46 2007 -0800) 1 commit
 . PARK: show-symref protocol extension.

This is a demonstration of a possible component in the future direction
for HEAD discovery done by git-clone.

----------------------------------------------------------------
[Graduated to 'master']

* jc/merge-recursive-gitlink (Mon Dec 10 11:22:05 2007 -0800) 1 commit
* ew/svn-rev-db (Sat Dec 8 23:27:42 2007 -0800) 2 commits
* jk/svn-color (Tue Dec 11 01:28:42 2007 -0500) 2 commits

These got success stories and Acks.  Thanks for testing.

* cc/help (Wed Dec 12 14:00:24 2007 -0800) 13 commits

I've updated RPM git.spec.in minimally to adjust to the locations things
are installed, and also made "browse-help" usable outside git repository.

* jc/shortlog-e (Tue Dec 11 10:09:04 2007 -0800) 4 commits

Ingo's wish resulted in reversal of numbers and names in short-summary
output, which I think is much saner than the original one, and more
importantly, a saner default behaviour when the user does not give
sufficient parameters to it.

----------------------------------------------------------------
[Will cook further in 'next' and then merge to 'master' soon]

We are at 1.5.4-rc0.  There is not much to see here ;-)

----------------------------------------------------------------
[Actively cooking]

* jc/api-doc (Sat Nov 24 23:48:04 2007 -0800) 1 commit
 - Start preparing the API documents.

The primary reason of this series is because I think we made the system
a lot less approachable by losing hackability.  Although we still have
sample scripts in contrib/example for use of plumbing in scripts, they
will not help aspiring git-hacker-wannabees when our primary attention
has already shifted to moving things to C.

This currently consists of mostly stubs, although I wrote about a few
topics as examples.  Nice to have in v1.5.4, but we need more writers.

----------------------------------------------------------------
[On hold]

* nd/dashless (Wed Nov 28 23:21:57 2007 +0700) 1 commit
 - Move all dashed-form commands to libexecdir

I think this is a sane thing to do in the longer term.  Will be in
'next' after v1.5.4.  I think "leave porcelain on PATH" might be also a
good thing as a transition measure.

Incidentally, if we do not install dashed form of built-ins anywhere
(which is not this series is about --- this is just moving them out of
user's PATH), "git help -a" will stop showing them.  I am not enthused
about removing the hardlinks to built-ins to begin with, but people who
want such a change need to first modify help.c:list_commands() to pick
up builtins without having git-foo hardlinks in gitexecdir.  This may
need to happen anyway as mingw fallouts.

* js/remote (Wed Dec 5 19:02:15 2007 +0000) 4 commits
 - Make git-remote a builtin
 - Test "git remote show" and "git remote prune"
 - parseopt: add flag to stop on first non option
 - path-list: add functions to work with unsorted lists

This and Kristian's "git-clone in C" are on hold and post 1.5.4.

----------------------------------------------------------------
[Stalled]

* ns/checkout-push-pop (Wed Dec 5 07:04:06 2007 +0900) 1 commit
 - git-checkout --push/--pop

A reasonably cleanly written cute hack, and I do not see this breaking
the normal codepath.  But I tend to agree with people that 'push' is
too late for forgetful mortals, and just a single "previous" would be
easier to use.

* mh/http (Tue Dec 11 00:08:25 2007 +0100) 6 commits
 - Move fetch_ref from http-push.c and http-walker.c to http.c
 - Fix various memory leaks in http-push.c and http-walker.c
 - Use strbuf in http code
 + Avoid redundant declaration of missing_target()
 + Remove a CURLOPT_HTTPHEADER (un)setting
 + Remove the default_headers variable from http-push.c

* js/reflog-delete (Wed Oct 17 02:50:45 2007 +0100) 1 commit
 + Teach "git reflog" a subcommand to delete single entries

* jk/builtin-alias (Fri Nov 30 11:22:58 2007 -0500) 1 commit
 + Support builtin aliases

Even the original author has slight NAK on this and I tend to agree.
May want to eventurally revert from 'next' but we are not in a hurry
even to do that.

* jc/nu (Sun Oct 14 22:07:34 2007 -0700) 3 commits
 - merge-nu: a new merge backend without using unpack_trees()
 - read_tree: take an explicit index structure
 - gcc 4.2.1 -Werror -Wall -ansi -pedantic -std=c99: minimum fix

* jc/diff-pathspec (Sun Nov 25 10:03:48 2007 -0800) 1 commit
 - Making ce_path_match() more useful by accepting globs

This was to allow "git diff-files -- '*.h'" (currently diff family
knows only the leading directory match and not fileglobs), but was shot
down by Alex.  I tend to agree with him.

* jc/diff-relative (Thu Dec 6 09:48:32 2007 -0800) 1 commit
 - Make "diff" Porcelain output paths as relative to subdirectory.

* jc/cherry-pick (Tue Nov 13 12:38:51 2007 -0800) 1 commit
 . revert/cherry-pick: start refactoring call to merge_recursive

* jc/pathspec (Thu Sep 13 13:38:19 2007 -0700) 3 commits
 . pathspec_can_match(): move it from builtin-ls-tree.c to tree.c
 . ls-tree.c: refactor show_recursive() and rename it.
 . tree-diff.c: split out a function to match a single pattern.

* jc/dashless (Sat Dec 1 22:09:22 2007 -0800) 2 commits
 . Prepare execv_git_cmd() for removal of builtins from the
   filesystem
 . git-shell: accept "git foo" form

We do not plan to remove git-foo form completely from the filesystem at
this point, so these are not strictly necessary.

^ permalink raw reply

* [PATCH] git-commit: squelch needless message during an empty merge
From: Junio C Hamano @ 2007-12-13  3:09 UTC (permalink / raw)
  To: git; +Cc: Kristian Høgsberg
In-Reply-To: <7vbq8v5n0u.fsf_-_@gitster.siamese.dyndns.org>

When recording a merge that conflicted and ends up in no changes after
manual resolution, commit callchain looked like this:

	cmd_commit() ->
            prepare_log_message() ->
                run_status() ->
		    wt_status_print()

This invocation of run_status() is asked to find out if there is a
committable change, but it unconditionally gave instructions such as
"use git-add" at the same time.  When in merge, we do allow an empty
change to be recorded, so after showing this message the code still went
ahead and made a commit.

This introduces "nowarn" parameter to run_status() to avoid these
useless messages.  If we are not allowed to create an empty commit, we
already call run_status() again in the original codepath, and the
message will be shown from that call anyway.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-commit.c |    9 +++++----
 wt-status.c      |    2 ++
 wt-status.h      |    1 +
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 9cb7589..ad9f921 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -280,7 +280,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
 	return false_lock.filename;
 }
 
-static int run_status(FILE *fp, const char *index_file, const char *prefix)
+static int run_status(FILE *fp, const char *index_file, const char *prefix, int nowarn)
 {
 	struct wt_status s;
 
@@ -296,6 +296,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix)
 	s.untracked = untracked_files;
 	s.index_file = index_file;
 	s.fp = fp;
+	s.nowarn = nowarn;
 
 	wt_status_print(&s);
 
@@ -412,7 +413,7 @@ static int prepare_log_message(const char *index_file, const char *prefix)
 
 	saved_color_setting = wt_status_use_color;
 	wt_status_use_color = 0;
-	commitable = run_status(fp, index_file, prefix);
+	commitable = run_status(fp, index_file, prefix, 1);
 	wt_status_use_color = saved_color_setting;
 
 	fclose(fp);
@@ -606,7 +607,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	index_file = prepare_index(argc, argv, prefix);
 
-	commitable = run_status(stdout, index_file, prefix);
+	commitable = run_status(stdout, index_file, prefix, 0);
 
 	rollback_index_files();
 
@@ -717,7 +718,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	if (!prepare_log_message(index_file, prefix) && !in_merge &&
 	    !allow_empty && !(amend && is_a_merge(head_sha1))) {
-		run_status(stdout, index_file, prefix);
+		run_status(stdout, index_file, prefix, 0);
 		rollback_index_files();
 		unlink(commit_editmsg);
 		return 1;
diff --git a/wt-status.c b/wt-status.c
index 51c1879..c0c2472 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -381,6 +381,8 @@ void wt_status_print(struct wt_status *s)
 	if (!s->commitable) {
 		if (s->amend)
 			fprintf(s->fp, "# No changes\n");
+		else if (s->nowarn)
+			; /* nothing */
 		else if (s->workdir_dirty)
 			printf("no changes added to commit (use \"git add\" and/or \"git commit -a\")\n");
 		else if (s->workdir_untracked)
diff --git a/wt-status.h b/wt-status.h
index 63d50f2..02afaa6 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -17,6 +17,7 @@ struct wt_status {
 	int verbose;
 	int amend;
 	int untracked;
+	int nowarn;
 	/* These are computed during processing of the individual sections */
 	int commitable;
 	int workdir_dirty;

^ permalink raw reply related

* Re: What's cooking in git.git (topics)
From: Nicolas Pitre @ 2007-12-13  3:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vabof5mze.fsf@gitster.siamese.dyndns.org>

On Wed, 12 Dec 2007, Junio C Hamano wrote:

> Here are the topics that have been cooking.

What about the blame speedup patch from Linus (Message-ID: 
<alpine.LFD.0.9999.0712111548200.25032@woody.linux-foundation.org>)


Nicolas

^ permalink raw reply

* Re: git-cvsexportcommit fails for huge commits
From: Jeff King @ 2007-12-13  4:17 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Markus Klinik, git
In-Reply-To: <46a038f90712121158n674a9044t75ef99473314457c@mail.gmail.com>

On Thu, Dec 13, 2007 at 08:58:33AM +1300, Martin Langhoff wrote:

> IOWs, the protocol *is* atomic, and this patch does make things
> slightly more brittle. Perhaps require an option to be set before we
> do this?

I started writing a patch to let the user specify the limit, but it was
just too ugly. What user knows the right limit? We are probably better
off just setting the limit at something high and reasonable (like 64K --
it would be nice to get feedback from Markus on what platform he is
using and what is a reasonable value), or just using a tempfile with
xargs, which should figure out the correct value.

-Peff

^ permalink raw reply

* Re: [PATCH] git-commit: squelch needless message during an empty merge
From: Jeff King @ 2007-12-13  4:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Kristian Høgsberg
In-Reply-To: <7v1w9r5m03.fsf_-_@gitster.siamese.dyndns.org>

On Wed, Dec 12, 2007 at 07:09:16PM -0800, Junio C Hamano wrote:

> This introduces "nowarn" parameter to run_status() to avoid these
> useless messages.  If we are not allowed to create an empty commit, we
> already call run_status() again in the original codepath, and the
> message will be shown from that call anyway.

Looks good to me.

Acked-by: Jeff King <peff@peff.net>

-Peff

^ permalink raw reply

* [PATCH resending] Fix spelling mistakes in user manual
From: Shawn Bohrer @ 2007-12-13  4:36 UTC (permalink / raw)
  To: gitster; +Cc: git, Shawn Bohrer

Signed-off-by: Shawn Bohrer <shawn.bohrer@gmail.com>
---

Not sure if this got lost, but I haven't see it applied and figured I
should resend.

 Documentation/user-manual.txt |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 93a47b4..f2b4206 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -1994,7 +1994,7 @@ $ git push ssh://yourserver.com/~you/proj.git +master
 -------------------------------------------------
 
 Normally whenever a branch head in a public repository is modified, it
-is modified to point to a descendent of the commit that it pointed to
+is modified to point to a descendant of the commit that it pointed to
 before.  By forcing a push in this situation, you break that convention.
 (See <<problems-with-rewriting-history>>.)
 
@@ -2921,7 +2921,7 @@ As you can see, a commit is defined by:
 - a tree: The SHA1 name of a tree object (as defined below), representing
   the contents of a directory at a certain point in time.
 - parent(s): The SHA1 name of some number of commits which represent the
-  immediately prevoius step(s) in the history of the project.  The
+  immediately previous step(s) in the history of the project.  The
   example above has one parent; merge commits may have more than
   one.  A commit with no parents is called a "root" commit, and
   represents the initial revision of a project.  Each project must have
@@ -3242,7 +3242,7 @@ to replace them by hand.  Back up your repository before attempting this
 in case you corrupt things even more in the process.
 
 We'll assume that the problem is a single missing or corrupted blob,
-which is sometimes a solveable problem.  (Recovering missing trees and
+which is sometimes a solvable problem.  (Recovering missing trees and
 especially commits is *much* harder).
 
 Before starting, verify that there is corruption, and figure out where
-- 
1.5.3.6

^ permalink raw reply related

* git describe candidate filtering
From: Eric Blake @ 2007-12-13  5:25 UTC (permalink / raw)
  To: git

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Would it be possible to add an option to git-describe that limits the
candidate tags to those matching a glob pattern or regular expression?
I've got a repository imported from CVS which has a number of less
interesting tags for events such as branch creation, but I'm only
interested in describing the current commit relative to tags matching
v[0-9]\.[0-9]*, rather than the closest tag of any spelling.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9@byu.net
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHYMJQ84KuGfSFAYARAsd8AKCwuCZsxZBcV3nd849Saxvch/OZiACfagTe
mitsZEVhXy7hyBg0KALQG0o=
=Qq8n
-----END PGP SIGNATURE-----

^ permalink raw reply

* [RFH] convert shortlog to use parse_options
From: Jeff King @ 2007-12-13  5:52 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git

Pierre,

I am converting shortlog to use parse_options, but I have hit a behavior
I can't seem to represent.

The "-w" option has an optional argument, and we used to accept only
"-w" or "-wX,Y,Z". However, parse_options allows "-w X,Y,Z".

This means that "-w HEAD" used to mean "turn on wrapping
with default parameters, look at HEAD" and now translates to
"-w with parameter HEAD".

Is there a way to do this currently, or can we add an option flag?

---
 builtin-shortlog.c |  111 +++++++++++++++++++++++++++++-----------------------
 1 files changed, 62 insertions(+), 49 deletions(-)

diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index 3d8d709..37fdb45 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -6,9 +6,12 @@
 #include "revision.h"
 #include "utf8.h"
 #include "mailmap.h"
+#include "parse-options.h"
 
-static const char shortlog_usage[] =
-"git-shortlog [-n] [-s] [<commit-id>... ]";
+static const char * const shortlog_usage[] = {
+	"git-shortlog [-n] [-s] [-e] [<commit-id>... ]",
+	NULL
+};
 
 static char *common_repo_prefix;
 static int email;
@@ -181,34 +184,45 @@ static int parse_uint(char const **arg, int comma)
 }
 
 static const char wrap_arg_usage[] = "-w[<width>[,<indent1>[,<indent2>]]]";
+struct wrap_arg {
+	int enabled;
+	int len;
+	int in1;
+	int in2;
+};
 #define DEFAULT_WRAPLEN 76
 #define DEFAULT_INDENT1 6
 #define DEFAULT_INDENT2 9
 
-static void parse_wrap_args(const char *arg, int *in1, int *in2, int *wrap)
+static int parse_wrap(const struct option *opt, const char *arg, int unset)
 {
-	arg += 2; /* skip -w */
+	struct wrap_arg *wrap = opt->value;
 
-	*wrap = parse_uint(&arg, ',');
-	if (*wrap < 0)
+	wrap->enabled = 1;
+
+	if (!arg)
+		return 0;
+	wrap->len = parse_uint(&arg, ',');
+	if (wrap->len < 0)
 		die(wrap_arg_usage);
-	*in1 = parse_uint(&arg, ',');
-	if (*in1 < 0)
+	wrap->in1 = parse_uint(&arg, ',');
+	if (wrap->in1 < 0)
 		die(wrap_arg_usage);
-	*in2 = parse_uint(&arg, '\0');
-	if (*in2 < 0)
+	wrap->in2 = parse_uint(&arg, '\0');
+	if (wrap->in2 < 0)
 		die(wrap_arg_usage);
 
-	if (!*wrap)
-		*wrap = DEFAULT_WRAPLEN;
-	if (!*in1)
-		*in1 = DEFAULT_INDENT1;
-	if (!*in2)
-		*in2 = DEFAULT_INDENT2;
-	if (*wrap &&
-	    ((*in1 && *wrap <= *in1) ||
-	     (*in2 && *wrap <= *in2)))
+	if (!wrap->len)
+		wrap->len = DEFAULT_WRAPLEN;
+	if (!wrap->in1)
+		wrap->in1 = DEFAULT_INDENT1;
+	if (!wrap->in2)
+		wrap->in2 = DEFAULT_INDENT2;
+	if (wrap->len &&
+	    ((wrap->in1 && wrap->len <= wrap->in1) ||
+	     (wrap->in2 && wrap->len <= wrap->in2)))
 		die(wrap_arg_usage);
+	return 0;
 }
 
 int cmd_shortlog(int argc, const char **argv, const char *prefix)
@@ -216,34 +230,31 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 	struct rev_info rev;
 	struct path_list list = { NULL, 0, 0, 1 };
 	int i, j, sort_by_number = 0, summary = 0;
-	int wrap_lines = 0;
-	int wrap = DEFAULT_WRAPLEN;
-	int in1 = DEFAULT_INDENT1;
-	int in2 = DEFAULT_INDENT2;
-
-	/* since -n is a shadowed rev argument, parse our args first */
-	while (argc > 1) {
-		if (!strcmp(argv[1], "-n") || !strcmp(argv[1], "--numbered"))
-			sort_by_number = 1;
-		else if (!strcmp(argv[1], "-s") ||
-				!strcmp(argv[1], "--summary"))
-			summary = 1;
-		else if (!strcmp(argv[1], "-e") ||
-			 !strcmp(argv[1], "--email"))
-			email = 1;
-		else if (!prefixcmp(argv[1], "-w")) {
-			wrap_lines = 1;
-			parse_wrap_args(argv[1], &in1, &in2, &wrap);
-		}
-		else if (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help"))
-			usage(shortlog_usage);
-		else
-			break;
-		argv++;
-		argc--;
-	}
+	struct wrap_arg wrap = {
+		0,
+		DEFAULT_WRAPLEN,
+		DEFAULT_INDENT1,
+		DEFAULT_INDENT2
+	};
+	struct option options[] = {
+		OPT_BOOLEAN('n', "numbered", &sort_by_number, "sort by number"),
+		OPT_BOOLEAN('s', "summary", &summary,
+				"show commit count summary"),
+		OPT_BOOLEAN('e', "email", &email, "show email addresses"),
+		{
+			OPTION_CALLBACK, 'w', NULL, &wrap, "wrap",
+			"line-wrap commit message", PARSE_OPT_OPTARG,
+			parse_wrap
+		},
+	};
+
+	argc = parse_options(argc, argv, options, shortlog_usage, 0);
+
 	init_revisions(&rev, prefix);
-	argc = setup_revisions(argc, argv, &rev, NULL);
+	/* setup_revisions expects to have the program name in argv[0],
+	 * but parse_options has removed it. We have to do parse_options
+	 * before setup_revisions, though, to claim "-n". */
+	argc = setup_revisions(argc+1, argv-1, &rev, NULL);
 	if (argc > 1)
 		die ("unrecognized argument: %s", argv[1]);
 
@@ -272,9 +283,11 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 			for (j = onelines->nr - 1; j >= 0; j--) {
 				const char *msg = onelines->items[j].path;
 
-				if (wrap_lines) {
-					int col = print_wrapped_text(msg, in1, in2, wrap);
-					if (col != wrap)
+				if (wrap.enabled) {
+					int col = print_wrapped_text(msg,
+							wrap.in1, wrap.in2,
+							wrap.len);
+					if (col != wrap.len)
 						putchar('\n');
 				}
 				else
-- 
1.5.4.rc0.1088.gfa79c-dirty

^ permalink raw reply related

* Re: git describe candidate filtering
From: Shawn O. Pearce @ 2007-12-13  5:57 UTC (permalink / raw)
  To: Eric Blake; +Cc: git
In-Reply-To: <4760C250.5080103@byu.net>

Eric Blake <ebb9@byu.net> wrote:
> Would it be possible to add an option to git-describe that limits the
> candidate tags to those matching a glob pattern or regular expression?
> I've got a repository imported from CVS which has a number of less
> interesting tags for events such as branch creation, but I'm only
> interested in describing the current commit relative to tags matching
> v[0-9]\.[0-9]*, rather than the closest tag of any spelling.

Sure, it shouldn't be too difficult, just need to alter the get_name()
function to add the item with add_to_known_names() only if it matches
the pattern.  Take a look at builtin-describe.c around line 74...

We only match things that were "added".  So if you don't add a tag
via add_to_known_names() then git-describe won't return it.

However that's only true for the standard case; for --contains its
a different story as we are actually passing everything to name-rev.

-- 
Shawn.

^ permalink raw reply

* Re: git describe candidate filtering
From: Junio C Hamano @ 2007-12-13  6:12 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Eric Blake, git
In-Reply-To: <20071213055746.GV14735@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> We only match things that were "added".  So if you don't add a tag
> via add_to_known_names() then git-describe won't return it.
>
> However that's only true for the standard case; for --contains its
> a different story as we are actually passing everything to name-rev.

A good news is that name-rev is supposed to respect the name limiter.

^ permalink raw reply

* git-svn: expand handling of From: and Signed-off-by:
From: Andy Whitcroft @ 2007-12-13  6:58 UTC (permalink / raw)
  To: git; +Cc: Eric Wong

The current parsing for From: and Signed-off-by: lines handles fully
specified names:

	From: Full Name <email@address>

Expand this to include the raw email addresses and straight "names":

	From: email@address       -> email <email@address>
	From: Full Name           -> Full Name <unknown>

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 git-svn.perl |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 54d7844..058f8e9 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2363,11 +2363,20 @@ sub make_log_entry {
 
 	my ($commit_name, $commit_email) = ($name, $email);
 	if ($_use_log_author) {
-		if ($log_entry{log} =~ /From:\s+(.*?)\s+<(.*)>\s*\n/) {
-			($name, $email) = ($1, $2);
-		} elsif ($log_entry{log} =~
-		                      /Signed-off-by:\s+(.*?)\s+<(.*)>\s*\n/) {
+		my $name_field;
+		if ($log_entry{log} =~ /From:\s+(.*\S)\s*\n/i) {
+			$name_field = $1;
+		} elsif ($log_entry{log} =~ /Signed-off-by:\s+(.*\S)\s*\n/i) {
+			$name_field = $1;
+		}
+		if (!defined $name_field) {
+			#
+		} elsif ($name_field =~ /(.*?)\s+<(.*)>/) {
 			($name, $email) = ($1, $2);
+        	} elsif ($name_field =~ /(.*)@/) {
+			($name, $email) = ($1, $name_field);
+		} else {
+			($name, $email) = ($name_field, 'unknown');
 		}
 	}
 	if (defined $headrev && $self->use_svm_props) {

^ permalink raw reply related

* Re: [PATCH 2/2] pack-objects: fix threaded load balancing
From: Johannes Sixt @ 2007-12-13  7:15 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, git, Jon Smirl
In-Reply-To: <alpine.LFD.0.99999.0712111227080.555@xanadu.home>

Nicolas Pitre schrieb:
> On Tue, 11 Dec 2007, Johannes Sixt wrote:
> 
>> Nicolas Pitre schrieb:
>>> @@ -1612,10 +1620,10 @@ static void *threaded_find_deltas(void *arg)
>>>  		pthread_mutex_lock(&data_ready);
>>>  		pthread_mutex_unlock(&data_request);
>>>  
>>> -		if (!me->list_size)
>>> +		if (!me->remaining)
>>>  			return NULL;
>>>  
>>> -		find_deltas(me->list, me->list_size,
>>> +		find_deltas(me->list, &me->remaining,
>>>  			    me->window, me->depth, me->processed);
>>>  	}
>>>  }
>> This hunk caught my attention. &data_ready is locked, but not released in
>> this function.
>>
>> Looking more closely at the code surrounding this hunk, it seems that the
>> lock is released in a *different* thread than the one that locked it. This
>> works on Linux, but is not portable. We will have to use condition variables
>> like every one else does in a producer-consumer-like scenario.
> 
> Are you willing to make a patch for it?

I'm working on it, slowly.

-- Hannes

^ permalink raw reply

* Re: Something is broken in repack
From: Andreas Ericsson @ 2007-12-13  7:32 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Jon Smirl, Junio C Hamano, gcc, Git Mailing List
In-Reply-To: <alpine.LFD.0.99999.0712121106400.555@xanadu.home>

Nicolas Pitre wrote:
> On Wed, 12 Dec 2007, Nicolas Pitre wrote:
> 
>> I did modify the progress display to show accounted memory that was 
>> allocated vs memory that was freed but still not released to the system.  
>> At least that gives you an idea of memory allocation and fragmentation 
>> with glibc in real time:
>>
>> diff --git a/progress.c b/progress.c
>> index d19f80c..46ac9ef 100644
>> --- a/progress.c
>> +++ b/progress.c
>> @@ -8,6 +8,7 @@
>>   * published by the Free Software Foundation.
>>   */
>>  
>> +#include <malloc.h>
>>  #include "git-compat-util.h"
>>  #include "progress.h"
>>  
>> @@ -94,10 +95,12 @@ static int display(struct progress *progress, unsigned n, const char *done)
>>  	if (progress->total) {
>>  		unsigned percent = n * 100 / progress->total;
>>  		if (percent != progress->last_percent || progress_update) {
>> +			struct mallinfo m = mallinfo();
>>  			progress->last_percent = percent;
>> -			fprintf(stderr, "%s: %3u%% (%u/%u)%s%s",
>> -				progress->title, percent, n,
>> -				progress->total, tp, eol);
>> +			fprintf(stderr, "%s: %3u%% (%u/%u) %u/%uMB%s%s",
>> +				progress->title, percent, n, progress->total,
>> +				m.uordblks >> 18, m.fordblks >> 18,
>> +				tp, eol);
> 
> Note: I didn't know what unit of memory those blocks represents, so the 
> shift is most probably wrong.
> 

Me neither, but it appears to me as if hblkhd holds the actual memory
consumed by the process. It seems to store the information in bytes,
which I find a bit dubious unless glibc has some internal multiplier.

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

^ permalink raw reply

* Re: [PATCH] Teach git-gui to split hunks
From: Johannes Sixt @ 2007-12-13  7:35 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Jason Sewall, Shawn O. Pearce, David, Marco Costalba,
	Andy Parkins, git
In-Reply-To: <Pine.LNX.4.64.0712121931050.27959@racer.site>

Johannes Schindelin schrieb:
> When you select the context menu item "Split Hunk" in the diff area,
> git-gui will now split the current hunk so that a new hunk starts at
> the current position.
> 
> For this to work, apply has to be called with --unidiff-zero, since
> the new hunks can start or stop with a "-" or "+" line.

NACK! --unidiff-zero eats your data.

1. Prepare a modification that adds 2 lines that are *not* adjacent, like this:

	@@ -6,6 +6,8 @@ git-checkout [options] [<branch>] [<paths>...]
	 --
	 b=          create a new branch started at <branch>
	+first
	 l           create the new branch's reflog
	 track       arrange that the new branch tracks the remote branch
	+after track
	 f           proceed even if the index or working tree is not HEAD
	 m           merge local modifications into the new branch

2. Reduce context to zero.
3. Stage *second* hunk.

Result: It is staged at the wrong place:

	@@ -9,4 +9,5 @@ l           create the new branch's reflog
	 track       arrange that the new branch tracks the remote branch
	 f           proceed even if the index or working tree is not HEAD
	+after track
	 m           merge local modifications into the new branch
	 q,quiet     be quiet


Reason: --unidiff-zero can only look at the line numbers. And those are
wrong because it doesn't account for the shift in line numbers caused by the
first hunk.

-- Hannes

^ permalink raw reply

* Re: [PATCH] git-commit: squelch needless message during an empty merge
From: Johannes Sixt @ 2007-12-13  7:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Kristian Høgsberg
In-Reply-To: <7v1w9r5m03.fsf_-_@gitster.siamese.dyndns.org>

Junio C Hamano schrieb:
> This introduces "nowarn" parameter to run_status() to avoid these
> useless messages.

Hmpf. We don't need no double negations, no?

-- Hannes

^ permalink raw reply

* Re: [PATCH] Teach git-gui to split hunks
From: Shawn O. Pearce @ 2007-12-13  7:48 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Johannes Schindelin, Jason Sewall, David, Marco Costalba,
	Andy Parkins, git
In-Reply-To: <4760E0CF.1030805@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> wrote:
> Johannes Schindelin schrieb:
> > When you select the context menu item "Split Hunk" in the diff area,
> > git-gui will now split the current hunk so that a new hunk starts at
> > the current position.
> > 
> > For this to work, apply has to be called with --unidiff-zero, since
> > the new hunks can start or stop with a "-" or "+" line.
> 
> NACK! --unidiff-zero eats your data.

Yea, don't worry about that, I won't be applying any patch to git-gui
that feeds data to git-apply with --undiff-zero.  Not unless its
completely bullet-proof that the hunk headers will *never* be wrong.

I'd rather always apply with context and let git-apply do its thing
to validate the hunks.  If you can get the hunk headers computed
right you can also get the context computed right, which means
git-apply can actually verify the patch can be applied, thus double
checking the splitter.
 
> Reason: --unidiff-zero can only look at the line numbers. And those are
> wrong because it doesn't account for the shift in line numbers caused by the
> first hunk.

-- 
Shawn.

^ permalink raw reply

* Re: git-cvsexportcommit fails for huge commits
From: Markus Klinik @ 2007-12-13  8:39 UTC (permalink / raw)
  To: Jeff King; +Cc: Martin Langhoff, git
In-Reply-To: <20071213041737.GA23624@coredump.intra.peff.net>

On Wed, Dec 12, 2007 at 11:17:37PM -0500, Jeff King wrote:
> it would be nice to get feedback from Markus on what platform he is
> using and what is a reasonable value), or just using a tempfile with
> xargs, which should figure out the correct value.

My platform is an ubuntu 7.04.

$ uname -a 
Linux mkl-desktop 2.6.20-16-386 #2 Sun Sep 23 19:47:10 UTC 2007 i686
GNU/Linux
$ ulimit -s
8192

I wasn't able to verify if the patch or increasing the stack limit works
because I stumbled upon what seems to be this problem:
http://marc.info/?l=git&m=119419453726664&w=2

There is already a patch for the cvs-status-fileorder bug but it doesn't
apply and obviously never made it to the public reopsitory.
http://marc.info/?l=git&m=118718448305647&w=2

At the end of this message is a session log that reproduces this
behaviour. The session uses git-cvsexportcommit without the xargs-patch.

If my understanding is correct the 'cvs status' query is just to make
sure everything is up to date, and can be overridden by the -f flag.
I'll try that today and report if the xargs-patch and the stack limit
thing worked.

-Markus

mkl@mkl-desktop:~/tmp$ # ---------------------- begin
mkl@mkl-desktop:~$ cvs --version

Concurrent Versions System (CVS) 1.12.13 (client/server)

Copyright (C) 2005 Free Software Foundation, Inc.

Senior active maintainers include Larry Jones, Derek R. Price,
and Mark D. Baushke.  Please see the AUTHORS and README files from the
CVS
distribution kit for a complete list of contributors and copyrights.

CVS may be copied only under the terms of the GNU General Public
License,
a copy of which can be found with the CVS distribution kit.

Specify the --help option for further information about CVS
mkl@mkl-desktop:~$ git --version
git version 1.5.3.7.1157.gbf82a
mkl@mkl-desktop:~/tmp$ mkdir ~/cvsroot
mkl@mkl-desktop:~/tmp$ export CVSROOT=~/cvsroot
mkl@mkl-desktop:~/tmp$ cvs init
mkl@mkl-desktop:~/tmp$ mkdir cfoo
mkl@mkl-desktop:~/tmp$ cd cfoo/
mkl@mkl-desktop:~/tmp/cfoo$ cvs import -m'import' cfoo A AA

No conflicts created by this import

mkl@mkl-desktop:~/tmp/cfoo$ cd ..
mkl@mkl-desktop:~/tmp$ mkdir gfoo
mkl@mkl-desktop:~/tmp$ cd gfoo/
mkl@mkl-desktop:~/tmp/gfoo$ git-init
Initialized empty Git repository in .git/
mkl@mkl-desktop:~/tmp/gfoo$ mkdir bar
mkl@mkl-desktop:~/tmp/gfoo$ touch a b c d bar/a bar/b bar/e
mkl@mkl-desktop:~/tmp/gfoo$ git add .
mkl@mkl-desktop:~/tmp/gfoo$ git commit -m'empty'
Created initial commit a11f9bc: empty
 0 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 a
 create mode 100644 b
 create mode 040000 bar
 create mode 100644 c
 create mode 100644 d
mkl@mkl-desktop:~/tmp/gfoo$ echo 'hi' > 0
mkl@mkl-desktop:~/tmp/gfoo$ echo 'hi' > a
mkl@mkl-desktop:~/tmp/gfoo$ echo 'hi' > bar/e
mkl@mkl-desktop:~/tmp/gfoo$ echo 'hi' > e
mkl@mkl-desktop:~/tmp/gfoo$ git add .
mkl@mkl-desktop:~/tmp/gfoo$ git commit -m 'hi'
Created commit e71a144: hi
 3 files changed, 3 insertions(+), 0 deletions(-)
 create mode 100644 0
 create mode 100644 e
mkl@mkl-desktop:~/tmp/gfoo$ cd ..
mkl@mkl-desktop:~/tmp$ cvs co cfoo
cvs checkout: Updating cfoo
mkl@mkl-desktop:~/tmp$ cd cfoo
mkl@mkl-desktop:~/tmp/cfoo$ export GIT_DIR=../gfoo/.git
mkl@mkl-desktop:~/tmp/cfoo$ git-cvsexportcommit -c HEAD^ 
Checking if patch will apply
cvs status: nothing known about `a'
cvs status: nothing known about `b'
cvs status: nothing known about `c'
cvs status: nothing known about `d'
Applying
Patch applied successfully. Adding new files and directories to CVS
Directory /home/mkl/cvsroot/cfoo/bar added to the repository
cvs add: scheduling file `a' for addition
cvs add: use `cvs commit' to add this file permanently
cvs add: scheduling file `b' for addition
cvs add: use `cvs commit' to add this file permanently
cvs add: scheduling file `bar/a' for addition
cvs add: use `cvs commit' to add this file permanently
cvs add: scheduling file `bar/b' for addition
cvs add: use `cvs commit' to add this file permanently
cvs add: scheduling file `bar/e' for addition
cvs add: use `cvs commit' to add this file permanently
cvs add: scheduling file `c' for addition
cvs add: use `cvs commit' to add this file permanently
cvs add: scheduling file `d' for addition
cvs add: use `cvs commit' to add this file permanently
Commit to CVS
Patch title (first comment line): empty
Autocommit
  cvs commit -F .msg 'a' 'b' 'bar/a' 'bar/b' 'bar/e' 'c' 'd'
/home/mkl/cvsroot/cfoo/a,v  <--  a
initial revision: 1.1
/home/mkl/cvsroot/cfoo/b,v  <--  b
initial revision: 1.1
/home/mkl/cvsroot/cfoo/c,v  <--  c
initial revision: 1.1
/home/mkl/cvsroot/cfoo/d,v  <--  d
initial revision: 1.1
/home/mkl/cvsroot/cfoo/bar/a,v  <--  bar/a
initial revision: 1.1
/home/mkl/cvsroot/cfoo/bar/b,v  <--  bar/b
initial revision: 1.1
/home/mkl/cvsroot/cfoo/bar/e,v  <--  bar/e
initial revision: 1.1
Committed successfully to CVS
mkl@mkl-desktop:~/tmp/cfoo$ git-cvsexportcommit -c HEAD
Checking if patch will apply
cvs status: nothing known about `0'
cvs status: nothing known about `e'
File e is already known in your CVS checkout -- perhaps it has been added by another user. Or this may indicate that it exists on a different branch. If this is the case, use -f to force the merge.
Status was: Up-to-date
File bar/e not up to date but has status 'Unknown' in your CVS checkout!
Exiting: your CVS tree is not clean for this merge. at /home/mkl/bin/git-cvsexportcommit line 235.
mkl@mkl-desktop:~/tmp/cfoo$ # ----------- end

^ permalink raw reply

* Re: [PATCH] Teach git-gui to split hunks
From: Junio C Hamano @ 2007-12-13  8:45 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Jason Sewall, Shawn O. Pearce, David, Marco Costalba,
	Andy Parkins, git
In-Reply-To: <Pine.LNX.4.64.0712121931050.27959@racer.site>

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

> For this to work, apply has to be called with --unidiff-zero, since
> the new hunks can start or stop with a "-" or "+" line.

You do not have to do "unidiff zero".  Suppose you have this hunk you
need to split.

diff --git a/read-cache.c b/read-cache.c
index 7db5588..4d12073 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -12,8 +12,8 @@
 /* Index extensions.
  *
  * The first letter should be 'A'..'Z' for extensions that are not
- * necessary for a correct operation (i.e. optimization data).
- * When new extensions are added that _needs_ to be understood in
+ * necessary for a correct operation (that is, optimization data).
+ * When new extensions are added that needs to be understood in
  * order to correctly interpret the index file, pick character that
  * is outside the range, to cause the reader to abort.
  */

Think about taking the s/i.e./that is,/ substitution without taking the
other s/_needs_/needs/ substitution.  You do not split the hunk between
two '-' lines, but effectively make it into this hunk instead:

diff --git a/read-cache.c b/read-cache.c
index 7db5588..4d12073 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -12,8 +12,8 @@
 /* Index extensions.
  *
  * The first letter should be 'A'..'Z' for extensions that are not
- * necessary for a correct operation (i.e. optimization data).
+ * necessary for a correct operation (that is, optimization data).
  * When new extensions are added that _needs_ to be understood in
  * order to correctly interpret the index file, pick character that
  * is outside the range, to cause the reader to abort.
  */

That is, , if you want to do finer grained hunk splitting than what "git
add -p" lets you do, you do _not_ let user specify "I want to split the
hunk into two, before this point and after this point".  Instead, let
the user pick zero or more '-' line and zero or more '+' line, and
adjust the context around it.  An unpicked '-' line becomes the common
context, and an unpicked '+' line disappears.  After that, you recount
the diff.  That way, you do not have to do any "unidiff zero" cop-out.

At the same time, you can stash away what was _not_ picked, creating two
variants to be applied on top of the result of applying (or not
applying) the picked patch, if you want to allow "undo".

(variant one: applies after the above is applied)
@@ -12,8 +12,8 @@
 /* Index extensions.
  *
  * The first letter should be 'A'..'Z' for extensions that are not
  * necessary for a correct operation (that is, optimization data).
- * When new extensions are added that _needs_ to be understood in
+ * When new extensions are added that needs to be understood in
  * order to correctly interpret the index file, pick character that
  * is outside the range, to cause the reader to abort.
  */

(variant two: applies if the above is not applied)
@@ -12,8 +12,8 @@
 /* Index extensions.
  *
  * The first letter should be 'A'..'Z' for extensions that are not
  * necessary for a correct operation (i.e. optimization data).
- * When new extensions are added that _needs_ to be understood in
+ * When new extensions are added that needs to be understood in
  * order to correctly interpret the index file, pick character that
  * is outside the range, to cause the reader to abort.
  */

^ permalink raw reply related

* Re: git-cvsexportcommit fails for huge commits
From: Jeff King @ 2007-12-13  9:01 UTC (permalink / raw)
  To: Markus Klinik; +Cc: Martin Langhoff, git
In-Reply-To: <20071213083931.GA6441@mkl-desktop>

On Thu, Dec 13, 2007 at 09:39:31AM +0100, Markus Klinik wrote:

> > it would be nice to get feedback from Markus on what platform he is
> > using and what is a reasonable value), or just using a tempfile with
> > xargs, which should figure out the correct value.
> 
> My platform is an ubuntu 7.04.

In that case, I think a very large value like 100K is fine. Then the code
won't activate unless it is really required.

It still feels a bit hack-ish and wrong, but I'm not sure what would be
better.

-Peff

^ permalink raw reply

* Re: [RFH] convert shortlog to use parse_options
From: Pierre Habouzit @ 2007-12-13  9:06 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20071213055226.GA3636@coredump.intra.peff.net>

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

On Thu, Dec 13, 2007 at 05:52:26AM +0000, Jeff King wrote:
> Pierre,
> 
> I am converting shortlog to use parse_options, but I have hit a behavior
> I can't seem to represent.
> 
> The "-w" option has an optional argument, and we used to accept only
> "-w" or "-wX,Y,Z". However, parse_options allows "-w X,Y,Z".
> 
> This means that "-w HEAD" used to mean "turn on wrapping
> with default parameters, look at HEAD" and now translates to
> "-w with parameter HEAD".
> 
> Is there a way to do this currently, or can we add an option flag?

No we can't. And I believe that such a thing is definitely bad practice
:/ So if you really need to, we will have to add some PARSE_OPT_STICKARG
or sth alike that would check that the argument was "sticked" to the
option either with `-wA,B,C` or `--long-opt=A,B,C` depending on the fact
that an option is short or long.

Though: `git shortlog -w -- HEAD` will work properly because option
arguments don't take the next token as an argument thing if it starts
with a dash.

Though note that you can't migrate things that use init_revisions and so
on to parseoptions yet, because revisions also have dashed tokens (--not
e.g.) and that the first run of parse_options will just hate it and
fail. Maybe we can do parse_options work in multiple passes though, but
that would require a quite extensive rethought of the module, the
introduction of a parseopt context to be freed after the last pass
(because we will have a lot of small allocation stuff going on). I'll
try to see what I can do in that direction.

For that we must migrate diff and revisions option parser as big macros
(I started[0] it but didn't had the time to complete it yet, and it's
quite a huge task, because there is no incremental upgrade path here,
and there are commands using both diff and revisions options so we must
migrate both at once) and use aggregated parseoptions specifiers.

  [0] http://git.madism.org/?p=git.git;a=commitdiff;h=059cfb6d4cfbdff68d81577d00c9dbce6fed443e

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

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

^ permalink raw reply

* Re: [RFH] convert shortlog to use parse_options
From: Jeff King @ 2007-12-13  9:10 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <20071213090604.GA12398@artemis.madism.org>

On Thu, Dec 13, 2007 at 10:06:04AM +0100, Pierre Habouzit wrote:

> No we can't. And I believe that such a thing is definitely bad practice
> :/ So if you really need to, we will have to add some PARSE_OPT_STICKARG
> or sth alike that would check that the argument was "sticked" to the
> option either with `-wA,B,C` or `--long-opt=A,B,C` depending on the fact
> that an option is short or long.

Yes, I am not sure if the right solution is to just say "we are changing
how -w works". Because it either must change, or it must be inconsistent
with the rest of the option parsing for the rest of eternity.

> Though note that you can't migrate things that use init_revisions and so
> on to parseoptions yet, because revisions also have dashed tokens (--not
> e.g.) and that the first run of parse_options will just hate it and

Ah, yes. I hadn't really considered that angle yet (my basic testing had
just been on non-dashed revision parameters).

I will table this patch for now, then, since it obviously is too complex
for v1.5.4.

Thanks for your input.

-Peff

^ permalink raw reply

* Re: [RFH] convert shortlog to use parse_options
From: Pierre Habouzit @ 2007-12-13  9:29 UTC (permalink / raw)
  To: Jeff King, git
In-Reply-To: <20071213090604.GA12398@artemis.madism.org>

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

On jeu, déc 13, 2007 at 09:06:04 +0000, Pierre Habouzit wrote:
> fail. Maybe we can do parse_options work in multiple passes though, but
> that would require a quite extensive rethought of the module, the

  In fact it's not doable because of short options. I knew I already had
that idea but dropped it, the reason is that if you have the atom:

  -ab

  That the first pass knows about a -b, and that the second pass knows
about -a, then you have a problem. Because at soon you meet a flag that
you don't know about, you cannot parse any further. So we're back at
what I said, we must migrate revs and diff options parsing first, and
that's a huge task.


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

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

^ permalink raw reply

* Re: [RFH] convert shortlog to use parse_options
From: Pierre Habouzit @ 2007-12-13  9:35 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20071213091055.GA5674@coredump.intra.peff.net>

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

On Thu, Dec 13, 2007 at 09:10:56AM +0000, Jeff King wrote:
> On Thu, Dec 13, 2007 at 10:06:04AM +0100, Pierre Habouzit wrote:
> 
> > No we can't. And I believe that such a thing is definitely bad practice
> > :/ So if you really need to, we will have to add some PARSE_OPT_STICKARG
> > or sth alike that would check that the argument was "sticked" to the
> > option either with `-wA,B,C` or `--long-opt=A,B,C` depending on the fact
> > that an option is short or long.
> 
> Yes, I am not sure if the right solution is to just say "we are changing
> how -w works". Because it either must change, or it must be inconsistent
> with the rest of the option parsing for the rest of eternity.

In fact we have kind of the issue for every single optional argument out
there:

$ git describe --abbrev HEAD
error: option `abbrev' expects a numerical value
[...]

  *ouch*

So I believe that with optional arguments we must change the way we do
things, and that we _must_ enforce the argument to be sticked in that
case. Because this kind of backward incompatibility I totally missed in
the first place is unacceptable. Patch on its way.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ 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