Kristian Høgsberg [off-list ref] writes:
quoted hunk
@@ -364,7 +365,6 @@ BUILTIN_OBJS = \
builtin-rev-parse.o \
builtin-revert.o \
builtin-rm.o \
- builtin-runstatus.o \
builtin-shortlog.o \
builtin-show-branch.o \
builtin-stripspace.o \
I did not read in the commit log that runstatus is gone...
quoted hunk
diff --git a/builtin-commit.c b/builtin-commit.c
new file mode 100644
index 0000000..56c7427
--- /dev/null
+++ b/builtin-commit.c
@@ -0,0 +1,608 @@
+/*
+ * Builtin "git commit"
+ *
+ * Copyright (c) 2007 Kristian Høgsberg <krh@redhat.com>
+ * Based on git-commit.sh by Junio C Hamano and Linus Torvalds
+ */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include "cache.h"
The system header files on some systems have a nasty habit of
changing the behaviour depending on the order they are included.
Since 85023577a8f4b540aa64aa37f6f44578c0c305a3 (simplify
inclusion of system header files) in late 2006, we have a few
rules for system-header inclusion to avoid the portability
issues:
(1) sources under compat/, platform sha-1 implementations, and
xdelta code are exempt from the following rules;
(2) the first #include must be "git-compat-util.h" or one of
our own header file that includes it first (e.g. config.h,
builtin.h, pkt-line.h);
(3) system headers that are included in "git-compat-util.h"
need not be included in individual C source files.
(4) "git-compat-util.h" does not have to include subsystem
specific header files (e.g. expat.h).
+static void determine_author_info(struct strbuf *sb)
+{
+ char *p, *eol;
+ char *name = NULL, *email = NULL;
+
+ if (use_message) {
+ p = strstr(use_message_buffer, "\nauthor");
+ if (!p)
+ die("invalid commit: %s\n", use_message);
+ p++;
+ eol = strchr(p, '\n');
+ if (!eol)
+ die("invalid commit: %s\n", use_message);
+
+ strbuf_add(sb, p, eol + 1 - p);
+ } else if (force_author) {
+ const char *eoname = strstr(force_author, " <");
+ const char *eomail = strchr(force_author, '>');
Doesn't this break:
$ git commit --amend --author='A U Thor [off-list ref]'
to fix a misattribution?
+static int parse_and_validate_options(int argc, const char *argv[])
+{
...
+ if (use_message) {
+ unsigned char sha1[20];
+ static char utf8[] = "UTF-8";
+ const char *out_enc;
+ char *enc, *end;
+ struct commit *commit;
+
+ if (get_sha1(use_message, sha1))
+ die("could not lookup commit %s", use_message);
+ commit = lookup_commit(sha1);
+ if (!commit || parse_commit(commit))
+ die("could not parse commit %s", use_message);
+
+ enc = strstr(commit->buffer, "\nencoding");
+ if (enc) {
+ end = strchr(enc + 10, '\n');
+ enc = xstrndup(enc + 10, end - (enc + 10));
+ } else {
+ enc = utf8;
+ }
+ out_enc = git_commit_encoding ? git_commit_encoding : utf8;
+
+ use_message_buffer =
+ reencode_string(commit->buffer, out_enc, enc);
+ if (enc != utf8)
+ free(enc);
A few issues.
* When use_message is set because of --amend that wanted to
amend a commit message that was recorded in a corrupt
encoding, and iconv() in reencode_string() fails, saying "I
cannot convert that completely", most of the message can
still be salvageable. This part should allow looser
reencoding if the message will be eyeballed and edited by the
user.
* We would want to avoid reencoding when !strcmp(out_enc, enc).
Both builtin-mailinfo.c and commit.c skip the call to the
function at the calling site, but it might make sense to
teach reencode_string() about such an optimization.
On Thu, 2007-11-01 at 16:51 -0700, Junio C Hamano wrote:
Hi Junio,
Thanks for the review (again). I've split the patch up in a couple of
test suite patches (a patch to accept ':' as a no-op editor, fixing
hard-coding of editor, and a test case for the commit --amend --author
case), exporting launch_editor() and a reworked version of the commit
patch that addresses your comments.
Kristian Høgsberg [off-list ref] writes:
quoted
@@ -364,7 +365,6 @@ BUILTIN_OBJS = \
builtin-rev-parse.o \
builtin-revert.o \
builtin-rm.o \
- builtin-runstatus.o \
builtin-shortlog.o \
builtin-show-branch.o \
builtin-stripspace.o \
I did not read in the commit log that runstatus is gone...
True, that should be documented. Added in the following patch.
quoted
diff --git a/builtin-commit.c b/builtin-commit.c
new file mode 100644
index 0000000..56c7427
--- /dev/null
+++ b/builtin-commit.c
@@ -0,0 +1,608 @@
+/*
+ * Builtin "git commit"
+ *
+ * Copyright (c) 2007 Kristian Høgsberg <krh@redhat.com>
+ * Based on git-commit.sh by Junio C Hamano and Linus Torvalds
+ */
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include "cache.h"
The system header files on some systems have a nasty habit of
changing the behaviour depending on the order they are included.
Since 85023577a8f4b540aa64aa37f6f44578c0c305a3 (simplify
inclusion of system header files) in late 2006, we have a few
rules for system-header inclusion to avoid the portability
issues:
(1) sources under compat/, platform sha-1 implementations, and
xdelta code are exempt from the following rules;
(2) the first #include must be "git-compat-util.h" or one of
our own header file that includes it first (e.g. config.h,
builtin.h, pkt-line.h);
(3) system headers that are included in "git-compat-util.h"
need not be included in individual C source files.
(4) "git-compat-util.h" does not have to include subsystem
specific header files (e.g. expat.h).
Ah, yes, this has been pointed out to me before, sorry. Patch updated.
quoted
+static void determine_author_info(struct strbuf *sb)
+{
+ char *p, *eol;
+ char *name = NULL, *email = NULL;
+
+ if (use_message) {
+ p = strstr(use_message_buffer, "\nauthor");
+ if (!p)
+ die("invalid commit: %s\n", use_message);
+ p++;
+ eol = strchr(p, '\n');
+ if (!eol)
+ die("invalid commit: %s\n", use_message);
+
+ strbuf_add(sb, p, eol + 1 - p);
+ } else if (force_author) {
+ const char *eoname = strstr(force_author, " <");
+ const char *eomail = strchr(force_author, '>');
Doesn't this break:
$ git commit --amend --author='A U Thor [off-list ref]'
to fix a misattribution?
Dang, good catch. I've fixed it by swapping the (use_message) and
(force_author) clauses and the new patch adds a test case for this.
quoted
+static int parse_and_validate_options(int argc, const char *argv[])
+{
...
+ if (use_message) {
+ unsigned char sha1[20];
+ static char utf8[] = "UTF-8";
+ const char *out_enc;
+ char *enc, *end;
+ struct commit *commit;
+
+ if (get_sha1(use_message, sha1))
+ die("could not lookup commit %s", use_message);
+ commit = lookup_commit(sha1);
+ if (!commit || parse_commit(commit))
+ die("could not parse commit %s", use_message);
+
+ enc = strstr(commit->buffer, "\nencoding");
+ if (enc) {
+ end = strchr(enc + 10, '\n');
+ enc = xstrndup(enc + 10, end - (enc + 10));
+ } else {
+ enc = utf8;
+ }
+ out_enc = git_commit_encoding ? git_commit_encoding : utf8;
+
+ use_message_buffer =
+ reencode_string(commit->buffer, out_enc, enc);
+ if (enc != utf8)
+ free(enc);
A few issues.
* When use_message is set because of --amend that wanted to
amend a commit message that was recorded in a corrupt
encoding, and iconv() in reencode_string() fails, saying "I
cannot convert that completely", most of the message can
still be salvageable. This part should allow looser
reencoding if the message will be eyeballed and edited by the
user.
So in this case we just want to copy the old message byte for byte,
right? That's what I have in the updated patch.
* We would want to avoid reencoding when !strcmp(out_enc, enc).
Both builtin-mailinfo.c and commit.c skip the call to the
function at the calling site, but it might make sense to
teach reencode_string() about such an optimization.
Right. I just added the call-site optimization for builtin-commit for
now, but I would expect iconv() to be smart in case input and output
encodings are the same.
cheers,
Kristian