[PATCH 00/12] Support columinized output in tag/branch/ls-files/grep

[PATCH 00/12] Support columinized output in tag/branch/ls-files/grep

[Nguyễn Thái Ngọc Duy]

This adds support to show columnized output in the mentioned commands.
I am quite happy with columnized "git branch/tag".

There is still room for improvement. For example, if "git column" is going to
display all items in one or two lines (too wide screen), then falling back to
one column mode would be better.

I'm also experimenting aligned grep output.  While it's useful
sometimes to look for things near the beginning of line, I don't know
if it's really worth the effort.

Anyway, enjoy the new output (Hmm.. "git config -l" may be applicable for
columnization too)

Nguyễn Thái Ngọc Duy (12):
  Move term_columns() to pager.c
  setup_pager(): save terminal width before redirecting stdout
  Add columnizer
  help: use columnizer
  Add builtin command "column"
  Add helpers to redirect stdout to "git column"
  add core.columns
  tag: support column output with --columns
  branch: support column output with --columns
  ls-files: support column output with --columns
  grep: do not return early in cmd_grep() if there is no error
  grep: support column output with --columns

 .gitignore                   |    1 +
 Documentation/config.txt     |    8 ++
 Documentation/git-branch.txt |    8 ++
 Documentation/git-column.txt |   40 ++++++++++
 Documentation/git-grep.txt   |   10 +++
 Documentation/git-tag.txt    |   11 +++-
 Makefile                     |    3 +
 builtin.h                    |    1 +
 builtin/branch.c             |   18 ++++-
 builtin/column.c             |   42 +++++++++++
 builtin/grep.c               |   48 +++++++------
 builtin/ls-files.c           |   10 +++
 builtin/tag.c                |   17 ++++-
 cache.h                      |    2 +
 column.c                     |  164 ++++++++++++++++++++++++++++++++++++++++++
 column.h                     |   25 +++++++
 command-list.txt             |    1 +
 config.c                     |    6 ++
 environment.c                |    1 +
 git.c                        |    1 +
 help.c                       |   55 +++-----------
 pager.c                      |   95 ++++++++++++++++++++++++
 t/t9002-column.sh            |  101 ++++++++++++++++++++++++++
 23 files changed, 595 insertions(+), 73 deletions(-)
 create mode 100644 Documentation/git-column.txt
 create mode 100644 builtin/column.c
 create mode 100644 column.c
 create mode 100644 column.h
 create mode 100755 t/t9002-column.sh

[PATCH 01/12] Move term_columns() to pager.c

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 cache.h |    1 +
 help.c  |   22 ----------------------
 pager.c |   23 +++++++++++++++++++++++
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/cache.h b/cache.h
index 89f6a40..124f100 100644
--- a/cache.h
+++ b/cache.h
@@ -985,6 +985,7 @@ extern void setup_pager(void);
 extern const char *pager_program;
 extern int pager_in_use(void);
 extern int pager_use_color;
+extern int term_columns();
 
 extern const char *editor_program;
 extern const char *excludes_file;
diff --git a/help.c b/help.c
index 7f4928e..888bf8c 100644
--- a/help.c
+++ b/help.c
@@ -4,28 +4,6 @@
 #include "levenshtein.h"
 #include "help.h"
 
-/* most GUI terminals set COLUMNS (although some don't export it) */
-static int term_columns(void)
-{
-	char *col_string = getenv("COLUMNS");
-	int n_cols;
-
-	if (col_string && (n_cols = atoi(col_string)) > 0)
-		return n_cols;
-
-#ifdef TIOCGWINSZ
-	{
-		struct winsize ws;
-		if (!ioctl(1, TIOCGWINSZ, &ws)) {
-			if (ws.ws_col)
-				return ws.ws_col;
-		}
-	}
-#endif
-
-	return 80;
-}
-
 void add_cmdname(struct cmdnames *cmds, const char *name, int len)
 {
 	struct cmdname *ent = xmalloc(sizeof(*ent) + len + 1);
diff --git a/pager.c b/pager.c
index dac358f..fc6c3e0 100644
--- a/pager.c
+++ b/pager.c
@@ -12,6 +12,7 @@
  */
 
 static int spawned_pager;
+static int max_columns;
 
 #ifndef WIN32
 static void pager_preexec(void)
@@ -116,3 +117,25 @@ int pager_in_use(void)
 	env = getenv("GIT_PAGER_IN_USE");
 	return env ? git_config_bool("GIT_PAGER_IN_USE", env) : 0;
 }
+
+int term_columns()
+{
+	char *col_string = getenv("COLUMNS");
+	int n_cols;
+
+	if (col_string && (n_cols = atoi(col_string)) > 0)
+		return n_cols;
+
+	else if (spawned_pager && max_columns)
+		return max_columns;
+#ifdef TIOCGWINSZ
+	else {
+		struct winsize ws;
+		if (!ioctl(1, TIOCGWINSZ, &ws)) {
+			if (ws.ws_col)
+				return ws.ws_col;
+		}
+	}
+#endif
+	return 80;
+}
-- 
1.7.0.1.370.gd3c5

[PATCH 02/12] setup_pager(): save terminal width before redirecting stdout

[Nguyễn Thái Ngọc Duy]

After redirecting, we may no longer read terminal width.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 pager.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/pager.c b/pager.c
index fc6c3e0..ad447cf 100644
--- a/pager.c
+++ b/pager.c
@@ -81,6 +81,15 @@ void setup_pager(void)
 
 	spawned_pager = 1; /* means we are emitting to terminal */
 
+#ifdef TIOCGWINSZ
+	{
+		struct winsize ws;
+		if (!ioctl(1, TIOCGWINSZ, &ws)) {
+			if (ws.ws_col)
+				max_columns = ws.ws_col;
+		}
+	}
+#endif
 	/* spawn the pager */
 	pager_argv[0] = pager;
 	pager_process.use_shell = 1;
-- 
1.7.0.1.370.gd3c5

[PATCH 03/12] Add columnizer

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 .gitignore       |    1 +
 Makefile         |    2 +
 column.c         |  164 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 column.h         |   23 ++++++++
 command-list.txt |    1 +
 5 files changed, 191 insertions(+), 0 deletions(-)
 create mode 100644 column.c
 create mode 100644 column.h

diff --git a/.gitignore b/.gitignore
index 7b3acb7..8e087e6 100644
--- a/.gitignore
+++ b/.gitignore
@@ -25,6 +25,7 @@
 /git-cherry-pick
 /git-clean
 /git-clone
+/git-column
 /git-commit
 /git-commit-tree
 /git-config
diff --git a/Makefile b/Makefile
index 7358a20..337f5bf 100644
--- a/Makefile
+++ b/Makefile
@@ -454,6 +454,7 @@ LIB_H += builtin.h
 LIB_H += cache.h
 LIB_H += cache-tree.h
 LIB_H += color.h
+LIB_H += column.h
 LIB_H += commit.h
 LIB_H += compat/bswap.h
 LIB_H += compat/cygwin.h
@@ -526,6 +527,7 @@ LIB_OBJS += branch.o
 LIB_OBJS += bundle.o
 LIB_OBJS += cache-tree.o
 LIB_OBJS += color.o
+LIB_OBJS += column.o
 LIB_OBJS += combine-diff.o
 LIB_OBJS += commit.o
 LIB_OBJS += config.o
diff --git a/column.c b/column.c
new file mode 100644
index 0000000..0e88763
--- /dev/null
+++ b/column.c
@@ -0,0 +1,164 @@
+#include "cache.h"
+#include "column.h"
+
+static int ansi_length(const char *s)
+{
+	int a_len = 0;
+
+	while ((s = strstr(s, "\033[")) != NULL) {
+		int len = strspn(s+2, "0123456789;");
+		s += len+3; /* \033[<len><func char> */
+		a_len += len+3;
+	}
+	return a_len;
+}
+
+static void print_row(struct columnizer *cp, int row)
+{
+	int mode = cp->flags & COLUMNIZER_MODE_MASK;
+	int j;
+
+	if (cp->left_space)
+		printf("%-*s", cp->left_space, "");
+
+	for (j = 0; j < cp->nr_cols; j++) {
+		int n, size;
+
+		switch (mode) {
+		case COLUMNIZER_COLUMN_FIRST:
+			n = j * cp->nr_rows + row;
+			size = cp->column_width;
+			break;
+		case COLUMNIZER_ROW_FIRST:
+			n = row * cp->nr_cols + j;
+			size = cp->column_width;
+			break;
+		}
+
+		if (n >= cp->cells_nr)
+			break;
+
+		size += cp->space;
+		if (cp->flags & COLUMNIZER_HAVE_ANSI)
+			size += ansi_length(cp->cells[n]);
+		if (j == cp->nr_cols-1)
+			size = 1;
+		printf("%-*s", size, cp->cells[n]);
+	}
+	putchar('\n');
+}
+
+static void calculate_column_width(struct columnizer *cp)
+{
+	int max_cols, longest;
+
+	if (cp->flags & COLUMNIZER_READY)
+		return;
+
+	if (!cp->terminal_width)
+		cp->terminal_width = term_columns();
+
+	if (cp->column_width)
+		longest = cp->column_width;
+	else {
+		int i, len;
+		longest = 0;
+		for (i = 0; i < cp->cells_nr; i++) {
+			len = strlen(cp->cells[i]);
+			if (cp->flags & COLUMNIZER_HAVE_ANSI)
+				len -= ansi_length(cp->cells[i]);
+			if (longest < len)
+				longest = len;
+		}
+	}
+
+	max_cols = cp->terminal_width - 1; /* don't print *on* the edge */
+	max_cols -= cp->left_space + cp->right_space;
+
+	/* we don't have space after the last column */
+	max_cols += cp->space;
+
+	cp->nr_cols = (longest + cp->space) < max_cols ? max_cols / (longest + cp->space) : 1;
+	cp->nr_rows = DIV_ROUND_UP(cp->cells_nr, cp->nr_cols);
+	if (!cp->column_width)
+		cp->column_width = max_cols / cp->nr_cols - cp->space;
+	cp->flags |= COLUMNIZER_READY;
+}
+
+static int feed_cell(struct columnizer *cp, char *cell)
+{
+	int len;
+
+	/* Non-homogeneous column is not supported yet */
+	if (!(cp->flags & COLUMNIZER_HOMOGENEOUS))
+		return -1;
+
+	if (!cell) {		/* EOF */
+		int i;
+
+		calculate_column_width(cp);
+		for (i = 0;i < cp->nr_rows; i++)
+			print_row(cp, i);
+		return 0;
+	}
+
+	cp->cells_nr++;
+	ALLOC_GROW(cp->cells, cp->cells_nr, cp->cells_alloc);
+	cp->cells[cp->cells_nr-1] = cell;
+
+	len = strlen(cell);
+	if (cp->flags & COLUMNIZER_HAVE_ANSI)
+		len -= ansi_length(cell);
+
+	if (!cp->terminal_width)
+		cp->terminal_width = term_columns();
+	if (cp->longest < len)
+		cp->longest = len;
+
+	/* Can't have more than one column? */
+	if (cp->terminal_width < cp->left_space+cp->longest*2+cp->space+cp->right_space) {
+		int i;
+		for (i = 0;i < cp->cells_nr;i++) {
+			if (cp->left_space)
+				printf("%-*s", cp->left_space, "");
+			printf("%s\n", cp->cells[i]);
+			if (cp->detach)
+				cp->detach(cp->cells[i]);
+		}
+		free(cp->cells);
+		cp->cells_alloc = cp->cells_nr = 0;
+
+		cp->nr_cols = 1;
+		cp->flags |= COLUMNIZER_READY;
+	}
+	return 0;
+}
+
+int feed_columnizer(struct columnizer *cp, char *cell)
+{
+	int ret;
+
+	/* Degradation case, no special layout needed */
+	if (cp->flags & COLUMNIZER_READY && cp->nr_cols == 1) {
+		if (cell) {
+			if (cp->left_space)
+				printf("%-*s", cp->left_space, "");
+			printf("%s\n", cell);
+		}
+		ret = 0;
+	}
+	else
+		ret = feed_cell(cp, cell);
+
+	if (!cell) {		/* EOF, cleanup */
+		if (cp->detach) {
+			int i;
+			for (i = 0;i < cp->cells_nr;i++)
+				cp->detach(cp->cells[i]);
+		}
+		if (cp->cells_alloc)
+			free(cp->cells);
+	}
+
+	return ret;
+}
diff --git a/column.h b/column.h
new file mode 100644
index 0000000..28122e0
--- /dev/null
+++ b/column.h
@@ -0,0 +1,23 @@
+#define COLUMNIZER_MODE_MASK    0x000F
+#define COLUMNIZER_COLUMN_FIRST      0
+#define COLUMNIZER_ROW_FIRST         1
+
+#define COLUMNIZER_HOMOGENEOUS  0x0010
+#define COLUMNIZER_HAVE_ANSI    0x0020
+#define COLUMNIZER_READY        0x0040
+
+struct columnizer {
+	int flags;
+	int terminal_width, column_width;
+	int left_space, right_space, space;
+	void (*detach)(void *p);
+
+	/* private variables */
+	char **cells;
+	int cells_nr, cells_alloc;
+	int nr_cols, nr_rows;
+
+	int longest;
+};
+
+extern int feed_columnizer(struct columnizer *cp, char *cell);
diff --git a/command-list.txt b/command-list.txt
index 95bf18c..5d6bfc8 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -20,6 +20,7 @@ git-cherry-pick                         mainporcelain
 git-citool                              mainporcelain
 git-clean                               mainporcelain
 git-clone                               mainporcelain common
+git-column                              purehelpers
 git-commit                              mainporcelain common
 git-commit-tree                         plumbingmanipulators
 git-config                              ancillarymanipulators
-- 
1.7.0.1.370.gd3c5

[PATCH 04/12] help: use columnizer

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 help.c |   33 +++++++++++----------------------
 1 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/help.c b/help.c
index 888bf8c..e039c14 100644
--- a/help.c
+++ b/help.c
@@ -3,6 +3,7 @@
 #include "exec_cmd.h"
 #include "levenshtein.h"
 #include "help.h"
+#include "column.h"
 
 void add_cmdname(struct cmdnames *cmds, const char *name, int len)
 {
@@ -71,29 +72,17 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
 
 static void pretty_print_string_list(struct cmdnames *cmds, int longest)
 {
-	int cols = 1, rows;
-	int space = longest + 1; /* min 1 SP between words */
-	int max_cols = term_columns() - 1; /* don't print *on* the edge */
-	int i, j;
-
-	if (space < max_cols)
-		cols = max_cols / space;
-	rows = DIV_ROUND_UP(cmds->cnt, cols);
+	struct columnizer cp;
+	int i;
 
-	for (i = 0; i < rows; i++) {
-		printf("  ");
+	memset(&cp, 0, sizeof(cp));
+	cp.flags |= COLUMNIZER_HOMOGENEOUS | COLUMNIZER_COLUMN_FIRST;
+	cp.space = 1;
+	cp.left_space = 2;
 
-		for (j = 0; j < cols; j++) {
-			int n = j * rows + i;
-			int size = space;
-			if (n >= cmds->cnt)
-				break;
-			if (j == cols-1 || n + rows >= cmds->cnt)
-				size = 1;
-			printf("%-*s", size, cmds->names[n]->name);
-		}
-		putchar('\n');
-	}
+	for (i = 0; i < cmds->cnt; i++)
+		feed_columnizer(&cp, cmds->names[i]->name);
+	feed_columnizer(&cp, NULL);
 }
 
 static int is_executable(const char *name)
-- 
1.7.0.1.370.gd3c5

[PATCH 05/12] Add builtin command "column"

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-column.txt |   40 ++++++++++++++++
 Makefile                     |    1 +
 builtin.h                    |    1 +
 builtin/column.c             |   42 +++++++++++++++++
 git.c                        |    1 +
 t/t9002-column.sh            |  101 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 186 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/git-column.txt
 create mode 100644 builtin/column.c
 create mode 100755 t/t9002-column.sh

diff --git a/Documentation/git-column.txt b/Documentation/git-column.txt
new file mode 100644
index 0000000..3ee887f
--- /dev/null
+++ b/Documentation/git-column.txt
@@ -0,0 +1,40 @@
+git-column(1)
+=============
+
+NAME
+----
+git-column - Columnate lists
+
+SYNOPSIS
+--------
+[verse]
+'git column' [--mode=<mode>] [--width=<width>] [--left-space=<N>] [--right-space=<N>]
+
+DESCRIPTION
+-----------
+This command formats its input into multiple columns. Rows are filled before
+columns.
+
+OPTIONS
+-------
+--width=<width>::
+	Specify the terminal width. By default 'git column' will detect the
+	terminal width, or fall back to 80 if it is unable to do so.
+
+--left-space=<N>::
+	Prepend a number of spaces on the first column.
+
+--right-space=<N>::
+	Append a number of spaces after the last column.
+
+--mode=<mode>::
+	Which layout mode to use. Supported modes are 'row' (default mode,
+	fill rows first) and 'column' (fill columns first)
+
+Author
+------
+Written by Nguyen Thai Ngoc Duy <pclouds@gmail.com>
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 337f5bf..53686a2 100644
--- a/Makefile
+++ b/Makefile
@@ -640,6 +640,7 @@ BUILTIN_OBJS += builtin/checkout-index.o
 BUILTIN_OBJS += builtin/checkout.o
 BUILTIN_OBJS += builtin/clean.o
 BUILTIN_OBJS += builtin/clone.o
+BUILTIN_OBJS += builtin/column.o
 BUILTIN_OBJS += builtin/commit-tree.o
 BUILTIN_OBJS += builtin/commit.o
 BUILTIN_OBJS += builtin/config.o
diff --git a/builtin.h b/builtin.h
index cdf9847..c68c758 100644
--- a/builtin.h
+++ b/builtin.h
@@ -39,6 +39,7 @@ extern int cmd_cherry(int argc, const char **argv, const char *prefix);
 extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);
 extern int cmd_clone(int argc, const char **argv, const char *prefix);
 extern int cmd_clean(int argc, const char **argv, const char *prefix);
+extern int cmd_column(int argc, const char **argv, const char *prefix);
 extern int cmd_commit(int argc, const char **argv, const char *prefix);
 extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_count_objects(int argc, const char **argv, const char *prefix);
diff --git a/builtin/column.c b/builtin/column.c
new file mode 100644
index 0000000..d5d7f80
--- /dev/null
+++ b/builtin/column.c
@@ -0,0 +1,42 @@
+#include "cache.h"
+#include "strbuf.h"
+#include "parse-options.h"
+#include "column.h"
+
+static const char * const builtin_column_usage[] = {
+	"git column [--mode=<mode>] [--width=<width>] [--left-space=<N>] [--right-space=<N>]",
+	NULL
+};
+
+int cmd_column(int argc, const char **argv, const char *prefix)
+{
+	struct columnizer cp;
+	const char *mode = NULL;
+	struct strbuf sb = STRBUF_INIT;
+	struct option options[] = {
+		OPT_INTEGER(0, "width", &cp.terminal_width, "Maximum width"),
+		OPT_INTEGER(0, "left-space", &cp.left_space, "Padding space on left border"),
+		OPT_INTEGER(0, "right-space", &cp.right_space, "Padding space on right border"),
+		OPT_STRING(0, "mode", &mode, "mode", "Which layout mode to use"),
+		OPT_END()
+	};
+
+	memset(&cp, 0, sizeof(cp));
+	cp.space = 1;
+	cp.detach = free;
+
+	argc = parse_options(argc, argv, prefix, options, builtin_column_usage, 0);
+
+	if (!mode || !strcmp(mode, "column"))
+		cp.flags = COLUMNIZER_COLUMN_FIRST | COLUMNIZER_HAVE_ANSI | COLUMNIZER_HOMOGENEOUS;
+	else if (!strcmp(mode, "row"))
+		cp.flags = COLUMNIZER_ROW_FIRST | COLUMNIZER_HAVE_ANSI | COLUMNIZER_HOMOGENEOUS;
+	else
+		die("Invalid mode '%s'", mode);
+
+	while (!strbuf_getline(&sb, stdin, '\n'))
+		feed_columnizer(&cp, strbuf_detach(&sb, NULL));
+
+	feed_columnizer(&cp, NULL);
+	return 0;
+}
diff --git a/git.c b/git.c
index 6bae305..8dc6fdd 100644
--- a/git.c
+++ b/git.c
@@ -301,6 +301,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
 		{ "clone", cmd_clone },
 		{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
+		{ "column", cmd_column },
 		{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
 		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
 		{ "config", cmd_config },
diff --git a/t/t9002-column.sh b/t/t9002-column.sh
new file mode 100755
index 0000000..b5b46fc
--- /dev/null
+++ b/t/t9002-column.sh
@@ -0,0 +1,101 @@
+#!/bin/sh
+
+test_description='git column'
+. ./test-lib.sh
+
+cat >lista <<\EOF
+one
+two
+three
+four
+five
+six
+seven
+eight
+nine
+ten
+EOF
+
+cat >expected <<\EOF
+one   two   three four  five  six   seven eight nine  ten   
+EOF
+test_expect_success '80 columns' '
+	COLUMNS=80 git column < lista > actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+one
+two
+three
+four
+five
+six
+seven
+eight
+nine
+ten
+EOF
+test_expect_success '1 column' '
+	COLUMNS=1 git column < lista > actual &&
+	test_cmp expected actual
+'
+
+test_expect_success '1 column (--width)' '
+	git column --width=1 < lista > actual &&
+	test_cmp expected actual
+'
+
+COLUMNS=20
+export COLUMNS
+
+cat >expected <<\EOF
+one   five  nine
+two   six   ten
+three seven 
+four  eight 
+EOF
+test_expect_success '20 columns' '
+	git column < lista > actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+one   two   three
+four  five  six
+seven eight nine
+ten   
+EOF
+test_expect_success '20 columns, row mode' '
+	git column --mode=row < lista > actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'unsupported mode' '
+	test_must_fail git column --mode=foo
+'
+
+cat >expected <<\EOF
+  one   five  nine
+  two   six   ten
+  three seven 
+  four  eight 
+EOF
+test_expect_success '20 columns, left indented' '
+	git column --left-space=2 < lista > actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<\EOF
+one     six
+two     seven
+three   eight
+four    nine
+five    ten
+EOF
+test_expect_success '20 columns, right indented' '
+	git column --right-space=3 < lista > actual &&
+	test_cmp expected actual
+'
+
+test_done
-- 
1.7.0.1.370.gd3c5

[PATCH 06/12] Add helpers to redirect stdout to "git column"

[Nguyễn Thái Ngọc Duy]

While the columnizer can be used directly, it requires output code
change. The helpers instead redirect all output to "git column" and
let "git column" do all the layout work. This keeps code change
minimum. All we need is:

start_columnizer();
/* do some complex computation and output to stdout */
stop_columnizer();

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 column.h |    2 +
 pager.c  |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 0 deletions(-)

diff --git a/column.h b/column.h
index 28122e0..6eb19e3 100644
--- a/column.h
+++ b/column.h
@@ -21,3 +21,5 @@ struct columnizer {
 };
 
 extern int feed_columnizer(struct columnizer *cp, char *cell);
+extern int start_columnizer(const char **argv);
+extern int stop_columnizer();
diff --git a/pager.c b/pager.c
index ad447cf..9df727d 100644
--- a/pager.c
+++ b/pager.c
@@ -13,6 +13,8 @@
 
 static int spawned_pager;
 static int max_columns;
+static int fd_out = -1;
+static struct child_process column_process;
 
 #ifndef WIN32
 static void pager_preexec(void)
@@ -148,3 +150,64 @@ int term_columns()
 #endif
 	return 80;
 }
+
+int start_columnizer(const char **argv)
+{
+	int new_argc, argc = 0;
+	const char **column_argv;
+	char columns_param[32];
+
+	if (fd_out != -1)
+		return -1;
+
+	if (argv) {
+		while (argv[argc])
+			argc++;
+	}
+	new_argc = argc + 1; /* argv[0] = "column" */
+	if (spawned_pager && max_columns) {
+		sprintf(columns_param, "--width=%d", max_columns);
+		new_argc++;
+	}
+
+	column_argv = xmalloc(sizeof(*column_argv)*(new_argc+1));
+	column_argv[0] = "column";
+	if (spawned_pager && max_columns)
+		column_argv[1] = columns_param;
+	if (argv)
+		memcpy(column_argv + (new_argc-argc), argv, argc*sizeof(*argv));
+	column_argv[new_argc] = NULL;
+
+	fflush(stdout);
+	memset(&column_process, 0, sizeof(column_process));
+	column_process.in = -1;
+	column_process.out = dup(1);
+	column_process.git_cmd = 1;
+	column_process.argv = column_argv;
+
+	if (start_command(&column_process)) {
+		free(column_argv);
+		return -2;
+	}
+	free(column_argv);
+
+	fd_out = dup(1);
+	close(1);
+	dup2(column_process.in, 1);
+	close(column_process.in);
+	return 0;
+}
+
+int stop_columnizer()
+{
+	if (fd_out == -1)
+		return -1;
+
+	fflush(stdout);
+	close(1);
+	finish_command(&column_process);
+	dup2(fd_out, 1);
+	close(fd_out);
+	fd_out = -1;
+	return 0;
+}
-- 
1.7.0.1.370.gd3c5

[PATCH 07/12] add core.columns

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt |    8 ++++++++
 cache.h                  |    1 +
 config.c                 |    6 ++++++
 environment.c            |    1 +
 4 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c80262b..d299eaf 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -533,6 +533,14 @@ core.sparseCheckout::
 	Enable "sparse checkout" feature. See section "Sparse checkout" in
 	linkgit:git-read-tree[1] for more information.
 
+core.columns::
+	Specify whether a command should output in columns. Only commands
+	that support `--columns` will be affected by this. This variable can
+	take either `never` (do not output in columns), `auto` (output in
+	columns if the output is to a terminal) or `always` (always output in
+	columns). Setting `--columns` or `--no-columns` will override this
+	variable. This option defaults to never.
+
 add.ignore-errors::
 	Tells 'git add' to continue adding files when some files cannot be
 	added due to indexing errors. Equivalent to the '--ignore-errors'
diff --git a/cache.h b/cache.h
index 124f100..84211d2 100644
--- a/cache.h
+++ b/cache.h
@@ -549,6 +549,7 @@ extern int read_replace_refs;
 extern int fsync_object_files;
 extern int core_preload_index;
 extern int core_apply_sparse_checkout;
+extern int core_column_output;
 
 enum safe_crlf {
 	SAFE_CRLF_FALSE = 0,
diff --git a/config.c b/config.c
index 6963fbe..602c4f6 100644
--- a/config.c
+++ b/config.c
@@ -7,6 +7,7 @@
  */
 #include "cache.h"
 #include "exec_cmd.h"
+#include "color.h"
 
 #define MAXNAME (256)
 
@@ -523,6 +524,11 @@ static int git_default_core_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.columns")) {
+		core_column_output = git_config_colorbool(var, value, -1);
+		return 0;
+	}
+
 	/* Add other config variables here and to Documentation/config.txt. */
 	return 0;
 }
diff --git a/environment.c b/environment.c
index 876c5e5..4329435 100644
--- a/environment.c
+++ b/environment.c
@@ -52,6 +52,7 @@ enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 char *notes_ref_name;
 int grafts_replace_parents = 1;
 int core_apply_sparse_checkout;
+int core_column_output;
 
 /* Parallel index stat data preload? */
 int core_preload_index = 0;
-- 
1.7.0.1.370.gd3c5

[PATCH 08/12] tag: support column output with --columns

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-tag.txt |   11 ++++++++++-
 builtin/tag.c             |   17 ++++++++++++++---
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 31c78a8..1ef8df2 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -12,7 +12,8 @@ SYNOPSIS
 'git tag' [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>]
 	<tagname> [<commit> | <object>]
 'git tag' -d <tagname>...
-'git tag' [-n[<num>]] -l [--contains <commit>] [<pattern>]
+'git tag' [-n[<num>]] -l [--columns | --no-columns]
+	[--contains <commit>] [<pattern>]
 'git tag' -v <tagname>...
 
 DESCRIPTION
@@ -71,6 +72,14 @@ OPTIONS
 	List tags with names that match the given pattern (or all if no pattern is given).
 	Typing "git tag" without arguments, also lists all tags.
 
+--columns::
+	Show tags in columns. This option is only applicable if `git tag` is
+	used to list tags.
+
+--no-columns::
+	Show tags in a single list. This option is used to override core.columns
+	if set. This option is only applicable if `git tag` is used to list tags.
+
 --contains <commit>::
 	Only list tags which contain the specified commit.
 
diff --git a/builtin/tag.c b/builtin/tag.c
index 4ef1c4f..1304bec 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -12,6 +12,7 @@
 #include "tag.h"
 #include "run-command.h"
 #include "parse-options.h"
+#include "column.h"
 
 static const char * const git_tag_usage[] = {
 	"git tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
@@ -369,7 +370,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	struct ref_lock *lock;
 
 	int annotate = 0, sign = 0, force = 0, lines = -1,
-		list = 0, delete = 0, verify = 0;
+		list = 0, delete = 0, verify = 0, column_output = 0;
 	const char *msgfile = NULL, *keyid = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct commit_list *with_commit = NULL;
@@ -391,6 +392,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		OPT_STRING('u', NULL, &keyid, "key-id",
 					"use another key to sign the tag"),
 		OPT_BOOLEAN('f', "force", &force, "replace the tag if exists"),
+		OPT_BOOLEAN(0, "columns", &column_output, "show tag list in columns"),
 
 		OPT_GROUP("Tag listing options"),
 		{
@@ -403,6 +405,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	};
 
 	git_config(git_tag_config, NULL);
+	column_output = core_column_output;
 
 	argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
 
@@ -421,9 +424,17 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 
 	if (list + delete + verify > 1)
 		usage_with_options(git_tag_usage, options);
-	if (list)
-		return list_tags(argv[0], lines == -1 ? 0 : lines,
+	if (list) {
+		int ret;
+
+		if (column_output)
+			start_columnizer(NULL);
+		ret =  list_tags(argv[0], lines == -1 ? 0 : lines,
 				 with_commit);
+		if (column_output)
+			stop_columnizer();
+		return ret;
+	}
 	if (lines != -1)
 		die("-n option is only allowed with -l.");
 	if (with_commit)
-- 
1.7.0.1.370.gd3c5

[PATCH 09/12] branch: support column output with --columns

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-branch.txt |    8 ++++++++
 builtin/branch.c             |   18 +++++++++++++++---
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 903a690..3d1d600 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -9,6 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git branch' [--color[=<when>] | --no-color] [-r | -a]
+	[--columns | --no-columns]
 	[-v [--abbrev=<length> | --no-abbrev]]
 	[(--merged | --no-merged | --contains) [<commit>]]
 'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
@@ -93,6 +94,13 @@ OPTIONS
 	default to color output.
 	Same as `--color=never`.
 
+--columns::
+	Show branches in columns. This option is ignored in verbose mode.
+
+--no-columns::
+	Show branches in a single list. This option is used to override
+	core.columns if set.
+
 -r::
 	List or delete (if used with -d) the remote-tracking branches.
 
diff --git a/builtin/branch.c b/builtin/branch.c
index 6cf7e72..16273cb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -15,6 +15,7 @@
 #include "branch.h"
 #include "diff.h"
 #include "revision.h"
+#include "column.h"
 
 static const char * const builtin_branch_usage[] = {
 	"git branch [options] [-r | -a] [--merged | --no-merged]",
@@ -598,7 +599,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 {
 	int delete = 0, rename = 0, force_create = 0;
 	int verbose = 0, abbrev = DEFAULT_ABBREV, detached = 0;
-	int reflog = 0;
+	int reflog = 0, column_output = 0;
 	enum branch_track track;
 	int kinds = REF_LOCAL_BRANCH;
 	struct commit_list *with_commit = NULL;
@@ -648,10 +649,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG,
 			opt_parse_merge_filter, (intptr_t) "HEAD",
 		},
+		OPT_BOOLEAN(0, "columns", &column_output, "list branches in columns"),
 		OPT_END(),
 	};
 
 	git_config(git_branch_config, NULL);
+	column_output = core_column_output;
 
 	if (branch_use_color == -1)
 		branch_use_color = git_use_color_default;
@@ -678,8 +681,17 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 	if (delete)
 		return delete_branches(argc, argv, delete > 1, kinds);
-	else if (argc == 0)
-		print_ref_list(kinds, detached, verbose, abbrev, with_commit);
+	else if (argc == 0) {
+		if (verbose)
+			print_ref_list(kinds, detached, verbose, abbrev, with_commit);
+		else {
+			if (column_output)
+				start_columnizer(NULL);
+			print_ref_list(kinds, detached, verbose, abbrev, with_commit);
+			if (column_output)
+				stop_columnizer();
+		}
+	}
 	else if (rename && (argc == 1))
 		rename_branch(head, argv[0], rename > 1);
 	else if (rename && (argc == 2))
-- 
1.7.0.1.370.gd3c5

[PATCH 10/12] ls-files: support column output with --columns

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/ls-files.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index b065061..10cb057 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -13,6 +13,7 @@
 #include "parse-options.h"
 #include "resolve-undo.h"
 #include "string-list.h"
+#include "column.h"
 
 static int abbrev;
 static int show_deleted;
@@ -33,6 +34,7 @@ static int error_unmatch;
 static char *ps_matched;
 static const char *with_tree;
 static int exc_given;
+static int show_colums = 0;
 
 static const char *tag_cached = "";
 static const char *tag_unmerged = "";
@@ -514,6 +516,7 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 			"if any <file> is not in the index, treat this as an error"),
 		OPT_STRING(0, "with-tree", &with_tree, "tree-ish",
 			"pretend that paths removed since <tree-ish> are still present"),
+		OPT_BOOLEAN(0, "columns", &show_colums, "show files in columns"),
 		OPT__ABBREV(&abbrev),
 		OPT_END()
 	};
@@ -589,6 +592,10 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 			die("ls-files --with-tree is incompatible with -s or -u");
 		overlay_tree_on_cache(with_tree, prefix);
 	}
+
+	if (show_colums)
+		start_columnizer(NULL);
+
 	show_files(&dir, prefix);
 	if (show_resolve_undo)
 		show_ru_info(prefix);
@@ -602,5 +609,8 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		return bad ? 1 : 0;
 	}
 
+	if (show_colums)
+		stop_columnizer();
+
 	return 0;
 }
-- 
1.7.0.1.370.gd3c5

[PATCH 11/12] grep: do not return early in cmd_grep() if there is no error

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/grep.c |   39 +++++++++++++++++----------------------
 1 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 40b9a93..b8a1d5c 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -568,7 +568,6 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached)
 		if (hit && opt->status_only)
 			break;
 	}
-	free_grep_patterns(opt);
 	return hit;
 }
 
@@ -969,7 +968,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	}
 
 	if (!use_index) {
-		int hit;
 		if (cached)
 			die("--cached cannot be used with --no-index.");
 		if (list.nr)
@@ -977,35 +975,32 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		hit = grep_directory(&opt, paths);
 		if (use_threads)
 			hit |= wait_all();
-		return !hit;
 	}
-
-	if (!list.nr) {
-		int hit;
+	else if (!list.nr) {
 		if (!cached)
 			setup_work_tree();
-
 		hit = grep_cache(&opt, paths, cached);
 		if (use_threads)
 			hit |= wait_all();
-		return !hit;
+		free_grep_patterns(&opt);
 	}
-
-	if (cached)
-		die("both --cached and trees are given.");
-
-	for (i = 0; i < list.nr; i++) {
-		struct object *real_obj;
-		real_obj = deref_tag(list.objects[i].item, NULL, 0);
-		if (grep_object(&opt, paths, real_obj, list.objects[i].name)) {
-			hit = 1;
-			if (opt.status_only)
-				break;
+	else {
+		if (cached)
+			die("both --cached and trees are given.");
+
+		for (i = 0; i < list.nr; i++) {
+			struct object *real_obj;
+			real_obj = deref_tag(list.objects[i].item, NULL, 0);
+			if (grep_object(&opt, paths, real_obj, list.objects[i].name)) {
+				hit = 1;
+				if (opt.status_only)
+					break;
+			}
 		}
+		if (use_threads)
+			hit |= wait_all();
+		free_grep_patterns(&opt);
 	}
 
-	if (use_threads)
-		hit |= wait_all();
-	free_grep_patterns(&opt);
 	return !hit;
 }
-- 
1.7.0.1.370.gd3c5

[PATCH 12/12] grep: support column output with --columns

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-grep.txt |   10 ++++++++++
 builtin/grep.c             |    9 +++++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 4b32322..7bd6a4f 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -18,6 +18,7 @@ SYNOPSIS
 	   [-c | --count] [--all-match] [-q | --quiet]
 	   [--max-depth <depth>]
 	   [--color[=<when>] | --no-color]
+	   [--columns | --no-columns]
 	   [-A <post-context>] [-B <pre-context>] [-C <context>]
 	   [-f <file>] [-e] <pattern>
 	   [--and|--or|--not|(|)|-e <pattern>...]
@@ -123,6 +124,15 @@ OPTIONS
 	gives the default to color output.
 	Same as `--color=never`.
 
+--columns::
+	Show matched files in columns. This option is only applicable when
+	used with --name-only or --files-without-match.
+
+--no-columns::
+	Show matched files in a single list.  This option is used to
+	override core.columns if set. This option is only applicable
+	when used with --name-only or --files-without-match.
+
 -[ABC] <context>::
 	Show `context` trailing (`A` -- after), or leading (`B`
 	-- before), or both (`C` -- context) lines, and place a
diff --git a/builtin/grep.c b/builtin/grep.c
index b8a1d5c..48c8c57 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -15,6 +15,7 @@
 #include "grep.h"
 #include "quote.h"
 #include "dir.h"
+#include "column.h"
 
 #ifndef NO_PTHREADS
 #include "thread-utils.h"
@@ -759,6 +760,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	int cached = 0;
 	int seen_dashdash = 0;
 	int external_grep_allowed__ignored;
+	int column_output = 0;
 	struct grep_opt opt;
 	struct object_array list = { 0, 0, NULL };
 	const char **paths = NULL;
@@ -849,6 +851,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			    "allow calling of grep(1) (ignored by this build)"),
 		{ OPTION_CALLBACK, 0, "help-all", &options, NULL, "show usage",
 		  PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, help_callback },
+		OPT_BOOLEAN(0, "columns", &column_output, "show match files in columns"),
 		OPT_END()
 	};
 
@@ -876,6 +879,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	git_config(grep_config, &opt);
 	if (opt.color == -1)
 		opt.color = git_use_color_default;
+	column_output = core_column_output;
 
 	/*
 	 * If there is no -- then the paths must exist in the working
@@ -967,6 +971,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		paths[1] = NULL;
 	}
 
+	if (opt.name_only && column_output)
+		start_columnizer(NULL);
+
 	if (!use_index) {
 		if (cached)
 			die("--cached cannot be used with --no-index.");
@@ -1002,5 +1009,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		free_grep_patterns(&opt);
 	}
 
+	if (opt.name_only && column_output)
+		stop_columnizer();
 	return !hit;
 }
-- 
1.7.0.1.370.gd3c5

Re: [PATCH 00/12] Support columinized output in tag/branch/ls-files/grep

[René Scharfe]

Am 07.03.2010 13:09, schrieb Nguyễn Thái Ngọc Duy:
> This adds support to show columnized output in the mentioned commands.

That's a good idea.

I'm not so sure about the interface, though.  Does the task really
warrant adding a new git command?

If a --column parameter is added, I think it should expose the full
range of options, i.e. fill columns first (ls -C style), fill rows first
(ls -x style) as well as off (ls -1 style) and auto.

Shouldn't the columnizer use utf8_width() instead of strlen(), like
strbuf_add_wrapped_text() in utf8.c?

René

Re: [PATCH 00/12] Support columinized output in tag/branch/ls-files/grep

[Nguyen Thai Ngoc Duy]

On 3/8/10, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> Am 07.03.2010 13:09, schrieb Nguyễn Thái Ngọc Duy:
>
> > This adds support to show columnized output in the mentioned commands.
>
>
> That's a good idea.
>
>  I'm not so sure about the interface, though.  Does the task really
>  warrant adding a new git command?

Several reasons:
 - Can be reused outside of git (I was surprised Solaris did not have "column")
 - Easier to test
 - Minimum code change in modifed commands
 - I can play with more complicated column layout, with minimum code
change in git (ok, that's the third reason).

>  If a --column parameter is added, I think it should expose the full
>  range of options, i.e. fill columns first (ls -C style), fill rows first
>  (ls -x style) as well as off (ls -1 style) and auto.

Maybe an env variable would be better, so you can pass abitrary
arguments to git-column. "--column=auto" should be supported, of
course.

>  Shouldn't the columnizer use utf8_width() instead of strlen(), like
>  strbuf_add_wrapped_text() in utf8.c?

Right. I missed that.
-- 
Duy

Re: [PATCH 00/12] Support columinized output in tag/branch/ls-files/grep

[René Scharfe]

Am 08.03.2010 15:32, schrieb Nguyen Thai Ngoc Duy:
> On 3/8/10, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
>>  I'm not so sure about the interface, though.  Does the task really
>>  warrant adding a new git command?
> 
> Several reasons:
>  - Can be reused outside of git (I was surprised Solaris did not have "column")

Granted, we have a precedent: git stripspace.  But I don't like that
one, either. ;)  If it's not specific to git, then it should not be a
git command.

Perhaps name it git-column--helper, so that it can still be used by git
commands that are implemented as scripts?

>  - Easier to test

Name it test-column?

>  - Minimum code change in modifed commands

OK, forking out is easy, but replacing printf() calls with calls to feed
the columnizer shouldn't be _that_ intrusive, either.

>  - I can play with more complicated column layout, with minimum code
> change in git (ok, that's the third reason).

This is possible regardless of the way how the columnizer is invoked if
its full functionality is exposed through the command line parameter.

With "more complicated", do you perhaps mean what GNU ls does, namely
having non-uniform column widths?  I never consciously noticed that it
actually goes out of its way to cram as may columns on the screen as
possible, it just feels so natural. :)

>>  If a --column parameter is added, I think it should expose the full
>>  range of options, i.e. fill columns first (ls -C style), fill rows first
>>  (ls -x style) as well as off (ls -1 style) and auto.
> 
> Maybe an env variable would be better, so you can pass abitrary
> arguments to git-column. "--column=auto" should be supported, of
> course.

I don't see any benefit of an environment variable over config options.

René

Re: [PATCH 00/12] Support columinized output in tag/branch/ls-files/grep

[Nguyen Thai Ngoc Duy]

On 3/9/10, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> Am 08.03.2010 15:32, schrieb Nguyen Thai Ngoc Duy:
>
> > On 3/8/10, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
>
> >>  I'm not so sure about the interface, though.  Does the task really
>  >>  warrant adding a new git command?
>  >
>  > Several reasons:
>  >  - Can be reused outside of git (I was surprised Solaris did not have "column")
>
>
> Granted, we have a precedent: git stripspace.  But I don't like that
>  one, either. ;)  If it's not specific to git, then it should not be a
>  git command.
>
>  Perhaps name it git-column--helper, so that it can still be used by git
>  commands that are implemented as scripts?

OK.

>
>  >  - Easier to test
>
>  Name it test-column?

If it turns out no external process is needed for column layout.

>  >  - Minimum code change in modifed commands
>
>
> OK, forking out is easy, but replacing printf() calls with calls to feed
>  the columnizer shouldn't be _that_ intrusive, either.

Well, also fwrite() and write(). If disliked "if (blah) feed_it();
else printf(as normal);" construct. But we can wrap it to
feed_or_printf(). We may also need to buffer printf() results until a
full line is received. Let me try and see how intrusive it is.

>  >  - I can play with more complicated column layout, with minimum code
>  > change in git (ok, that's the third reason).
>
>
> This is possible regardless of the way how the columnizer is invoked if
>  its full functionality is exposed through the command line parameter.
>
>  With "more complicated", do you perhaps mean what GNU ls does, namely
>  having non-uniform column widths?  I never consciously noticed that it
>  actually goes out of its way to cram as may columns on the screen as
>  possible, it just feels so natural. :)

That. And aligned grep output like this

pclouds@do ~/w/git/builtin $ git grep -n 38
count-objects.c  |  35 |                if (cp - ent->d_name != 38)
count-objects.c  |  39 |                        memcpy(path + len + 3,
ent->d_name, 38);
count-objects.c  |  59 |                memcpy(hex+2, ent->d_name, 38);
fsck.c           | 405 |                if (strlen(de->d_name) == 38) {
gc.c             | 112 |                if (strspn(ent->d_name,
"0123456789abcdef") != 38 ||
gc.c             | 113 |                    ent->d_name[38] != '\0')
prune-packed.c   |  24 |                if (strlen(de->d_name) != 38)
prune-packed.c   |  26 |                memcpy(hex+2, de->d_name, 38);
prune-packed.c   |  31 |                memcpy(pathname + len, de->d_name, 38);
prune.c          |  64 |                if (strlen(de->d_name) == 38) {
receive-pack.c   | 588 |        char hdr_arg[38];
upload-archive.c |  86 |        char buf[16384];

>  >>  If a --column parameter is added, I think it should expose the full
>  >>  range of options, i.e. fill columns first (ls -C style), fill rows first
>  >>  (ls -x style) as well as off (ls -1 style) and auto.
>  >
>  > Maybe an env variable would be better, so you can pass abitrary
>  > arguments to git-column. "--column=auto" should be supported, of
>  > course.
>
>
> I don't see any benefit of an environment variable over config options.

Currently we may pass --column=<foo> from a porcelain to "git column
--mode=<foo>", <foo> could be column first, or row first, or either
with non-uniform columns (not implemented yet). We can also pass other
things to "git column". Putting everything in "<foo>" is OK, although
looks ugly. In my private tree, I also have "git column
--min-rows/--max-items" that forces the columnizer to one column mode
if:
 - there will be only one or two rows after columnized, too wide
screen for example (--min-rows)
 - too many lines and the layout has not been fixed, so nothing gets
printed (--max-items). Forcing back to one column mode to stop wait
time.
-- 
Duy

Re: [PATCH 00/12] Support columinized output in tag/branch/ls-files/grep

[Johannes Sixt]

Nguyen Thai Ngoc Duy schrieb:
> On 3/9/10, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
>> OK, forking out is easy, but replacing printf() calls with calls to feed
>>  the columnizer shouldn't be _that_ intrusive, either.
> 
> Well, also fwrite() and write(). If disliked "if (blah) feed_it();
> else printf(as normal);" construct. But we can wrap it to
> feed_or_printf().

How about merging this into color_fprintf and friends?

-- Hannes

-- 
"Atomic objects are neither active nor radioactive." --
Doc.No N3035 (Working Draft, Standard for Programming Language C++)

Re: [PATCH 00/12] Support columinized output in tag/branch/ls-files/grep

[Nguyen Thai Ngoc Duy]

On 3/10/10, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Nguyen Thai Ngoc Duy schrieb:
>
> > On 3/9/10, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
>
> >> OK, forking out is easy, but replacing printf() calls with calls to feed
>  >>  the columnizer shouldn't be _that_ intrusive, either.
>  >
>  > Well, also fwrite() and write(). If disliked "if (blah) feed_it();
>  > else printf(as normal);" construct. But we can wrap it to
>  > feed_or_printf().
>
>
> How about merging this into color_fprintf and friends?

Good idea.
-- 
Duy

Re: [PATCH 00/12] Support columinized output in tag/branch/ls-files/grep

[René Scharfe]

Am 10.03.2010 01:27, schrieb Nguyen Thai Ngoc Duy:
> On 3/9/10, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
>>  With "more complicated", do you perhaps mean what GNU ls does, namely
>>  having non-uniform column widths?  I never consciously noticed that it
>>  actually goes out of its way to cram as may columns on the screen as
>>  possible, it just feels so natural. :)
> 
> That. And aligned grep output like this
> 
> pclouds@do ~/w/git/builtin $ git grep -n 38
> count-objects.c  |  35 |                if (cp - ent->d_name != 38)
> count-objects.c  |  39 |                        memcpy(path + len + 3,
> ent->d_name, 38);
> count-objects.c  |  59 |                memcpy(hex+2, ent->d_name, 38);
> fsck.c           | 405 |                if (strlen(de->d_name) == 38) {
> gc.c             | 112 |                if (strspn(ent->d_name,
> "0123456789abcdef") != 38 ||
> gc.c             | 113 |                    ent->d_name[38] != '\0')
> prune-packed.c   |  24 |                if (strlen(de->d_name) != 38)
> prune-packed.c   |  26 |                memcpy(hex+2, de->d_name, 38);
> prune-packed.c   |  31 |                memcpy(pathname + len, de->d_name, 38);
> prune.c          |  64 |                if (strlen(de->d_name) == 38) {
> receive-pack.c   | 588 |        char hdr_arg[38];
> upload-archive.c |  86 |        char buf[16384];

Hmm, I'm not sure that this columnizing is very useful in this instance.
 You can more easily compare the line numbers and the indent level of
the hits, but both pieces of information are only useful in the context
of the file, so this easier comparison doesn't buy you much.

Another possible use might be the list of untracked files shown by
status and commit, by the way.

>> I don't see any benefit of an environment variable over config options.
> 
> Currently we may pass --column=<foo> from a porcelain to "git column
> --mode=<foo>", <foo> could be column first, or row first, or either
> with non-uniform columns (not implemented yet). We can also pass other
> things to "git column". Putting everything in "<foo>" is OK, although
> looks ugly. In my private tree, I also have "git column
> --min-rows/--max-items" that forces the columnizer to one column mode
> if:
>  - there will be only one or two rows after columnized, too wide
> screen for example (--min-rows)

Well, I can't imagine when I would want to use this option.  If I'm OK
with n + 100 items being displayed in x columns, I'd certainly be OK
with n items being displayed the same way, even if they only take up a
single row.

>  - too many lines and the layout has not been fixed, so nothing gets
> printed (--max-items). Forcing back to one column mode to stop wait
> time.

Interesting idea, but I'm not sure if I'd want to use it, too.  Best
effort pretty-printing combines fast output and optimized display at
first glance.  However, if there are lots of items then the user would
benefit the most from having them columnized.

If it takes too long to show the first line of output (since the
columnizer needs to wait for all items to be generated) then the command
should only columnize on request.

René

Re: [PATCH 00/12] Support columinized output in tag/branch/ls-files/grep

[Nguyen Thai Ngoc Duy]

On Fri, Mar 12, 2010 at 4:13 AM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
> Am 10.03.2010 01:27, schrieb Nguyen Thai Ngoc Duy:
>> On 3/9/10, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
>>>  With "more complicated", do you perhaps mean what GNU ls does, namely
>>>  having non-uniform column widths?  I never consciously noticed that it
>>>  actually goes out of its way to cram as may columns on the screen as
>>>  possible, it just feels so natural. :)
>>
>> That. And aligned grep output like this
>>
>> pclouds@do ~/w/git/builtin $ git grep -n 38
>> count-objects.c  |  35 |                if (cp - ent->d_name != 38)
>> count-objects.c  |  39 |                        memcpy(path + len + 3,
>> ent->d_name, 38);
>> count-objects.c  |  59 |                memcpy(hex+2, ent->d_name, 38);
>> fsck.c           | 405 |                if (strlen(de->d_name) == 38) {
>> gc.c             | 112 |                if (strspn(ent->d_name,
>> "0123456789abcdef") != 38 ||
>> gc.c             | 113 |                    ent->d_name[38] != '\0')
>> prune-packed.c   |  24 |                if (strlen(de->d_name) != 38)
>> prune-packed.c   |  26 |                memcpy(hex+2, de->d_name, 38);
>> prune-packed.c   |  31 |                memcpy(pathname + len, de->d_name, 38);
>> prune.c          |  64 |                if (strlen(de->d_name) == 38) {
>> receive-pack.c   | 588 |        char hdr_arg[38];
>> upload-archive.c |  86 |        char buf[16384];
>
> Hmm, I'm not sure that this columnizing is very useful in this instance.
>  You can more easily compare the line numbers and the indent level of
> the hits, but both pieces of information are only useful in the context
> of the file, so this easier comparison doesn't buy you much.

It's useful when I need to look at the beginning of matched lines.
With current output, all lines are not aligned, so it's hard too see
where lines begin.

> Another possible use might be the list of untracked files shown by
> status and commit, by the way.

Thanks.

>>> I don't see any benefit of an environment variable over config options.
>>
>> Currently we may pass --column=<foo> from a porcelain to "git column
>> --mode=<foo>", <foo> could be column first, or row first, or either
>> with non-uniform columns (not implemented yet). We can also pass other
>> things to "git column". Putting everything in "<foo>" is OK, although
>> looks ugly. In my private tree, I also have "git column
>> --min-rows/--max-items" that forces the columnizer to one column mode
>> if:
>>  - there will be only one or two rows after columnized, too wide
>> screen for example (--min-rows)
>
> Well, I can't imagine when I would want to use this option.  If I'm OK
> with n + 100 items being displayed in x columns, I'd certainly be OK
> with n items being displayed the same way, even if they only take up a
> single row.

It's probably just me. With 279 character-wide terminal, I find it
much easier to look vertically than 16 items on a single row.

>>  - too many lines and the layout has not been fixed, so nothing gets
>> printed (--max-items). Forcing back to one column mode to stop wait
>> time.
>
> Interesting idea, but I'm not sure if I'd want to use it, too.  Best
> effort pretty-printing combines fast output and optimized display at
> first glance.  However, if there are lots of items then the user would
> benefit the most from having them columnized.
>
> If it takes too long to show the first line of output (since the
> columnizer needs to wait for all items to be generated) then the command
> should only columnize on request.

Yeah. That's the idea of "core.columns = auto". If you specify
--[no-]columns on command line, then it must do as you wish. However,
if you set "core.columns = auto" without additional command line
arguments, then it only shows column layout when it's good to do.
-- 
Duy

Re: [PATCH 00/12] Support columinized output in tag/branch/ls-files/grep

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Am 07.03.2010 13:09, schrieb Nguyễn Thái Ngọc Duy:
>> This adds support to show columnized output in the mentioned commands.
>
> That's a good idea.
>
> I'm not so sure about the interface, though.  Does the task really
> warrant adding a new git command?
>
> If a --column parameter is added, I think it should expose the full
> range of options, i.e. fill columns first (ls -C style), fill rows first
> (ls -x style) as well as off (ls -1 style) and auto.
>
> Shouldn't the columnizer use utf8_width() instead of strlen(), like
> strbuf_add_wrapped_text() in utf8.c?

I agree with all the above.  Also I think it should not be able to change
the behaviour of these commands so much without being explicitly asked
from the command line, especially for plumbing commands like ls-files.
IOW, introduction of the configuration variable without thinking things
through is a bad idea.

Re: [PATCH 00/12] Support columinized output in tag/branch/ls-files/grep

[Nguyen Thai Ngoc Duy]

On Tue, Mar 9, 2010 at 6:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Also I think it should not be able to change
> the behaviour of these commands so much without being explicitly asked
> from the command line, especially for plumbing commands like ls-files.
> IOW, introduction of the configuration variable without thinking things
> through is a bad idea.

Hmm.. ls-files does not take core.columns into account. Command line
parameter --columns is the only way to turn column layout on in
ls-files.
-- 
Duy

Re: [PATCH 00/12] Support columinized output in tag/branch/ls-files/grep

[Junio C Hamano]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> Hmm.. ls-files does not take core.columns into account. Command line
> parameter --columns is the only way to turn column layout on in
> ls-files.

Ok, then that's good.

By the way, I didn't single out ls-files as something special that must be
kept (i.e. "it is ok to touch any other things as long as you never
touched ls-files" is not what I meant) but mentioned it as an example of
plumbing.