[RFC/PATCH 0/3] git log --pretty=lua

[RFC/PATCH 0/3] git log --pretty=lua

[Jeff King]

We've talked off and on about extending the --pretty=format specifiers
to something more flexible. There's also been talk recently of more
flexible commit-filtering (e.g., grepping individual notes).  Rather
than invent a new Turing-complete language, I thought I'd try building
on somebody else's work by embedding an existing language.

Why Lua? I don't especially like it as a language. But it's designed for
this purpose, which makes it very lightweight and relatively simple to
embed. Here are timing results for a few log commands (best-of-five,
warm cache):

  $ git log --oneline >/dev/null
  real    0m1.042s
  user    0m0.660s
  sys     0m0.372s

  $ git log --tformat:"%h %s" >/dev/null
  real    0m1.039s
  user    0m0.624s
  sys     0m0.396s

  $ git log --pretty=lua:'return abbrev(hash()) .. " " .. subject()'
  real    0m1.112s
  user    0m0.716s
  sys     0m0.388s

So you can see that we're a little bit slower than the existing format,
but not too much. There may well be some optimizations we can do, too.
This is the first time I've ever played with embedding Lua, so I would
not be surprised if I got something wrong or suboptimal.

The syntax, on the other hand...yuck. One thing that makes Lua
horrible for this use is that it does not have interpolated strings.
However, there are template libraries for Lua, so maybe there's
something there.

The patches are:

  [1/3]: pretty: make some commit-parsing helpers more public
  [2/3]: add basic lua infrastructure
  [3/3]: add a "lua" pretty format

And a "4/3" patch would probably add "--lua-filter" as a revision option
for limiting commits.

The patches are very rough and not meant to be applied. For me, this was
a bit of an experiment. I'm not sure if I like it or not. It seems like
a cool direction to go, but to be perfectly honest, I do not generally
feel like git's existing filtering or output are inadequate (sure, it's
slower to pipe --pretty=raw out to a separate perl filter and then do
fancy formatting, but it's usually fast enough, and it's very flexible).

So I don't have plans to work on it more any time soon, but I thought
I'd share in case anybody is interested. And if somebody wants to pick
up the topic and run with it, I'd be happy to help.

-Peff

[PATCH 1/3] pretty: make some commit-parsing helpers more public

[Jeff King]

This is a quick hack to make these functions available for
the lua code. It would be way cleaner to move all of the
incremental parsing bits to format-commit.[ch] and clean up
the names (e.g., "struct chunk" is not nearly descriptive
enough for a global).

Signed-off-by: Jeff King <peff@peff.net>
---
 commit.h | 36 ++++++++++++++++++++++++++++++++++++
 pretty.c | 38 ++------------------------------------
 2 files changed, 38 insertions(+), 36 deletions(-)

diff --git a/commit.h b/commit.h
index 9f21313..71cd4af 100644
--- a/commit.h
+++ b/commit.h
@@ -126,6 +126,42 @@ void pp_remainder(const struct pretty_print_context *pp,
 		  struct strbuf *sb,
 		  int indent);
 
+struct chunk {
+	size_t off;
+	size_t len;
+};
+
+struct format_commit_context {
+	const struct commit *commit;
+	const struct pretty_print_context *pretty_ctx;
+	unsigned commit_header_parsed:1;
+	unsigned commit_message_parsed:1;
+	unsigned commit_signature_parsed:1;
+	struct {
+		char *gpg_output;
+		char good_bad;
+		char *signer;
+	} signature;
+	char *message;
+	size_t width, indent1, indent2;
+
+	/* These offsets are relative to the start of the commit message. */
+	struct chunk author;
+	struct chunk committer;
+	struct chunk encoding;
+	size_t message_off;
+	size_t subject_off;
+	size_t body_off;
+
+	/* The following ones are relative to the result struct strbuf. */
+	struct chunk abbrev_commit_hash;
+	struct chunk abbrev_tree_hash;
+	struct chunk abbrev_parent_hashes;
+	size_t wrap_start;
+};
+
+void parse_commit_header(struct format_commit_context *);
+void parse_commit_message(struct format_commit_context *);
 
 /** Removes the first commit from a list sorted by date, and adds all
  * of its parents.
diff --git a/pretty.c b/pretty.c
index 8b1ea9f..0d4eb3d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -612,40 +612,6 @@ skip:
 	return 0; /* unknown placeholder */
 }
 
-struct chunk {
-	size_t off;
-	size_t len;
-};
-
-struct format_commit_context {
-	const struct commit *commit;
-	const struct pretty_print_context *pretty_ctx;
-	unsigned commit_header_parsed:1;
-	unsigned commit_message_parsed:1;
-	unsigned commit_signature_parsed:1;
-	struct {
-		char *gpg_output;
-		char good_bad;
-		char *signer;
-	} signature;
-	char *message;
-	size_t width, indent1, indent2;
-
-	/* These offsets are relative to the start of the commit message. */
-	struct chunk author;
-	struct chunk committer;
-	struct chunk encoding;
-	size_t message_off;
-	size_t subject_off;
-	size_t body_off;
-
-	/* The following ones are relative to the result struct strbuf. */
-	struct chunk abbrev_commit_hash;
-	struct chunk abbrev_tree_hash;
-	struct chunk abbrev_parent_hashes;
-	size_t wrap_start;
-};
-
 static int add_again(struct strbuf *sb, struct chunk *chunk)
 {
 	if (chunk->len) {
@@ -663,7 +629,7 @@ static int add_again(struct strbuf *sb, struct chunk *chunk)
 	return 0;
 }
 
-static void parse_commit_header(struct format_commit_context *context)
+void parse_commit_header(struct format_commit_context *context)
 {
 	const char *msg = context->message;
 	int i;
@@ -749,7 +715,7 @@ const char *format_subject(struct strbuf *sb, const char *msg,
 	return msg;
 }
 
-static void parse_commit_message(struct format_commit_context *c)
+void parse_commit_message(struct format_commit_context *c)
 {
 	const char *msg = c->message + c->message_off;
 	const char *start = c->message;
-- 
1.7.12.1.10.g6537447

[PATCH 2/3] add basic lua infrastructure

[Jeff King]

This adds a small module for examining parts of a commit
from inside a lua interpreter. Eventually you'll be able to
do grep-like filtering and --pretty formatting.

The most naive presentation would be to parse the whole
commit and put it in a lua table. However, instead we build
upon the incremental parsing used by the --format parser,
and lazily parse bits of the commit as the lua code requests
them.

Signed-off-by: Jeff King <peff@peff.net>
---
Set "USE_LUA" in your Makefile to turn it on.

 Makefile     |   7 +++
 lua-commit.c | 166 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lua-commit.h |   9 ++++
 3 files changed, 182 insertions(+)
 create mode 100644 lua-commit.c
 create mode 100644 lua-commit.h

diff --git a/Makefile b/Makefile
index a49d1db..54473e2 100644
--- a/Makefile
+++ b/Makefile
@@ -636,6 +636,7 @@ LIB_H += log-tree.h
 LIB_H += list-objects.h
 LIB_H += ll-merge.h
 LIB_H += log-tree.h
+LIB_H += lua-commit.h
 LIB_H += mailmap.h
 LIB_H += merge-file.h
 LIB_H += merge-recursive.h
@@ -749,6 +750,7 @@ LIB_OBJS += log-tree.o
 LIB_OBJS += ll-merge.o
 LIB_OBJS += lockfile.o
 LIB_OBJS += log-tree.o
+LIB_OBJS += lua-commit.o
 LIB_OBJS += mailmap.o
 LIB_OBJS += match-trees.o
 LIB_OBJS += merge-file.o
@@ -1818,6 +1820,11 @@ endif
        COMPAT_OBJS += compat/nedmalloc/nedmalloc.o
 endif
 
+ifdef USE_LUA
+	BASIC_CFLAGS += -DUSE_LUA `pkg-config --cflags lua5.2`
+	EXTLIBS += `pkg-config --libs lua5.2`
+endif
+
 ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT
 	export GIT_TEST_CMP_USE_COPIED_CONTEXT
 endif
diff --git a/lua-commit.c b/lua-commit.c
new file mode 100644
index 0000000..ce1eeeb
--- /dev/null
+++ b/lua-commit.c
@@ -0,0 +1,166 @@
+#include "cache.h"
+#include "lua-commit.h"
+#include "commit.h"
+
+#ifndef USE_LUA
+
+static const char msg[] = "git was built without lua support";
+
+void lua_commit_init(const char *)
+{
+	die(msg);
+}
+
+void lua_commit_format(struct strbuf *,
+		       struct format_commit_context *)
+{
+	die(msg);
+}
+
+#else
+
+#include <lua.h>
+#include <lauxlib.h>
+#include <lualib.h>
+
+static lua_State *lua;
+
+/* XXX
+ * We need to access this from functions called from inside lua. Probably it
+ * would be cleaner use a lua "register" to let each function access it, but I
+ * haven't looked into it.
+ */
+static struct format_commit_context *c;
+
+static int lua_fun_hash(lua_State *lua)
+{
+	lua_pushstring(lua, sha1_to_hex(c->commit->object.sha1));
+	return 1;
+}
+
+static int lua_fun_abbrev(lua_State *lua)
+{
+	const char *hex;
+	unsigned char sha1[20];
+
+	hex = lua_tostring(lua, -1);
+	if (!hex || get_sha1_hex(hex, sha1)) {
+		lua_pushstring(lua, "abbrev requires a sha1");
+		lua_error(lua);
+	}
+
+	lua_pushstring(lua, find_unique_abbrev(sha1, c->pretty_ctx->abbrev));
+	return 1;
+}
+
+static int get_ident(lua_State *lua, const char *line, int len)
+{
+	struct ident_split s;
+
+	if (split_ident_line(&s, line, len) < 0) {
+		lua_pushstring(lua, "unable to parse ident line");
+		lua_error(lua);
+	}
+
+	lua_createtable(lua, 0, 2);
+	lua_pushstring(lua, "name");
+	lua_pushlstring(lua, s.name_begin, s.name_end - s.name_begin);
+	lua_settable(lua, -3);
+	lua_pushstring(lua, "email");
+	lua_pushlstring(lua, s.mail_begin, s.mail_end - s.mail_begin);
+	lua_settable(lua, -3);
+
+	/* XXX should also put date in the table */
+
+	return 1;
+}
+
+static int lua_fun_author(lua_State *lua)
+{
+	if (!c->commit_header_parsed)
+		parse_commit_header(c);
+	return get_ident(lua, c->message + c->author.off, c->author.len);
+}
+
+static int lua_fun_committer(lua_State *lua)
+{
+	if (!c->commit_header_parsed)
+		parse_commit_header(c);
+	return get_ident(lua, c->message + c->committer.off, c->committer.len);
+}
+
+static int lua_fun_message(lua_State *lua)
+{
+	lua_pushstring(lua, c->message + c->message_off + 1);
+	return 1;
+}
+
+static int lua_fun_subject(lua_State *lua)
+{
+	struct strbuf tmp = STRBUF_INIT;
+
+	if (!c->commit_header_parsed)
+		parse_commit_header(c);
+	if (!c->commit_message_parsed)
+		parse_commit_message(c);
+
+	format_subject(&tmp, c->message + c->subject_off, " ");
+	lua_pushlstring(lua, tmp.buf, tmp.len);
+	return 1;
+}
+
+static int lua_fun_body(lua_State *lua)
+{
+	if (!c->commit_header_parsed)
+		parse_commit_header(c);
+	if (!c->commit_message_parsed)
+		parse_commit_message(c);
+
+	lua_pushstring(lua, c->message + c->body_off);
+	return 1;
+}
+
+void lua_commit_init(const char *snippet)
+{
+	if (!lua) {
+		lua = luaL_newstate();
+		if (!lua)
+			die("unable to open lua interpreter");
+		luaL_openlibs(lua);
+
+#define REG(name) do { \
+	lua_pushcfunction(lua, lua_fun_##name); \
+	lua_setglobal(lua, #name); \
+} while(0)
+
+		REG(hash);
+		REG(abbrev);
+		REG(author);
+		REG(committer);
+		REG(message);
+		REG(subject);
+		REG(body);
+	}
+
+	if (luaL_loadstring(lua, snippet))
+		die("unable to load lua snippet: %s", snippet);
+}
+
+void lua_commit_format(struct strbuf *out,
+		       struct format_commit_context *context)
+{
+	const char *ret;
+	size_t len;
+
+	c = context;
+
+	lua_pushvalue(lua, -1);
+	if (lua_pcall(lua, 0, 1, 0))
+		die("lua failed: %s", lua_tostring(lua, -1));
+
+	ret = lua_tolstring(lua, -1, &len);
+	strbuf_add(out, ret, len);
+	lua_pop(lua, 1);
+}
+
+#endif /* USE_LUA */
diff --git a/lua-commit.h b/lua-commit.h
new file mode 100644
index 0000000..aaffced
--- /dev/null
+++ b/lua-commit.h
@@ -0,0 +1,9 @@
+#ifndef LUA_COMMIT_H
+#define LUA_COMMIT_H
+
+struct format_commit_context;
+
+void lua_commit_init(const char *snippet);
+void lua_commit_format(struct strbuf *out, struct format_commit_context *context);
+
+#endif
-- 
1.7.12.1.10.g6537447

Re: [PATCH 2/3] add basic lua infrastructure

[Nguyen Thai Ngoc Duy]

On Tue, Sep 25, 2012 at 7:25 AM, Jeff King <peff@peff.net> wrote:
> +ifdef USE_LUA
> +       BASIC_CFLAGS += -DUSE_LUA `pkg-config --cflags lua5.2`
> +       EXTLIBS += `pkg-config --libs lua5.2`
> +endif
> +

I remember we paid noticeable penalty when linking with libcurl to
main git binary and Linus removed libcurl from main git, moving it to
git-http-*. Do we pay similar penalty linking to liblua?
-- 
Duy

Re: [PATCH 2/3] add basic lua infrastructure

[Jeff King]

On Tue, Sep 25, 2012 at 08:55:23AM +0700, Nguyen Thai Ngoc Duy wrote:

> On Tue, Sep 25, 2012 at 7:25 AM, Jeff King <peff@peff.net> wrote:
> > +ifdef USE_LUA
> > +       BASIC_CFLAGS += -DUSE_LUA `pkg-config --cflags lua5.2`
> > +       EXTLIBS += `pkg-config --libs lua5.2`
> > +endif
> > +
> 
> I remember we paid noticeable penalty when linking with libcurl to
> main git binary and Linus removed libcurl from main git, moving it to
> git-http-*. Do we pay similar penalty linking to liblua?

I don't think so. The real problem with libcurl is that it brings in a
ton of other libraries:

  $ ldd /usr/lib/x86_64-linux-gnu/libcurl.so | awk '{print $1}'
  linux-vdso.so.1
  libidn.so.11
  libssh2.so.1
  liblber-2.4.so.2
  libldap_r-2.4.so.2
  librt.so.1
  libgssapi_krb5.so.2
  libssl.so.1.0.0
  libcrypto.so.1.0.0
  librtmp.so.0
  libz.so.1
  libc.so.6
  libgcrypt.so.11
  libresolv.so.2
  libsasl2.so.2
  libgnutls.so.26
  libpthread.so.0
  /lib64/ld-linux-x86-64.so.2
  libkrb5.so.3
  libk5crypto.so.3
  libcom_err.so.2
  libkrb5support.so.0
  libdl.so.2
  libkeyutils.so.1
  libgpg-error.so.0
  libtasn1.so.3
  libp11-kit.so.0

Compare with lua:

  $ ldd /usr/lib/x86_64-linux-gnu/liblua5.2.so | awk '{print $1}'
  linux-vdso.so.1
  libm.so.6
  libdl.so.2
  libc.so.6
  /lib64/ld-linux-x86-64.so.2

The original timings from Linus are here:

  http://article.gmane.org/gmane.comp.version-control.git/123946

The main issue is really hitting all those libraries on a cold cache.
Here are before-and-after timings of:

  echo 3 >/proc/sys/vm/drop_caches && git

which should basically just measure startup time. All times are
best-of-five.

  [before]
  real    0m0.065s
  user    0m0.000s
  sys     0m0.004s

  [after]
  real    0m0.063s
  user    0m0.000s
  sys     0m0.004s

So we actually did better, though the difference is well within the
run-to-run noise. I don't think it's a big deal.

-Peff

Re: [PATCH 2/3] add basic lua infrastructure

[Robin H. Johnson]

On Mon, Sep 24, 2012 at 08:25:12PM -0400,  Jeff King wrote:
> +ifdef USE_LUA
> +	BASIC_CFLAGS += -DUSE_LUA `pkg-config --cflags lua5.2`
> +	EXTLIBS += `pkg-config --libs lua5.2`
> +endif
Can you please hoist the packagename out to a variable? It's just plain
"lua" on Gentoo.

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85

Re: [PATCH 2/3] add basic lua infrastructure

[Jeff King]

On Tue, Sep 25, 2012 at 03:21:10AM +0000, Robin H. Johnson wrote:

> On Mon, Sep 24, 2012 at 08:25:12PM -0400,  Jeff King wrote:
> > +ifdef USE_LUA
> > +	BASIC_CFLAGS += -DUSE_LUA `pkg-config --cflags lua5.2`
> > +	EXTLIBS += `pkg-config --libs lua5.2`
> > +endif
> Can you please hoist the packagename out to a variable? It's just plain
> "lua" on Gentoo.

Yeah. I mentioned these patches were very rough, but I didn't go into
detail on all the bad points.  That is definitely one of them. I have no
idea what the "normal" name is; my debian system sticks the version
number in to allow multiple concurrent versions.

I was hoping somebody with more Lua experience could tell me what's
usual. It would be nice if it just worked out of the box as soon as you
said USE_LUA, but that may not be realistic.

-Peff

Re: [PATCH 2/3] add basic lua infrastructure

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Sep 25, 2012 at 03:21:10AM +0000, Robin H. Johnson wrote:
>
>> On Mon, Sep 24, 2012 at 08:25:12PM -0400,  Jeff King wrote:
>> > +ifdef USE_LUA
>> > +	BASIC_CFLAGS += -DUSE_LUA `pkg-config --cflags lua5.2`
>> > +	EXTLIBS += `pkg-config --libs lua5.2`
>> > +endif
>> Can you please hoist the packagename out to a variable? It's just plain
>> "lua" on Gentoo.
>
> Yeah. I mentioned these patches were very rough, but I didn't go into
> detail on all the bad points.  That is definitely one of them. I have no
> idea what the "normal" name is; my debian system sticks the version
> number in to allow multiple concurrent versions.

Yeah, there is no point nitpicking yet.  Even the choice of lua is
not all that interesting; embedding _any_ reasonable interpreter,
and figuring out which operations and codepaths in us benefit most
from such embedding, are of bigger interest at this early stage.

How about doing this on top at the minimum?  You can let pkg-config
to tell you where -I<directory> and what -l<lib> is, or you can set
it yourself.

    $ make USE_LUA=YesPlease \
    	LUA_INCLUDE_ARG=-I/usr/include/lua5.2 \
        LUA_LINK_ARG=-llua5.2

or

    $ make USE_LUA=lua5.2


 Makefile | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git i/Makefile w/Makefile
index 620df89..90335ba 100644
--- i/Makefile
+++ w/Makefile
@@ -1898,8 +1898,14 @@ ifdef USE_NED_ALLOCATOR
 endif
 
 ifdef USE_LUA
-	BASIC_CFLAGS += -DUSE_LUA `pkg-config --cflags lua5.2`
-	EXTLIBS += `pkg-config --libs lua5.2`
+	# You can say
+	# $ make USE_LUA=YesPlease LUA_INCLUDE_ARG=-I/usr/include
+	# or
+	# $ make USE_LUA=lua5.2
+	LUA_INCLUDE_ARG ?= $(shell pkg-config --cflags $(USE_LUA))
+	LUA_LINK_ARG ?= $(shell pkg-config --libs $(USE_LUA))
+	BASIC_CFLAGS += -DUSE_LUA $(LUA_INCLUDE_ARG)
+	EXTLIBS += $(LUA_LINK_ARG)
 endif
 
 ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT

[PATCH 3/3] add a "lua" pretty format

[Jeff King]

With this patch, you can do:

  git log --pretty=lua:'
    return abbrev(hash()) .. " (" .. author().email .. ") " .. subject()
  '

Signed-off-by: Jeff King <peff@peff.net>
---
 commit.h   |  1 +
 log-tree.c |  3 ++-
 pretty.c   | 21 +++++++++++++++++++--
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/commit.h b/commit.h
index 71cd4af..8865a36 100644
--- a/commit.h
+++ b/commit.h
@@ -73,6 +73,7 @@ enum cmit_fmt {
 	CMIT_FMT_ONELINE,
 	CMIT_FMT_EMAIL,
 	CMIT_FMT_USERFORMAT,
+	CMIT_FMT_LUA,
 
 	CMIT_FMT_UNSPECIFIED
 };
diff --git a/log-tree.c b/log-tree.c
index c894930..c8274d1 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -599,7 +599,8 @@ void show_log(struct rev_info *opt)
 	if (opt->commit_format == CMIT_FMT_EMAIL) {
 		log_write_email_headers(opt, commit, &ctx.subject, &extra_headers,
 					&ctx.need_8bit_cte);
-	} else if (opt->commit_format != CMIT_FMT_USERFORMAT) {
+	} else if (opt->commit_format != CMIT_FMT_USERFORMAT &&
+		   opt->commit_format != CMIT_FMT_LUA) {
 		fputs(diff_get_color_opt(&opt->diffopt, DIFF_COMMIT), stdout);
 		if (opt->commit_format != CMIT_FMT_ONELINE)
 			fputs("commit ", stdout);
diff --git a/pretty.c b/pretty.c
index 0d4eb3d..fdd4258 100644
--- a/pretty.c
+++ b/pretty.c
@@ -10,6 +10,7 @@
 #include "color.h"
 #include "reflog-walk.h"
 #include "gpg-interface.h"
+#include "lua-commit.h"
 
 static char *user_format;
 static struct cmt_fmt_map {
@@ -33,6 +34,13 @@ static void save_user_format(struct rev_info *rev, const char *cp, int is_tforma
 	rev->commit_format = CMIT_FMT_USERFORMAT;
 }
 
+static void save_lua_format(struct rev_info *rev, const char *cp, int is_tformat)
+{
+	lua_commit_init(cp);
+	save_user_format(rev, cp, is_tformat);
+	rev->commit_format = CMIT_FMT_LUA;
+}
+
 static int git_pretty_formats_config(const char *var, const char *value, void *cb)
 {
 	struct cmt_fmt_map *commit_format = NULL;
@@ -155,6 +163,10 @@ void get_commit_format(const char *arg, struct rev_info *rev)
 		save_user_format(rev, strchr(arg, ':') + 1, arg[0] == 't');
 		return;
 	}
+	if (!prefixcmp(arg, "lua:")) {
+		save_lua_format(rev, arg + 4, 1);
+		return;
+	}
 
 	if (strchr(arg, '%')) {
 		save_user_format(rev, arg, 1);
@@ -1168,7 +1180,11 @@ void format_commit_message(const struct commit *commit,
 		free(enc);
 	}
 
-	strbuf_expand(sb, format, format_commit_item, &context);
+	if (pretty_ctx->fmt == CMIT_FMT_USERFORMAT)
+		strbuf_expand(sb, format, format_commit_item, &context);
+	else if (pretty_ctx->fmt == CMIT_FMT_LUA)
+		lua_commit_format(sb, &context);
+
 	rewrap_message_tail(sb, &context, 0, 0, 0);
 
 	if (context.message != commit->buffer)
@@ -1328,7 +1344,8 @@ void pretty_print_commit(const struct pretty_print_context *pp,
 	const char *encoding;
 	int need_8bit_cte = pp->need_8bit_cte;
 
-	if (pp->fmt == CMIT_FMT_USERFORMAT) {
+	if (pp->fmt == CMIT_FMT_USERFORMAT ||
+	    pp->fmt == CMIT_FMT_LUA) {
 		format_commit_message(commit, user_format, sb, pp);
 		return;
 	}
-- 
1.7.12.1.10.g6537447

Re: [PATCH 3/3] add a "lua" pretty format

[Jeff King]

On Mon, Sep 24, 2012 at 08:25:39PM -0400, Jeff King wrote:

> @@ -1168,7 +1180,11 @@ void format_commit_message(const struct commit *commit,
>  		free(enc);
>  	}
>  
> -	strbuf_expand(sb, format, format_commit_item, &context);
> +	if (pretty_ctx->fmt == CMIT_FMT_USERFORMAT)
> +		strbuf_expand(sb, format, format_commit_item, &context);
> +	else if (pretty_ctx->fmt == CMIT_FMT_LUA)
> +		lua_commit_format(sb, &context);
> +

This hunk breaks lots of tests. I know we are not seriously considering
the lua series as-is, but in case anybody wants to play with it, here is
the fix (and we would need the same fix regardless of language, anyway).

You might want to queue this on jk/lua-hackery (probably it would be
squashed in for a real series).

-- >8 --
Subject: [PATCH] pretty: fix up one-off format_commit_message calls

If the usual pretty-print code invokes format_commit_message,
the "fmt" field of the pretty_print_context will always have
either CMIT_FMT_USERFORMAT or CMIT_FMT_LUA in it, and we can
just choose which to use.

However, many call sites invoke format_commit_message
directly without bothering to set the "fmt" field of the
context; they expect format_commit_message to just default
to CMIT_FMT_USERFORMAT in that case, since previously that
was the only format it handled.

The recent addition of the lua formatter broke that
assumption. Rather than require each caller to be more
strict, let's just default to USERFORMAT when the format is
set to something nonsensical.

Signed-off-by: Jeff King <peff@peff.net>
---
 pretty.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/pretty.c b/pretty.c
index fdd4258..7289590 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1180,10 +1180,10 @@ void format_commit_message(const struct commit *commit,
 		free(enc);
 	}
 
-	if (pretty_ctx->fmt == CMIT_FMT_USERFORMAT)
-		strbuf_expand(sb, format, format_commit_item, &context);
-	else if (pretty_ctx->fmt == CMIT_FMT_LUA)
+	if (pretty_ctx->fmt == CMIT_FMT_LUA)
 		lua_commit_format(sb, &context);
+	else
+		strbuf_expand(sb, format, format_commit_item, &context);
 
 	rewrap_message_tail(sb, &context, 0, 0, 0);
 
-- 
1.8.0.rc0.22.g285fd2d

Re: [RFC/PATCH 0/3] git log --pretty=lua

[Nguyen Thai Ngoc Duy]

On Tue, Sep 25, 2012 at 7:23 AM, Jeff King <peff@peff.net> wrote:
> We've talked off and on about extending the --pretty=format specifiers
> to something more flexible. There's also been talk recently of more
> flexible commit-filtering (e.g., grepping individual notes).  Rather
> than invent a new Turing-complete language, I thought I'd try building
> on somebody else's work by embedding an existing language.
>
> Why Lua? I don't especially like it as a language. But it's designed for
> this purpose, which makes it very lightweight and relatively simple to
> embed.

Personally I'd prefer a Scheme variant.

> The syntax, on the other hand...yuck.

Oops. Scheme is out then.

> One thing that makes Lua
> horrible for this use is that it does not have interpolated strings.
> However, there are template libraries for Lua, so maybe there's
> something there.

For --pretty, the first thing I looked up was utf-8 support and Lua
does not seem to have that built in. Libraries can help but it'll be
more verbose than native language support.

> And a "4/3" patch would probably add "--lua-filter" as a revision option
> for limiting commits.

I was thinking of nearly the same thing, except that I hide the
filters behind sha-1 extended syntax. Users can link <sha-1>@{foo} to
a lua function, for example.

I wonder what areas in git might benefit from such a scripting
language, and whether someday we would convert some of git builtin
commands to $NEWLANG, if $NEWLANG proves easier to maintain for
complex logic commands. grep and rev-list (searching in general) are
probably where $NEWLANG shines. But for really complex searches, one
may want to go with libgit2 or other bindings to their favourite
language than one git may provide. gitignore and gitattributes can
make use of $NEWLANG but it has to be really fast. There was talk
about conditionals in config file, which $NEWLANG might also help.

In general I'm quite happy with what git provides. There are many
advanced features that I have never used/or even aware of.
-- 
Duy

Re: [RFC/PATCH 0/3] git log --pretty=lua

[Stephen Bash]

----- Original Message -----
> From: "Nguyen Thai Ngoc Duy" <pclouds@gmail.com>
> Sent: Tuesday, September 25, 2012 7:22:49 AM
> Subject: Re: [RFC/PATCH 0/3] git log --pretty=lua
> 
> On Tue, Sep 25, 2012 at 7:23 AM, Jeff King <peff@peff.net> wrote:
> > We've talked off and on about extending the --pretty=format
> > specifiers to something more flexible. There's also been talk
> > recently of more flexible commit-filtering (e.g., grepping
> > individual notes).  Rather than invent a new Turing-complete
> > language, I thought I'd try building on somebody else's work by
> > embedding an existing language.
> >
> > Why Lua? I don't especially like it as a language. But it's designed
> > for this purpose, which makes it very lightweight and relatively
> > simple to embed.
> 
> Personally I'd prefer a Scheme variant.

Scheme only brings up bad memories for me ;)  And while we use Lua at
$dayjob, I, like Peff, am not a huge fan of the syntax.  So turning to
the internet to solve my problem, a quick Google search for embeddable
scripting languages (assuming the heavyweights like Perl and Python are
already out) produces Lua, JavaScript or specifically SpiderMonkey [1]
(yay buzzword compliance!), Ch [2] (unfortunately closed source), and
AngelScript [3].

>From a brief read of the webpage, AngelScript looks pretty interesting.
I'm much better with (and thus preferential to) Python and Perl myself,
but I can understand anyone's reservation in bundling/depending on
libraries of that size.

[1] https://developer.mozilla.org/en-US/docs/SpiderMonkey
[2] http://www.softintegration.com/
[3] http://angelcode.com/angelscript/

Thanks,
Stephen

Re: [RFC/PATCH 0/3] git log --pretty=lua

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> We've talked off and on about extending the --pretty=format specifiers
> to something more flexible. There's also been talk recently of more
> flexible commit-filtering (e.g., grepping individual notes).

Mercurial has a similar thing, which can be a source of inspiration:

http://www.selenic.com/hg/help/revsets

On the one hand, if find it a bit overkill to have a full language for
this, but on the other hand, it allows expressing easily and explicitely
boolean operators.

I would find

  git log 'grep(foo) or grep(bar)'

very intuitive and elegant, while I never know whether

  git log --grep=foo --grep=bar

is a OR or a AND (there was a patch recently to clarify the doc).

Note that Mercurial's version seem to really act on sets of commits, not
just individual commits, as it allows things like

  hg log -r 'sort(date("May 2008"), user)'

or the equivalent of git's negative revision argument :

  "x - y"
      Changesets in x but not in y.

So it would be more a new "git log --filter='some expression'", not a
--pretty=lua.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [RFC/PATCH 0/3] git log --pretty=lua

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Jeff King <peff@peff.net> writes:
>
>> We've talked off and on about extending the --pretty=format specifiers
>> to something more flexible. There's also been talk recently of more
>> flexible commit-filtering (e.g., grepping individual notes).
>
> Mercurial has a similar thing, which can be a source of inspiration:
>
> http://www.selenic.com/hg/help/revsets
>
> On the one hand, if find it a bit overkill to have a full language for
> this, but on the other hand, it allows expressing easily and explicitely
> boolean operators.
>
> I would find
>
>   git log 'grep(foo) or grep(bar)'
>
> very intuitive and elegant,...

You have to be careful with "grep", though.  It would be unclear
what "and" there would mean if you replaced your "or" with.

Peff's earlier examples e.g.

  git log --lua-filter='
    return
      author().name.match("Junio") &&
      note("p4").match("1234567")
  '

  git log --lua-filter='return subject().len > 100'

are clearly good ones that illustrate the power of filtering.

Re: [RFC/PATCH 0/3] git log --pretty=lua

[Nguyen Thai Ngoc Duy]

On Tue, Sep 25, 2012 at 7:23 AM, Jeff King <peff@peff.net> wrote:
> Why Lua? I don't especially like it as a language. But it's designed for
> this purpose, which makes it very lightweight and relatively simple to
> embed.

Another option is tcl. String-based approach feels more natural for
pretty.c and shell users. >=8.1 supports utf-8. We might be able to
share some routines between the embedded tcl interpreter and
gitk/git-gui. The drawback is we won't get the speed of luajit if we
ever need to (I'm looking at filter-branch, but I assume eliminating
fork() should already give decent speedup).
-- 
Duy