* [PATCH v5 12/12] tag: add --column
From: Nguyễn Thái Ngọc Duy @ 2012-02-04 15:59 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1328371156-4009-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/config.txt | 4 ++++
Documentation/git-tag.txt | 11 ++++++++++-
Makefile | 2 +-
builtin/tag.c | 30 +++++++++++++++++++++++++++---
t/t7004-tag.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 86 insertions(+), 5 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ebb210c..145336a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -855,6 +855,10 @@ column.status::
Specify whether to output untracked files in `git status` in columns.
See `column.ui` for details.
+column.tag::
+ Specify whether to output tag listing in `git tag` in columns.
+ See `column.ui` for details.
+
commit.status::
A boolean to enable/disable inclusion of status information in the
commit message template when using an editor to prepare the commit
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 53ff5f6..5ead91e 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 [--contains <commit>]
+ [--column[=<options>] | --no-column] [<pattern>...]
'git tag' -v <tagname>...
DESCRIPTION
@@ -83,6 +84,14 @@ OPTIONS
using fnmatch(3)). Multiple patterns may be given; if any of
them matches, the tag is shown.
+--column[=<options>]::
+--no-column::
+ Display tag listing in columns. See configuration variable
+ column.tag for option syntax.`--column` and `--no-column`
+ without options are equivalent to 'always' and 'never' respectively.
++
+This option is only applicable when listing tags without annotation lines.
+
--contains <commit>::
Only list tags which contain the specified commit.
diff --git a/Makefile b/Makefile
index b2644bc..eb7b6fc 100644
--- a/Makefile
+++ b/Makefile
@@ -2116,7 +2116,7 @@ builtin/prune.o builtin/reflog.o reachable.o: reachable.h
builtin/commit.o builtin/revert.o wt-status.o: wt-status.h
builtin/tar-tree.o archive-tar.o: tar.h
connect.o transport.o url.o http-backend.o: url.h
-builtin/branch.o builtin/commit.o column.o help.o pager.o: column.h
+builtin/branch.o builtin/commit.o builtin/tag.o column.o help.o pager.o: column.h
http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h url.h
diff --git a/builtin/tag.c b/builtin/tag.c
index 31f02e8..d01f918 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -15,6 +15,7 @@
#include "diff.h"
#include "revision.h"
#include "gpg-interface.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>]",
@@ -30,6 +31,8 @@ struct tag_filter {
struct commit_list *with_commit;
};
+static unsigned int colopts;
+
static int match_pattern(const char **patterns, const char *ref)
{
/* no pattern means match everything */
@@ -230,6 +233,9 @@ static int git_tag_config(const char *var, const char *value, void *cb)
int status = git_gpg_config(var, value, cb);
if (status)
return status;
+ status = git_column_config(var, value, "tag", &colopts);
+ if (status <= 0)
+ return status;
return git_default_config(var, value, cb);
}
@@ -409,6 +415,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT_STRING('u', "local-user", &keyid, "key-id",
"use another key to sign the tag"),
OPT__FORCE(&force, "replace the tag if exists"),
+ OPT_COLUMN(0, "column", &colopts, "show tag list in columns" ),
OPT_GROUP("Tag listing options"),
{
@@ -424,6 +431,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
memset(&opt, 0, sizeof(opt));
+ colopts &= ~COL_ENABLED_SET;
argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
if (keyid) {
@@ -441,9 +449,25 @@ 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, lines == -1 ? 0 : lines,
- with_commit);
+ if (list && lines != -1) {
+ /* only die when --column is given explicitly */
+ if ((colopts & (COL_ENABLED_SET | COL_ENABLED)) == (COL_ENABLED_SET | COL_ENABLED))
+ die(_("--column and -n are incompatible"));
+ colopts = 0;
+ }
+ if (list) {
+ int ret;
+ if (lines == -1 && colopts & COL_ENABLED) {
+ struct column_options copts;
+ memset(&copts, 0, sizeof(copts));
+ copts.padding = 2;
+ run_column_filter(colopts, &copts);
+ }
+ ret = list_tags(argv, lines == -1 ? 0 : lines, with_commit);
+ if (lines == -1 && colopts & COL_ENABLED)
+ stop_column_filter();
+ return ret;
+ }
if (lines != -1)
die(_("-n option is only allowed with -l."));
if (with_commit)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index e93ac73..579a298 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -263,6 +263,50 @@ test_expect_success 'tag -l can accept multiple patterns' '
test_cmp expect actual
'
+test_expect_success 'listing tags in column' '
+ COLUMNS=40 git tag -l --column=row >actual &&
+ cat >expected <<\EOF &&
+a1 aa1 cba t210 t211
+v0.2.1 v1.0 v1.0.1 v1.1.3
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'listing tags in column with column.*' '
+ git config column.tag row &&
+ git config column.ui dense &&
+ COLUMNS=40 git tag -l >actual &&
+ git config --unset column.ui &&
+ git config --unset column.tag &&
+ cat >expected <<\EOF &&
+a1 aa1 cba t210 t211
+v0.2.1 v1.0 v1.0.1 v1.1.3
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'listing tag with -n --column should fail' '
+ test_must_fail git tag --column -n
+'
+
+test_expect_success 'listing tags -n in column with column.ui ignored' '
+ git config column.ui "row dense" &&
+ COLUMNS=40 git tag -l -n >actual &&
+ git config --unset column.ui &&
+ cat >expected <<\EOF &&
+a1 Foo
+aa1 Foo
+cba Foo
+t210 Foo
+t211 Foo
+v0.2.1 Foo
+v1.0 Foo
+v1.0.1 Foo
+v1.1.3 Foo
+EOF
+ test_cmp expected actual
+'
+
# creating and verifying lightweight tags:
test_expect_success \
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH v6 03/12] Add git-column and column mode parsing
From: Nguyễn Thái Ngọc Duy @ 2012-02-04 16:11 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1328371156-4009-4-git-send-email-pclouds@gmail.com>
A column option string consists of many token separated by either
space of commas. A token belongs to one of three groups:
- enabling: always, never and auto
- layout mode: to be implemented
- other tuning, which could be negated be prefix 'no'
A command line option without argument (e.g. --column) will enable
column output and reuse existing settings (layout mode and options..).
--no-column disables columnar output.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Turn colopts to unsigned int from the beginning.
.gitignore | 1 +
Documentation/git-column.txt | 49 ++++++++++++++++
Makefile | 1 +
builtin.h | 1 +
builtin/column.c | 41 ++++++++++++++
column.c | 125 ++++++++++++++++++++++++++++++++++++++++++
column.h | 7 ++
command-list.txt | 1 +
git.c | 1 +
parse-options.h | 2 +
t/t9002-column.sh | 27 +++++++++
11 files changed, 256 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/.gitignore b/.gitignore
index 3b7680e..039e5ef 100644
--- a/.gitignore
+++ b/.gitignore
@@ -26,6 +26,7 @@
/git-cherry-pick
/git-clean
/git-clone
+/git-column
/git-commit
/git-commit-tree
/git-config
diff --git a/Documentation/git-column.txt b/Documentation/git-column.txt
new file mode 100644
index 0000000..508b85f
--- /dev/null
+++ b/Documentation/git-column.txt
@@ -0,0 +1,49 @@
+git-column(1)
+=============
+
+NAME
+----
+git-column - Display data in columns
+
+SYNOPSIS
+--------
+[verse]
+'git column' [--mode=<mode> | --rawmode=<n>] [--width=<width>]
+ [--indent=<string>] [--nl=<string>] [--pading=<n>]
+
+DESCRIPTION
+-----------
+This command formats its input into multiple columns.
+
+OPTIONS
+-------
+--mode=<mode>::
+ Specify layout mode. See configuration variable column.ui for option
+ syntax.
+
+--rawmode=<n>::
+ Same as --mode but take mode encoded as a number. This is mainly used
+ by other commands that have already parsed layout mode.
+
+--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.
+
+--indent=<string>::
+ String to be printed at the beginning of each line.
+
+--nl=<N>::
+ String to be printed at the end of each line,
+ including newline character.
+
+--padding=<N>::
+ The number of spaces between columns. One space by default.
+
+
+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 5f0531b..92700ca 100644
--- a/Makefile
+++ b/Makefile
@@ -766,6 +766,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 857b9c8..338f540 100644
--- a/builtin.h
+++ b/builtin.h
@@ -61,6 +61,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_config(int argc, const char **argv, const char *prefix);
diff --git a/builtin/column.c b/builtin/column.c
new file mode 100644
index 0000000..3b0f74e
--- /dev/null
+++ b/builtin/column.c
@@ -0,0 +1,41 @@
+#include "builtin.h"
+#include "cache.h"
+#include "strbuf.h"
+#include "parse-options.h"
+#include "string-list.h"
+#include "column.h"
+
+static const char * const builtin_column_usage[] = {
+ "git column [options]",
+ NULL
+};
+static unsigned int colopts;
+
+int cmd_column(int argc, const char **argv, const char *prefix)
+{
+ struct string_list list = STRING_LIST_INIT_DUP;
+ struct strbuf sb = STRBUF_INIT;
+ struct column_options copts;
+ struct option options[] = {
+ OPT_COLUMN(0, "mode", &colopts, "layout to use"),
+ OPT_INTEGER(0, "rawmode", &colopts, "layout to use"),
+ OPT_INTEGER(0, "width", &copts.width, "Maximum width"),
+ OPT_STRING(0, "indent", &copts.indent, "string", "Padding space on left border"),
+ OPT_INTEGER(0, "nl", &copts.nl, "Padding space on right border"),
+ OPT_INTEGER(0, "padding", &copts.padding, "Padding space between columns"),
+ OPT_END()
+ };
+
+ memset(&copts, 0, sizeof(copts));
+ copts.width = term_columns();
+ copts.padding = 1;
+ argc = parse_options(argc, argv, "", options, builtin_column_usage, 0);
+ if (argc)
+ usage_with_options(builtin_column_usage, options);
+
+ while (!strbuf_getline(&sb, stdin, '\n'))
+ string_list_append(&list, sb.buf);
+
+ print_columns(&list, colopts, &copts);
+ return 0;
+}
diff --git a/column.c b/column.c
index 742ae18..d20cf8f 100644
--- a/column.c
+++ b/column.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "column.h"
#include "string-list.h"
+#include "parse-options.h"
#define MODE(mode) ((mode) & COL_MODE)
@@ -37,3 +38,127 @@ void print_columns(const struct string_list *list, unsigned int mode,
}
die("BUG: invalid mode %d", MODE(mode));
}
+
+struct colopt {
+ enum {
+ ENABLE,
+ MODE,
+ OPTION
+ } type;
+ const char *name;
+ int value;
+};
+
+/*
+ * Set COL_ENABLED and COL_ENABLED_SET. If 'set' is -1, check if
+ * stdout is tty.
+ */
+static int set_enable_bit(unsigned int *mode, int set, int stdout_is_tty)
+{
+ if (set < 0) { /* auto */
+ if (stdout_is_tty < 0)
+ stdout_is_tty = isatty(1);
+ set = stdout_is_tty || (pager_in_use() && pager_use_color);
+ }
+ if (set)
+ *mode = *mode | COL_ENABLED | COL_ENABLED_SET;
+ else
+ *mode = (*mode & ~COL_ENABLED) | COL_ENABLED_SET;
+ return 0;
+}
+
+/*
+ * Set COL_MODE_*. mode is intially copied from column.ui. If
+ * COL_ENABLED_SET is not set, then neither 'always', 'never' nor
+ * 'auto' has been used. Default to 'always'.
+ */
+static int set_mode(unsigned int *mode, unsigned int value)
+{
+ *mode = (*mode & ~COL_MODE) | value;
+ if (!(*mode & COL_ENABLED_SET))
+ *mode |= COL_ENABLED | COL_ENABLED_SET;
+
+ return 0;
+}
+
+/* Set or unset other COL_* */
+static int set_option(unsigned int *mode, unsigned int opt, int set)
+{
+ if (set)
+ *mode |= opt;
+ else
+ *mode &= ~opt;
+ return 0;
+}
+
+static int parse_option(const char *arg, int len,
+ unsigned int *mode, int stdout_is_tty)
+{
+ struct colopt opts[] = {
+ { ENABLE, "always", 1 },
+ { ENABLE, "never", 0 },
+ { ENABLE, "auto", -1 },
+ };
+ int i, set, name_len;
+
+ for (i = 0; i < ARRAY_SIZE(opts); i++) {
+ if (opts[i].type == OPTION) {
+ if (len > 2 && !strncmp(arg, "no", 2)) {
+ arg += 2;
+ len -= 2;
+ set = 0;
+ }
+ else
+ set = 1;
+ }
+
+ name_len = strlen(opts[i].name);
+ if (len != name_len ||
+ strncmp(arg, opts[i].name, name_len))
+ continue;
+
+ switch (opts[i].type) {
+ case ENABLE: return set_enable_bit(mode, opts[i].value,
+ stdout_is_tty);
+ case MODE: return set_mode(mode, opts[i].value);
+ case OPTION: return set_option(mode, opts[i].value, set);
+ default: die("BUG: Unknown option type %d", opts[i].type);
+ }
+ }
+
+ return error("unsupported style '%s'", arg);
+}
+
+int git_config_column(unsigned int *mode, const char *value,
+ int stdout_is_tty)
+{
+ const char *sep = " ,";
+
+ while (*value) {
+ int len = strcspn(value, sep);
+ if (len) {
+ if (parse_option(value, len, mode, stdout_is_tty))
+ return -1;
+
+ value += len;
+ }
+ value += strspn(value, sep);
+ }
+ return 0;
+}
+
+int parseopt_column_callback(const struct option *opt,
+ const char *arg, int unset)
+{
+ unsigned int *mode = opt->value;
+ if (unset) {
+ *mode = (*mode & ~COL_ENABLED) | COL_ENABLED_SET;
+ return 0;
+ }
+ if (arg)
+ return git_config_column(mode, arg, -1);
+
+ /* no arg, turn it on */
+ *mode |= COL_ENABLED | COL_ENABLED_SET;
+ return 0;
+}
diff --git a/column.h b/column.h
index 8e4fdaa..f48fa81 100644
--- a/column.h
+++ b/column.h
@@ -3,6 +3,7 @@
#define COL_MODE 0x000F
#define COL_ENABLED (1 << 4)
+#define COL_ENABLED_SET (1 << 5) /* Has COL_ENABLED been set by config? */
struct column_options {
int width;
@@ -15,5 +16,11 @@ extern int term_columns(void);
extern void print_columns(const struct string_list *list,
unsigned int mode,
struct column_options *opts);
+extern int git_config_column(unsigned int *mode, const char *value,
+ int stdout_is_tty);
+
+struct option;
+extern int parseopt_column_callback(const struct option *opt,
+ const char *arg, int unset);
#endif
diff --git a/command-list.txt b/command-list.txt
index a36ee9b..fe06f15 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
diff --git a/git.c b/git.c
index 3805616..419e3cc 100644
--- a/git.c
+++ b/git.c
@@ -348,6 +348,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
{ "clone", cmd_clone },
+ { "column", cmd_column },
{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
{ "commit-tree", cmd_commit_tree, RUN_SETUP },
{ "config", cmd_config, RUN_SETUP_GENTLY },
diff --git a/parse-options.h b/parse-options.h
index 2e811dc..56fcafd 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -238,5 +238,7 @@ extern int parse_opt_noop_cb(const struct option *, const char *, int);
PARSE_OPT_OPTARG, &parse_opt_abbrev_cb, 0 }
#define OPT__COLOR(var, h) \
OPT_COLOR_FLAG(0, "color", (var), (h))
+#define OPT_COLUMN(s, l, v, h) \
+ { OPTION_CALLBACK, (s), (l), (v), "style", (h), PARSE_OPT_OPTARG, parseopt_column_callback }
#endif
diff --git a/t/t9002-column.sh b/t/t9002-column.sh
new file mode 100755
index 0000000..b0b6d62
--- /dev/null
+++ b/t/t9002-column.sh
@@ -0,0 +1,27 @@
+#!/bin/sh
+
+test_description='git column'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ cat >lista <<\EOF
+one
+two
+three
+four
+five
+six
+seven
+eight
+nine
+ten
+eleven
+EOF
+'
+
+test_expect_success 'never' '
+ git column --mode=never <lista >actual &&
+ test_cmp lista actual
+'
+
+test_done
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH v6 07/12] column: add column.ui for default column output settings
From: Nguyễn Thái Ngọc Duy @ 2012-02-04 16:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1328371156-4009-8-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Really get rid of git_colopts (aka git_column_mode in v5). Sorry.
Documentation/config.txt | 26 ++++++++++++++++++++++++++
Documentation/git-column.txt | 6 +++++-
builtin/column.c | 21 +++++++++++++++++++++
column.c | 36 ++++++++++++++++++++++++++++++++++++
column.h | 2 ++
5 files changed, 90 insertions(+), 1 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index abeb82b..5216598 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -821,6 +821,32 @@ color.ui::
`never` if you prefer git commands not to use color unless enabled
explicitly with some other configuration or the `--color` option.
+column.ui::
+ Specify whether supported commands should output in columns.
+ This variable consists of a list of tokens separated by spaces
+ or commas:
++
+--
+`always`;;
+ always show in columns
+`never`;;
+ never show in columns
+`auto`;;
+ show in columns if the output is to the terminal
+`column`;;
+ fill columns before rows (default)
+`row`;;
+ fill rows before columns
+`dense`;;
+ make unequal size columns to utilize more space
+`nodense`;;
+ make equal size columns
+`color`;;
+ input contains ANSI escape sequence for coloring
+--
++
+ This option defaults to 'never'.
+
commit.status::
A boolean to enable/disable inclusion of status information in the
commit message template when using an editor to prepare the commit
diff --git a/Documentation/git-column.txt b/Documentation/git-column.txt
index 508b85f..94fd7ac 100644
--- a/Documentation/git-column.txt
+++ b/Documentation/git-column.txt
@@ -8,7 +8,7 @@ git-column - Display data in columns
SYNOPSIS
--------
[verse]
-'git column' [--mode=<mode> | --rawmode=<n>] [--width=<width>]
+'git column' [--command=<name>] [--[raw]mode=<mode>] [--width=<width>]
[--indent=<string>] [--nl=<string>] [--pading=<n>]
DESCRIPTION
@@ -17,6 +17,10 @@ This command formats its input into multiple columns.
OPTIONS
-------
+--command=<name>::
+ Look up layout mode using configuration variable column.<name> and
+ column.ui.
+
--mode=<mode>::
Specify layout mode. See configuration variable column.ui for option
syntax.
diff --git a/builtin/column.c b/builtin/column.c
index 3b0f74e..351375f 100644
--- a/builtin/column.c
+++ b/builtin/column.c
@@ -11,12 +11,19 @@ static const char * const builtin_column_usage[] = {
};
static unsigned int colopts;
+static int column_config(const char *var, const char *value, void *cb)
+{
+ return git_column_config(var, value, cb, &colopts);
+}
+
int cmd_column(int argc, const char **argv, const char *prefix)
{
struct string_list list = STRING_LIST_INIT_DUP;
struct strbuf sb = STRBUF_INIT;
struct column_options copts;
+ const char *command = NULL, *real_command = NULL;
struct option options[] = {
+ OPT_STRING(0, "command", &real_command, "name", "lookup config vars"),
OPT_COLUMN(0, "mode", &colopts, "layout to use"),
OPT_INTEGER(0, "rawmode", &colopts, "layout to use"),
OPT_INTEGER(0, "width", &copts.width, "Maximum width"),
@@ -26,6 +33,15 @@ int cmd_column(int argc, const char **argv, const char *prefix)
OPT_END()
};
+ /* This one is special and must be the first one */
+ if (argc > 1 && !prefixcmp(argv[1], "--command=")) {
+ int nonitok = 0;
+ setup_git_directory_gently(&nonitok);
+
+ command = argv[1] + 10;
+ git_config(column_config, (void*)command);
+ }
+
memset(&copts, 0, sizeof(copts));
copts.width = term_columns();
copts.padding = 1;
@@ -33,6 +49,11 @@ int cmd_column(int argc, const char **argv, const char *prefix)
if (argc)
usage_with_options(builtin_column_usage, options);
+ if (real_command || command) {
+ if (!real_command || !command || strcmp(real_command, command))
+ die(_("--command must be the first argument"));
+ }
+
while (!strbuf_getline(&sb, stdin, '\n'))
string_list_append(&list, sb.buf);
diff --git a/column.c b/column.c
index fcff2fd..e3cec1c 100644
--- a/column.c
+++ b/column.c
@@ -2,6 +2,7 @@
#include "column.h"
#include "string-list.h"
#include "parse-options.h"
+#include "color.h"
#include "utf8.h"
#define MODE(mode) ((mode) & COL_MODE)
@@ -356,6 +357,41 @@ int git_config_column(unsigned int *mode, const char *value,
return 0;
}
+static int column_config(const char *var, const char *value,
+ const char *key, unsigned int *colopts)
+{
+ if (!strcmp(var, key)) {
+ int ret = git_config_column(colopts, value, -1);
+ if (ret)
+ die("invalid %s mode %s", key, value);
+ return 0;
+ }
+ return 1; /* go on */
+}
+
+int git_column_config(const char *var, const char *value,
+ const char *command, unsigned int *colopts)
+{
+ int ret;
+
+ *colopts &= ~COL_ENABLED_SET;
+ ret = column_config(var, value, "column.ui", colopts);
+ if (ret <= 0)
+ return ret;
+
+ if (command) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_addf(&sb, "column.%s", command);
+ *colopts &= ~COL_ENABLED_SET;
+ ret = column_config(var, value, sb.buf, colopts);
+ strbuf_release(&sb);
+ if (ret <= 0)
+ return ret;
+ }
+
+ return 1; /* go on */
+}
+
int parseopt_column_callback(const struct option *opt,
const char *arg, int unset)
{
diff --git a/column.h b/column.h
index 2255851..3d2ed52 100644
--- a/column.h
+++ b/column.h
@@ -23,6 +23,8 @@ extern void print_columns(const struct string_list *list,
struct column_options *opts);
extern int git_config_column(unsigned int *mode, const char *value,
int stdout_is_tty);
+extern int git_column_config(const char *var, const char *value,
+ const char *command, unsigned int *colopts);
struct option;
extern int parseopt_column_callback(const struct option *opt,
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH v2] branch: borrow --sort and --count from for-each-ref
From: Nguyễn Thái Ngọc Duy @ 2012-02-04 16:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326805907-19416-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Fix sorted refs being fnmatch()'d twice.
Documentation/git-branch.txt | 13 +++++++++
Makefile | 1 +
builtin/branch.c | 60 +++++++++++++++++++++++++++++++++--------
builtin/for-each-ref.c | 33 +++-------------------
builtin/for-each-ref.h | 32 ++++++++++++++++++++++
5 files changed, 99 insertions(+), 40 deletions(-)
create mode 100644 builtin/for-each-ref.h
diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 0427e80..b6f2826 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -10,6 +10,7 @@ SYNOPSIS
[verse]
'git branch' [--color[=<when>] | --no-color] [-r | -a]
[--list] [-v [--abbrev=<length> | --no-abbrev]]
+ [--count=<count>] [(--sort=<key>)...]
[(--merged | --no-merged | --contains) [<commit>]] [<pattern>...]
'git branch' [--set-upstream | --track | --no-track] [-l] [-f] <branchname> [<start-point>]
'git branch' (-m | -M) [<oldbranch>] <newbranch>
@@ -192,6 +193,18 @@ start-point is either a local or remote-tracking branch.
The new name for an existing branch. The same restrictions as for
<branchname> apply.
+<count>::
+ By default the command shows all refs that match
+ `<pattern>`. This option makes it stop after showing
+ that many refs.
+
+<key>::
+ A field name to sort on. Prefix `-` to sort in descending
+ order of the value. When unspecified, `refname` is used. You
+ may use the --sort=<key> option multiple times, in which case
+ the last key becomes the primary key. See
+ linkgit:for-each-ref[1] for field name details.
+
Examples
--------
diff --git a/Makefile b/Makefile
index a782409..daf3e46 100644
--- a/Makefile
+++ b/Makefile
@@ -2108,6 +2108,7 @@ builtin/log.o builtin/shortlog.o: shortlog.h
builtin/prune.o builtin/reflog.o reachable.o: reachable.h
builtin/commit.o builtin/revert.o wt-status.o: wt-status.h
builtin/tar-tree.o archive-tar.o: tar.h
+builtin/branch.o builtin/for-each-ref.o: builtin/for-each-ref.h
connect.o transport.o url.o http-backend.o: url.h
http-fetch.o http-walker.o remote-curl.o transport.o walker.o: walker.h
http.o http-walker.o http-push.o http-fetch.o remote-curl.o: http.h url.h
diff --git a/builtin/branch.c b/builtin/branch.c
index 7095718..97bf96c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -15,6 +15,7 @@
#include "branch.h"
#include "diff.h"
#include "revision.h"
+#include "for-each-ref.h"
static const char * const builtin_branch_usage[] = {
"git branch [options] [-r | -a] [--merged | --no-merged]",
@@ -30,6 +31,9 @@ static const char * const builtin_branch_usage[] = {
static const char *head;
static unsigned char head_sha1[20];
+static struct ref_sort *sort = NULL, **sort_tail = &sort;
+static int maxcount;
+
static int branch_use_color = -1;
static char branch_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
@@ -312,7 +316,7 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,
if ((kind & ref_list->kinds) == 0)
return 0;
- if (!match_patterns(cb->pattern, refname))
+ if (cb->pattern && !match_patterns(cb->pattern, refname))
return 0;
commit = NULL;
@@ -510,10 +514,37 @@ static void show_detached(struct ref_list *ref_list)
}
}
-static int print_ref_list(int kinds, int detached, int verbose, int abbrev, struct commit_list *with_commit, const char **pattern)
+static int fetch_branches(struct ref_list *ref_list,
+ const char **pattern)
{
- int i;
struct append_ref_cb cb;
+ memset(&cb, 0, sizeof(cb));
+ cb.ref_list = ref_list;
+ if (sort) {
+ struct grab_ref_cbdata cbdata;
+ int i;
+ memset(&cbdata, 0, sizeof(cbdata));
+ cbdata.grab_pattern = pattern;
+ for_each_rawref(grab_single_ref, &cbdata);
+ sort_refs(sort, cbdata.grab_array, cbdata.grab_cnt);
+ for (i = 0; i < cbdata.grab_cnt; i++) {
+ struct refinfo *ri = cbdata.grab_array[i];
+ append_ref(ri->refname, ri->objectname, ri->flag, &cb);
+ }
+ } else {
+ cb.pattern = pattern;
+ for_each_rawref(append_ref, &cb);
+ qsort(ref_list->list, ref_list->index,
+ sizeof(struct ref_item), ref_cmp);
+ }
+ return cb.ret;
+}
+
+static int print_ref_list(int kinds, int detached, int verbose, int abbrev,
+ struct commit_list *with_commit,
+ const char **pattern)
+{
+ int i, ret;
struct ref_list ref_list;
memset(&ref_list, 0, sizeof(ref_list));
@@ -523,10 +554,7 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
ref_list.with_commit = with_commit;
if (merge_filter != NO_FILTER)
init_revisions(&ref_list.revs, NULL);
- cb.ref_list = &ref_list;
- cb.pattern = pattern;
- cb.ret = 0;
- for_each_rawref(append_ref, &cb);
+ ret = fetch_branches(&ref_list, pattern);
if (merge_filter != NO_FILTER) {
struct commit *filter;
filter = lookup_commit_reference_gently(merge_filter_ref, 0);
@@ -539,13 +567,13 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
ref_list.maxwidth = calc_maxwidth(&ref_list);
}
- qsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp);
-
detached = (detached && (kinds & REF_LOCAL_BRANCH));
if (detached && match_patterns(pattern, "HEAD"))
show_detached(&ref_list);
- for (i = 0; i < ref_list.index; i++) {
+ if (!maxcount)
+ maxcount = ref_list.index;
+ for (i = 0; i < maxcount; i++) {
int current = !detached &&
(ref_list.list[i].kind == REF_LOCAL_BRANCH) &&
!strcmp(ref_list.list[i].name, head);
@@ -558,10 +586,10 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
free_ref_list(&ref_list);
- if (cb.ret)
+ if (ret)
error(_("some refs could not be read"));
- return cb.ret;
+ return ret;
}
static void rename_branch(const char *oldname, const char *newname, int force)
@@ -702,6 +730,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
parse_opt_with_commit, (intptr_t) "HEAD",
},
OPT__ABBREV(&abbrev),
+ OPT_CALLBACK(0 , "sort", sort_tail, "key",
+ "field name to sort on", &opt_parse_sort),
+ OPT_INTEGER( 0 , "count", &maxcount, "show only <n> matched refs"),
OPT_GROUP("Specific git-branch actions:"),
OPT_SET_INT('a', "all", &kinds, "list both remote-tracking and local branches",
@@ -752,6 +783,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
0);
+ if (maxcount < 0) {
+ error("invalid --count argument: `%d'", maxcount);
+ usage_with_options(builtin_branch_usage, options);
+ }
+
if (!delete && !rename && !edit_description && argc == 0)
list = 1;
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index b01d76a..7b25c54 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,6 +9,7 @@
#include "quote.h"
#include "parse-options.h"
#include "remote.h"
+#include "for-each-ref.h"
/* Quoting styles */
#define QUOTE_NONE 0
@@ -19,25 +20,6 @@
typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
-struct atom_value {
- const char *s;
- unsigned long ul; /* used for sorting when not FIELD_STR */
-};
-
-struct ref_sort {
- struct ref_sort *next;
- int atom; /* index into used_atom array */
- unsigned reverse : 1;
-};
-
-struct refinfo {
- char *refname;
- unsigned char objectname[20];
- int flag;
- const char *symref;
- struct atom_value *value;
-};
-
static struct {
const char *name;
cmp_type cmp_type;
@@ -765,17 +747,12 @@ static void get_value(struct refinfo *ref, int atom, struct atom_value **v)
*v = &ref->value[atom];
}
-struct grab_ref_cbdata {
- struct refinfo **grab_array;
- const char **grab_pattern;
- int grab_cnt;
-};
-
/*
* A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
*/
-static int grab_single_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+int grab_single_ref(const char *refname, const unsigned char *sha1,
+ int flag, void *cb_data)
{
struct grab_ref_cbdata *cb = cb_data;
struct refinfo *ref;
@@ -858,7 +835,7 @@ static int compare_refs(const void *a_, const void *b_)
return 0;
}
-static void sort_refs(struct ref_sort *sort, struct refinfo **refs, int num_refs)
+void sort_refs(struct ref_sort *sort, struct refinfo **refs, int num_refs)
{
ref_sort = sort;
qsort(refs, num_refs, sizeof(struct refinfo *), compare_refs);
@@ -953,7 +930,7 @@ static struct ref_sort *default_sort(void)
return sort;
}
-static int opt_parse_sort(const struct option *opt, const char *arg, int unset)
+int opt_parse_sort(const struct option *opt, const char *arg, int unset)
{
struct ref_sort **sort_tail = opt->value;
struct ref_sort *s;
diff --git a/builtin/for-each-ref.h b/builtin/for-each-ref.h
new file mode 100644
index 0000000..8542d66
--- /dev/null
+++ b/builtin/for-each-ref.h
@@ -0,0 +1,32 @@
+struct atom_value {
+ const char *s;
+ unsigned long ul; /* used for sorting when not FIELD_STR */
+};
+
+struct ref_sort {
+ struct ref_sort *next;
+ int atom; /* index into used_atom array */
+ unsigned reverse : 1;
+};
+
+struct refinfo {
+ char *refname;
+ unsigned char objectname[20];
+ int flag;
+ const char *symref;
+ struct atom_value *value;
+};
+
+struct grab_ref_cbdata {
+ struct refinfo **grab_array;
+ const char **grab_pattern;
+ int grab_cnt;
+};
+
+extern int grab_single_ref(const char *refname,
+ const unsigned char *sha1,
+ int flag, void *cb_data);
+extern int opt_parse_sort(const struct option *opt,
+ const char *arg, int unset);
+extern void sort_refs(struct ref_sort *sort,
+ struct refinfo **refs, int num_refs);
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH] send-email: add extra safetly in address sanitazion
From: Felipe Contreras @ 2012-02-04 16:32 UTC (permalink / raw)
To: git
Cc: Felipe Contreras, Brandon Casey, Uwe Kleine-König,
Brian Gernhardt, Robin H. Johnson,
Ævar Arnfjörð Bjarmason
Currently bad addresses like 'Foo Bar <foo@bar.com>>' will just be sent
verbatim -- that's not good; we should either error out, or sanitize
them.
The following patch adds extra sanitazion so the following
transformations are performed:
'Foo Bar <foo@bar.com>' -> 'Foo Bar <foo@bar.com>'
'"Foo Bar" <foo@bar.com>' -> '"Foo Bar" <foo@bar.com>'
'foo@bar.com' -> 'foo@bar.com'
'<foo@bar.com>' -> 'foo@bar.com'
'Foo Bar' -> 'Foo Bar'
'Foo Bar <foo@bar.com>>' -> 'Foo Bar <foo@bar.com>'
'"Foo Bar" <foo@bar.com>>' -> '"Foo Bar" <foo@bar.com>'
'<foo@bar.com>>' -> 'foo@bar.com'
Basically, we try to check that the address is in the form of
"Name <email>", and if not, assume it's "email". According to commit
155197e[1], the "prhase" should not be empty, so if it is, remove the
<>. Extra characters after the first ">" are ignored.
[1] send-email: rfc822 forbids using <address@domain> without a non-empty "phrase"
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
git-send-email.perl | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index ef30c55..19c600f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -889,15 +889,21 @@ sub is_rfc2047_quoted {
# use the simplest quoting being able to handle the recipient
sub sanitize_address {
my ($recipient) = @_;
- my ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\s*(<.*)/);
+ my ($recipient_name, $recipient_addr);
+
+ if ($recipient =~ /^(.*?)\s*<(.*?)>/) {
+ ($recipient_name, $recipient_addr) = ($1, $2);
+ } else {
+ $recipient_addr = $recipient;
+ }
if (not $recipient_name) {
- return $recipient;
+ return $recipient_addr;
}
# if recipient_name is already quoted, do nothing
if (is_rfc2047_quoted($recipient_name)) {
- return $recipient;
+ return "$recipient_name <$recipient_addr>";
}
# rfc2047 is needed if a non-ascii char is included
@@ -912,7 +918,7 @@ sub sanitize_address {
$recipient_name = qq["$recipient_name"];
}
- return "$recipient_name $recipient_addr";
+ return "$recipient_name <$recipient_addr>";
}
--
1.7.9
^ permalink raw reply related
* RE: Git performance results on a large repository
From: Joshua Redstone @ 2012-02-04 18:05 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git@vger.kernel.org
In-Reply-To: <CACsJy8DkLCK0ZUKNz_PJazsxjsRbWVVZwjAU5n2EAjJfCYtpoQ@mail.gmail.com>
[ wanted to reply to my initial msg, but wasn't subscribed to the list at time of mailing, so replying to most recent post instead ]
Thanks to everyone for the questions and suggestions. I'll try to respond here. One high-level clarification - this synthetic repo for which I've reported perf times is representative of where we think we'll be in the future. Git is slow but marginally acceptable for today. We want to start planning now for any big changes we need to make going forward.
Evgeny Sazhin, Slinky and Ævar Arnfjörð Bjarmason suggested splitting up the repo into multiple, smaller repos. I indicated before that we have a lot of cross-dependencies. Our largest repo by number of files and commits is the repo containing the front-end server. It is a large code base in which the tight integration of various components results in many of the cross dependencies. We are working slowly to split things up more, for example into services, but that is a long-term process.
To get a bit abstract for a moment, in an ideal world, it doesn't seem like performance constraints of a source-control-system should dictate how we choose to structure our code. Ideally, seems like we should be able to choose to structure our code in whatever way we feel maximizes developer productivity. If development and code/release management seem easier in a single repo, than why not make an SCM that can handle it? This is one reason I've been leaning towards figuring out an SCM approach that can work well with our current practices rather than changing them as a prerequisite for good SCM performance.
Sam Vilain: Thanks for the pointer, i didn't realize that fast-import was bi-directional. I used it for generating the synthetic repo. Will look into using it the other way around. Though that still won't speed up things like git-blame, presumably? The sparse-checkout issue you mention is a good one. There is a good question of how to support quick checkout, branch switching, clone, push and so forth. I'll look into the approaches you suggest. One consideration is coming up with a high-leverage approach - i.e. not doing heavy dev work if we can avoid it. On the other hand, it would be nice if we (including the entire community :) ) improve git in areas that others that share similar issues benefit from as well.
Matt Graham: I don't have file stats at the moment. It's mostly code files, with a few larger data files here and there. We also don't do sparse checkouts, primarily because most people use git (whether on top of SVN or not), which doesn't support it.
Chris Lee: When I was building up the repo (e.g., doing lots of commits, before I started using fast-import), i noticed that flash was not much faster - stat'ing the whole repo takes a lot of kernel time, even with flash. My hunch is that we'd see similar issues with other operations, like git-blame.
Zeki Mokhtarzada: Dumping history I think would speed up operations for which we don't care about old history, like git-blame in which we only want to see recent modifications. We'd also need a good story for other kinds of operations. In my mental model of git scalability, I categorize git structures into three kinds: those for reasoning about history, those for the index and those for the working directory (yeah, I know these don't map precisely to actual on-disk things like the object store, including trees, etc.). One scaling approach we've been thinking of is to focus on each individually: develop a specialized thing to handle history commands efficiently (git-blame, git-log, git-diff, etc.), something to speed up or bypass the index, and something to make large changes to the working directly quickly.
Joey Hess: Separating the factors is a good suggestion. My hunch is that the various git operations test the performance issues in isolation. For example, git-status performance depends just on the number of files, not on the depth of history. On the other hand, my guess is that git-blame performance is more a function of the length of history rather than the number of files. Though, certainly with compression and indexing in pack files, I could imagine there being cross-effects between length of history and number of files. The git-status suggestion definitely helps when you know which directory you are concerned about. Often I'm lazy and stat the repo root so I trade-off slowness for being more sure I'm not missing anything.
@Joey, I think you're also touching on a good meta point which is that, there's probably no silver bullet here. If we want git to efficiently handle repos that are large across a number of dimensions (size, # commits, # files, etc.), there's multiple parts of git that would need enhancement of some form.
Nguyen Thai Ngoc Duy: At which point in the test flow should I insert git-update-index? I'm happy to try it out. Will compress index when I next get to a terminal. My guess is it'll compress a bunch. It's also conceivable that, if there were an external interface in git to attach other systems to efficiently report which files have changed (e.g., via file-system integration), it's possible that we could omit managing the index in many cases. I know that would be a big change, but the benefits are intriguing.
Cheers,
Josh
________________________________________
From: Nguyen Thai Ngoc Duy [pclouds@gmail.com]
Sent: Friday, February 03, 2012 10:53 PM
To: Joshua Redstone
Cc: git@vger.kernel.org
Subject: Re: Git performance results on a large repository
On Fri, Feb 3, 2012 at 9:20 PM, Joshua Redstone <joshua.redstone@fb.com> wrote:
> I timed a few common operations with both a warm OS file cache and a cold
> cache. i.e., I did a 'echo 3 | tee /proc/sys/vm/drop_caches' and then did
> the operation in question a few times (first timing is the cold timing,
> the next few are the warm timings). The following results are on a server
> with average hard drive (I.e., not flash) and > 10GB of ram.
>
> 'git status' : 39 minutes cold, and 24 seconds warm.
>
> 'git blame': 44 minutes cold, 11 minutes warm.
>
> 'git add' (appending a few chars to the end of a file and adding it): 7
> seconds cold and 5 seconds warm.
>
> 'git commit -m "foo bar3" --no-verify --untracked-files=no --quiet
> --no-status': 41 minutes cold, 20 seconds warm. I also hacked a version
> of git to remove the three or four places where 'git commit' stats every
> file in the repo, and this dropped the times to 30 minutes cold and 8
> seconds warm.
Have you tried "git update-index --assume-unchaged"? That should
reduce mass lstat() and hopefully improve the above numbers. The
interface is not exactly easy-to-use, but if it has significant gain,
then we can try to improve UI.
On the index size issue, ideally we should make minimum writes to
index instead of rewriting 191 MB index. An improvement we could do
now is to compress it, reduce disk footprint, thus disk I/O. If you
compress the index with gzip, how big is it?
--
Duy
^ permalink raw reply
* [bug] blame duplicates trailing ">" in mailmapped emails
From: Jeff King @ 2012-02-04 18:26 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Junio C Hamano, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <CAMP44s2QdJ4+qgg4fF5-DOWHx3Btd0pTivTT9s_E=qqxg16YLQ@mail.gmail.com>
On Sat, Feb 04, 2012 at 05:46:05PM +0200, Felipe Contreras wrote:
> In any case, the one to blame for the header corruption is git:
> [...]
> f2bb9f88 (<spearce@spearce.org>> 2006-11-27 03:41:01 -0500 952)
>
> Notice the mail is wrong.
Ugh. The fault lies in this code:
$ sed -n 1405,1414p builtin/blame.c
if (map_user(&mailmap, mail+1, mail_len-1, person, tmp-person-1)) {
/* Add a trailing '>' to email, since map_user returns plain emails
Note: It already has '<', since we replace from mail+1 */
mailpos = memchr(mail, '\0', mail_len);
if (mailpos && mailpos-mail < mail_len - 1) {
*mailpos = '>';
*(mailpos+1) = '\0';
}
}
}
But that comment is wrong. If there's no email mapping needed, map_user
will leave the "mail" buffer intact, in which case it will have the
trailing ">" (because we feed the address with enclosing angle
brackets). So while map_user tries to accept either "foo@example.com\0"
and "foo@example.com>", it is up to the contents of the mailmap whether
you get back something with the closing angle bracket or not. Which is a
pretty error-prone interface.
You can fix it with this:
diff --git a/builtin/blame.c b/builtin/blame.c
index 5a67c20..9b886fa 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1406,7 +1406,8 @@ static void get_ac_line(const char *inbuf, const char *what,
/* Add a trailing '>' to email, since map_user returns plain emails
Note: It already has '<', since we replace from mail+1 */
mailpos = memchr(mail, '\0', mail_len);
- if (mailpos && mailpos-mail < mail_len - 1) {
+ if (mailpos && mailpos-mail < mail_len - 1 &&
+ mailpos > mail && *(mailpos-1) != '>') {
*mailpos = '>';
*(mailpos+1) = '\0';
}
but it feels like the fix should go into map_user. I tried a few things,
like "git log -1 --format=%aE", and couldn't find other code paths with
this problem. So presumably they are all feeding email addresses without
the closing ">" (so one option is to just say "map_user needs to get
NUL-terminated strings).
-Peff
^ permalink raw reply related
* [PATCH/RFCv2 5/5] gitweb: Highlight matched part of shortened project description
From: Jakub Narebski @ 2012-02-04 18:56 UTC (permalink / raw)
To: git
In-Reply-To: <1328359648-29511-6-git-send-email-jnareb@gmail.com>
Previous commit make gitweb use esc_html_match_hl() to mark match in
the _whole_ description of a project when searching projects.
This commit makes gitweb highlight match in _shortened_ description,
based on match in whole description, using esc_html_match_hl_chopped()
subroutine.
If match is in removed (chopped) part, even partially, then trailing
"... " is highlighted.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This version removes code duplication from v1.
It is still marked as RFC, because I am not sure if it is right way
to highlight match in shortened string, or if we better use full string,
or full string if match is in chopped part.
gitweb/gitweb.perl | 40 ++++++++++++++++++++++++++++++++++++++--
1 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c650268..f71afe0 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1720,11 +1720,46 @@ sub esc_html_match_hl {
my ($str, $regexp) = @_;
return esc_html($str) unless defined $regexp;
+ return esc_html_match_hl_chopped($str, undef, $regexp);
+}
+
+
+# highlight match (if any) of shortened string, and escape HTML
+sub esc_html_match_hl_chopped {
+ my ($str, $chopped, $regexp) = @_;
+ return esc_html($chopped) unless defined $regexp;
+
my @matches;
while ($str =~ /$regexp/g) {
push @matches, [$-[0], $+[0]];
}
- return esc_html($str) unless @matches;
+ return esc_html(defined $chopped ? $chopped : $str) unless @matches;
+
+ # filter matches so that we mark chopped string, if it is present
+ if (defined $chopped) {
+ my $tail = "... "; # see chop_str
+ unless ($chopped =~ s/\Q$tail\E$//) {
+ $tail = '';
+ }
+ my $chop_len = length($chopped);
+ my $tail_len = length($tail);
+ my @filtered;
+
+ for my $m (@matches) {
+ if ($m->[0] > $chop_len) {
+ push @filtered, [ $chop_len, $chop_len + $tail_len ] if ($tail_len > 0);
+ last;
+ } elsif ($m->[1] > $chop_len) {
+ push @filtered, [ $m->[0], $chop_len + $tail_len ];
+ last;
+ }
+ push @filtered, $m;
+ }
+
+ # further operations are on chopped string
+ $str = $chopped . $tail;
+ @matches = @filtered;
+ }
my $out = '';
my $pos = 0;
@@ -5372,7 +5407,8 @@ sub git_project_list_rows {
"<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
-class => "list", -title => $pr->{'descr_long'}},
$search_regexp
- ? esc_html_match_hl($pr->{'descr_long'}, $search_regexp)
+ ? esc_html_match_hl_chopped($pr->{'descr_long'},
+ $pr->{'descr'}, $search_regexp)
: esc_html($pr->{'descr'})) .
"</td>\n" .
"<td><i>" . chop_and_escape_str($pr->{'owner'}, 15) . "</i></td>\n";
--
1.7.9
^ permalink raw reply related
* Re: [PATCH 0/9] respect binary attribute in grep
From: Pete Wyckoff @ 2012-02-04 19:22 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Thomas Rast, Conrad Irwin, git,
Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <20120202081747.GA10271@sigill.intra.peff.net>
I took a look at this series. It's nice. My worry was that the
extra open() of non-existent .gitattributes files in all the
directories would cause performance problems across networked
filesystems like NFS.
My usual (non-public) repository has order:
100k files
10k directories
and no files marked as binary. The grep string is such that it
is disk-bound, and not expected to match in any file (or binary):
"time ~/src/git/bin-wrappers/git grep unfindable-string".
With your change, there are 10k new open() calls looking for
.gitattributes in each directory, all of which return ENOENT.
This turns out to have an insignificant impact on performance due
to the much bigger time sink of stat()-ing all the files.
I think this happens to be true because the gitattributes lookups
run in parallel to all the file stat work, as the main thread
dispatches file work while doing its own gitattributes lookups.
It could be plausible that deep directory structures with few
grep-able files will suffer with this change. For example, many
big binary blobs in deep directory hierarchies, but also some
useful files here and there.
One could argue that with the use of .gitattributes to specify
which blobs should not be searched, this series makes this faster
by not having to to read the binary blobs at all. And I'd be
okay with that.
Just FYI that there may be a performance impact on certain
repositories.
-- Pete
^ permalink raw reply
* Re: [bug] blame duplicates trailing ">" in mailmapped emails
From: Felipe Contreras @ 2012-02-04 19:30 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Jonathan Nieder, git, SZEDER Gábor
In-Reply-To: <20120204182611.GA31091@sigill.intra.peff.net>
On Sat, Feb 4, 2012 at 8:26 PM, Jeff King <peff@peff.net> wrote:
> On Sat, Feb 04, 2012 at 05:46:05PM +0200, Felipe Contreras wrote:
>
>> In any case, the one to blame for the header corruption is git:
>> [...]
>> f2bb9f88 (<spearce@spearce.org>> 2006-11-27 03:41:01 -0500 952)
>>
>> Notice the mail is wrong.
>
> Ugh. The fault lies in this code:
Yes, I found that as well.
> But that comment is wrong. If there's no email mapping needed, map_user
> will leave the "mail" buffer intact, in which case it will have the
> trailing ">" (because we feed the address with enclosing angle
> brackets). So while map_user tries to accept either "foo@example.com\0"
> and "foo@example.com>", it is up to the contents of the mailmap whether
> you get back something with the closing angle bracket or not. Which is a
> pretty error-prone interface.
>
> You can fix it with this:
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 5a67c20..9b886fa 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1406,7 +1406,8 @@ static void get_ac_line(const char *inbuf, const char *what,
> /* Add a trailing '>' to email, since map_user returns plain emails
> Note: It already has '<', since we replace from mail+1 */
> mailpos = memchr(mail, '\0', mail_len);
> - if (mailpos && mailpos-mail < mail_len - 1) {
> + if (mailpos && mailpos-mail < mail_len - 1 &&
> + mailpos > mail && *(mailpos-1) != '>') {
> *mailpos = '>';
> *(mailpos+1) = '\0';
> }
Yes, I already did this. I'm writing tests for this right now, but I
think I found yet another bug...
> but it feels like the fix should go into map_user. I tried a few things,
> like "git log -1 --format=%aE", and couldn't find other code paths with
> this problem. So presumably they are all feeding email addresses without
> the closing ">" (so one option is to just say "map_user needs to get
> NUL-terminated strings).
Perhaps, but I though the idea was to make it efficient. I think the
above fix should be ok.
We should have tests for this though, to make sure it doesn't get
broken again. I'm on that.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* [RFD] Rewriting safety - warn before/when rewriting published history
From: Jakub Narebski @ 2012-02-04 19:45 UTC (permalink / raw)
To: git
Git includes protection against rewriting published history on the
receive side with fast-forward check by default (which can be
overridden) and various receive.deny* configuration variables,
including receive.denyNonFastForwards.
Nevertheless git users requested (among others in Git User's Survey)
more help on creation side, namely preventing rewriting parts of
history which was already made public (or at least warning that one is
about to rewrite published history). The "warn before/when rewriting
published history" answer in "17. Which of the following features would
you like to see implemented in git?" multiple-choice question in latest
Git User's Survey 2011[1] got 24% (1525) responses.
[1]: https://www.survs.com/results/Q5CA9SKQ/P7DE07F0PL
So people would like for git to warn them about rewriting history before
they attempt a push and it turns out to not fast-forward.
What prompted this email is the fact that Mercurial includes support for
tracking which revisions (changesets) are safe to modify in its 2.1
latest version:
http://lwn.net/Articles/478795/
http://mercurial.selenic.com/wiki/WhatsNew
It does that by tracking so called "phase" of a changeset (revision).
http://mercurial.selenic.com/wiki/Phases
http://mercurial.selenic.com/wiki/PhasesDevel
http://www.logilab.org/blogentry/88203
http://www.logilab.org/blogentry/88219
http://www.logilab.org/blogentry/88259
While we don't have to play catch-up with Mercurial features, I think
something similar to what Mercurial has to warn about rewriting
published history (amend, rebase, perhaps even filter-branch) would
be nice to have. Perhaps even follow UI used by Mercurial, and/or
translating its implementation into git terms.
In Mercurial 2.1 there are three available phases: 'public' for
published commits, 'draft' for local un-published commits and
'secret' for local un-published commits which are not meant to
be published.
The phase of a changeset is always equal to or higher than the phase
of it's descendants, according to the following order:
public < draft < secret
Commits start life as 'draft', and move to 'public' on push.
Mercurial documentation talks about phase of a commit, which might
be a good UI, ut also about commits in 'public' phase being "immutable".
As commits in Git are immutable, and rewriting history is in fact
re-doing commits, this description should probably be changed.
While default "push matching" behavior makes it possible to have
"secret" commits, being able to explicitly mark commits as not for
publishing might be a good idea also for Git.
What do you think about this?
--
Jakub Narebski
Poland
^ permalink raw reply
* [PATCH 0/3] blame: fix output with mailmap
From: Felipe Contreras @ 2012-02-04 19:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras
The fix, and the tests.
Felipe Contreras (3):
blame: fix email output with mailmap
t: mailmap: add 'git blame -e' tests
t: mailmap: add simple name translation test
builtin/blame.c | 9 ++++++---
t/t4203-mailmap.sh | 32 ++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 3 deletions(-)
--
1.7.9.1.g97f7d
^ permalink raw reply
* [PATCH 1/3] blame: fix email output with mailmap
From: Felipe Contreras @ 2012-02-04 19:50 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Felipe Contreras, Brian Gianforcaro,
Marius Storm-Olsen, Junio C Hamano
In-Reply-To: <1328385024-6955-1-git-send-email-felipe.contreras@gmail.com>
An extra '>' is added in some cases (<spearce@spearce.org>>), for
example:
% git blame -e -L 947,+7 contrib/completion/git-completion.bash v1.7.9
The current code assumes map_user() *always* returns plain emails, but
that's not true; when there's no email substitution (only name),
map_user() would return 1, but don't touch the mail.
Also, add some tests.
This got broken by d20d654[1].
[1] Change current mailmap usage to do matching on both name and email
of author/committer.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/blame.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 5a67c20..dd69e51 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1403,10 +1403,13 @@ static void get_ac_line(const char *inbuf, const char *what,
* Now, convert both name and e-mail using mailmap
*/
if (map_user(&mailmap, mail+1, mail_len-1, person, tmp-person-1)) {
- /* Add a trailing '>' to email, since map_user returns plain emails
- Note: It already has '<', since we replace from mail+1 */
+ /*
+ * Add a trailing '>' to email, since map_user returns plain
+ * emails when it finds a matching mail.
+ * Note: It already has '<', since we replace from mail + 1
+ */
mailpos = memchr(mail, '\0', mail_len);
- if (mailpos && mailpos-mail < mail_len - 1) {
+ if (mailpos && mailpos-mail < mail_len - 1 && *(mailpos - 1) != '>') {
*mailpos = '>';
*(mailpos+1) = '\0';
}
--
1.7.9.1.g97f7d
^ permalink raw reply related
* [PATCH 3/3] t: mailmap: add simple name translation test
From: Felipe Contreras @ 2012-02-04 19:50 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Felipe Contreras, Marius Storm-Olsen,
Jim Meyering, Jonathan Nieder
In-Reply-To: <1328385024-6955-1-git-send-email-felipe.contreras@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t4203-mailmap.sh | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index b1bc521..589e39e 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -157,6 +157,9 @@ A U Thor <author@example.com> (1):
CTO <cto@company.xx> (1):
seventh
+Mr. Right <right@company.xx> (1):
+ eight
+
Other Author <other@author.xx> (2):
third
fourth
@@ -196,6 +199,11 @@ test_expect_success 'Shortlog output (complex mapping)' '
test_tick &&
git commit --author "CTO <cto@coompany.xx>" -m seventh &&
+ echo eight >>one &&
+ git add one &&
+ test_tick &&
+ git commit --author "Wrong <right@company.xx>" -m eight &&
+
mkdir -p internal_mailmap &&
echo "Committed <committer@example.com>" > internal_mailmap/.mailmap &&
echo "<cto@company.xx> <cto@coompany.xx>" >> internal_mailmap/.mailmap &&
@@ -204,6 +212,7 @@ test_expect_success 'Shortlog output (complex mapping)' '
echo "Other Author <other@author.xx> <nick2@company.xx>" >> internal_mailmap/.mailmap &&
echo "Santa Claus <santa.claus@northpole.xx> <me@company.xx>" >> internal_mailmap/.mailmap &&
echo "Santa Claus <santa.claus@northpole.xx> <me@company.xx>" >> internal_mailmap/.mailmap &&
+ echo "Mr. Right <right@company.xx>" >> internal_mailmap/.mailmap &&
git shortlog -e HEAD >actual &&
test_cmp expect actual
@@ -212,6 +221,9 @@ test_expect_success 'Shortlog output (complex mapping)' '
# git log with --pretty format which uses the name and email mailmap placemarkers
cat >expect <<\EOF
+Author Wrong <right@company.xx> maps to Mr. Right <right@company.xx>
+Committer C O Mitter <committer@example.com> maps to Committed <committer@example.com>
+
Author CTO <cto@coompany.xx> maps to CTO <cto@company.xx>
Committer C O Mitter <committer@example.com> maps to Committed <committer@example.com>
@@ -248,6 +260,7 @@ OBJID (Other Author DATE 4) four
OBJID (Santa Claus DATE 5) five
OBJID (Santa Claus DATE 6) six
OBJID (CTO DATE 7) seven
+OBJID (Mr. Right DATE 8) eight
EOF
test_expect_success 'Blame output (complex mapping)' '
git blame one >actual &&
@@ -264,6 +277,7 @@ OBJID (<other@author.xx> DATE 4) four
OBJID (<santa.claus@northpole.xx> DATE 5) five
OBJID (<santa.claus@northpole.xx> DATE 6) six
OBJID (<cto@company.xx> DATE 7) seven
+OBJID (<right@company.xx> DATE 8) eight
EOF
test_expect_success 'Blame output (complex mapping)' '
git blame -e one >actual &&
--
1.7.9.1.g97f7d
^ permalink raw reply related
* [PATCH 2/3] t: mailmap: add 'git blame -e' tests
From: Felipe Contreras @ 2012-02-04 19:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Felipe Contreras, Jonathan Nieder,
Marius Storm-Olsen
In-Reply-To: <1328385024-6955-1-git-send-email-felipe.contreras@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t4203-mailmap.sh | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 1f182f6..b1bc521 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -255,4 +255,22 @@ test_expect_success 'Blame output (complex mapping)' '
test_cmp expect actual.fuzz
'
+# git blame
+cat >expect <<\EOF
+^OBJI (<author@example.com> DATE 1) one
+OBJID (<some@dude.xx> DATE 2) two
+OBJID (<other@author.xx> DATE 3) three
+OBJID (<other@author.xx> DATE 4) four
+OBJID (<santa.claus@northpole.xx> DATE 5) five
+OBJID (<santa.claus@northpole.xx> DATE 6) six
+OBJID (<cto@company.xx> DATE 7) seven
+EOF
+test_expect_success 'Blame output (complex mapping)' '
+ git blame -e one >actual &&
+ cp actual /tmp &&
+ cp internal_mailmap/.mailmap /tmp &&
+ fuzz_blame actual >actual.fuzz &&
+ test_cmp expect actual.fuzz
+'
+
test_done
--
1.7.9.1.g97f7d
^ permalink raw reply related
* RE: Git performance results on a large repository
From: Joshua Redstone @ 2012-02-04 20:05 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git@vger.kernel.org
In-Reply-To: <CACsJy8DkLCK0ZUKNz_PJazsxjsRbWVVZwjAU5n2EAjJfCYtpoQ@mail.gmail.com>
One more follow-on thought. I imagine that most consumers of git are nowhere near the scale of the test repo that I described. They may still enjoy benefit from efforts to improve git support for large repos. A few possible reasons:
1. The performance improvements should speed things up for smaller repos as well.
2. They may find their repos growing to a 'large scale' at some point in the future.
3. Any code cleanup as part of an effort to support git scalability is good for code base health and e.g., would facilitate future modifications that may more directly affect them.
Cheers,
Josh
________________________________________
From: Nguyen Thai Ngoc Duy [pclouds@gmail.com]
Sent: Friday, February 03, 2012 10:53 PM
To: Joshua Redstone
Cc: git@vger.kernel.org
Subject: Re: Git performance results on a large repository
On Fri, Feb 3, 2012 at 9:20 PM, Joshua Redstone <joshua.redstone@fb.com> wrote:
> I timed a few common operations with both a warm OS file cache and a cold
> cache. i.e., I did a 'echo 3 | tee /proc/sys/vm/drop_caches' and then did
> the operation in question a few times (first timing is the cold timing,
> the next few are the warm timings). The following results are on a server
> with average hard drive (I.e., not flash) and > 10GB of ram.
>
> 'git status' : 39 minutes cold, and 24 seconds warm.
>
> 'git blame': 44 minutes cold, 11 minutes warm.
>
> 'git add' (appending a few chars to the end of a file and adding it): 7
> seconds cold and 5 seconds warm.
>
> 'git commit -m "foo bar3" --no-verify --untracked-files=no --quiet
> --no-status': 41 minutes cold, 20 seconds warm. I also hacked a version
> of git to remove the three or four places where 'git commit' stats every
> file in the repo, and this dropped the times to 30 minutes cold and 8
> seconds warm.
Have you tried "git update-index --assume-unchaged"? That should
reduce mass lstat() and hopefully improve the above numbers. The
interface is not exactly easy-to-use, but if it has significant gain,
then we can try to improve UI.
On the index size issue, ideally we should make minimum writes to
index instead of rewriting 191 MB index. An improvement we could do
now is to compress it, reduce disk footprint, thus disk I/O. If you
compress the index with gzip, how big is it?
--
Duy
^ permalink raw reply
* How to change the index for git status?
From: Peng Yu @ 2012-02-04 20:06 UTC (permalink / raw)
To: git
Hi,
When I run git status, it show the status compared against the branch
upstream/develop. I want it to compare against origin/develop. I
checked progit. But I don't see how to change the branch to compare
to. Could anybody let me know how to do so? Thanks!
~/dvcs_src/craftyjs1/Crafty/build/api$ git status
# On branch 2D_doc_dev
# Your branch is ahead of 'upstream/develop' by 2 commits.
#
nothing to commit (working directory clean)
--
Regards,
Peng
^ permalink raw reply
* Re: [PATCH 0/5] gitweb: Faster and imrpoved project search
From: Felipe Contreras @ 2012-02-04 20:07 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <1328359648-29511-1-git-send-email-jnareb@gmail.com>
On Sat, Feb 4, 2012 at 2:47 PM, Jakub Narebski <jnareb@gmail.com> wrote:
Typo: improved
--
Felipe Contreras
^ permalink raw reply
* How to change the index for git status?
From: Peng Yu @ 2012-02-04 20:08 UTC (permalink / raw)
To: git
In-Reply-To: <CABrM6wm2MochqgC0+FByDA-6nmo0XHtZy1FDXJRkc30+2eyiTw@mail.gmail.com>
Hi,
When I run git status, it show the status compared against the branch
upstream/develop. I want it to compare against origin/develop. I
checked progit. But I don't see how to change the branch to compare
to. Could anybody let me know how to do so? Thanks!
~/dvcs_src/craftyjs1/Crafty/build/api$ git status
# On branch 2D_doc_dev
# Your branch is ahead of 'upstream/develop' by 2 commits.
#
nothing to commit (working directory clean)
--
Regards,
Peng
^ permalink raw reply
* Re: [PATCH 2/3] t: mailmap: add 'git blame -e' tests
From: Jonathan Nieder @ 2012-02-04 20:10 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Marius Storm-Olsen
In-Reply-To: <1328385024-6955-3-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras wrote:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Thanks for writing tests. I think there is room for a few lines of
explanation above.
[...]
> --- a/t/t4203-mailmap.sh
> +++ b/t/t4203-mailmap.sh
> @@ -255,4 +255,22 @@ test_expect_success 'Blame output (complex mapping)' '
> test_cmp expect actual.fuzz
> '
>
> +# git blame
> +cat >expect <<\EOF
> +^OBJI (<author@example.com> DATE 1) one
> +OBJID (<some@dude.xx> DATE 2) two
> +OBJID (<other@author.xx> DATE 3) three
> +OBJID (<other@author.xx> DATE 4) four
> +OBJID (<santa.claus@northpole.xx> DATE 5) five
> +OBJID (<santa.claus@northpole.xx> DATE 6) six
> +OBJID (<cto@company.xx> DATE 7) seven
> +EOF
> +test_expect_success 'Blame output (complex mapping)' '
Since I didn't receive a copy of the cover letter or patch 1, I don't
know what this is intended to test _for_. Good --- I can more easily
convey the reaction of future readers who do not necessarily know the
context in which the patch was written (and the commit message does
not seem to say).
Looking above, I see
- a lone comment "git blame". What is it trying to tell me? I guess
you copy/pasted it, but is there any purpose to it?
- a test asserting the claim "Blame output (complex mapping)". This
title is identical to the test before. I have no idea what this is
about.
Puzzled,
Jonathan
^ permalink raw reply
* Re: [PATCH 3/3] t: mailmap: add simple name translation test
From: Jonathan Nieder @ 2012-02-04 20:12 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Marius Storm-Olsen, Jim Meyering
In-Reply-To: <1328385024-6955-4-git-send-email-felipe.contreras@gmail.com>
Felipe Contreras wrote:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Similar comments to the last patch apply here. This time the patch
is even more mysterious, since it seems to touch a number of test
assertions, even while I assume not all of them relate to whatever
this is supposed to check for.
^ permalink raw reply
* Re: [PATCH 0/5] gitweb: Faster and imrpoved project search
From: Jakub Narebski @ 2012-02-04 20:59 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <CAMP44s2gLvXXCTTpAV78=DeJA9dSV793+bx=yJmns7vCwegagQ@mail.gmail.com>
On Sat, 4 Feb 2012, Felipe Contreras wrote:
>
> Typo: improved
Yeah, I have noticed this just as I have send it, but because it is in
_cover letter_ rather than in commit message...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 2/3] t: mailmap: add 'git blame -e' tests
From: Felipe Contreras @ 2012-02-04 21:04 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C Hamano, Marius Storm-Olsen
In-Reply-To: <20120204201027.GE22928@burratino>
On Sat, Feb 4, 2012 at 10:10 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Since I didn't receive a copy of the cover letter or patch 1, I don't
> know what this is intended to test _for_. Good --- I can more easily
> convey the reaction of future readers who do not necessarily know the
> context in which the patch was written (and the commit message does
> not seem to say).
>
> Looking above, I see
>
> - a lone comment "git blame". What is it trying to tell me? I guess
> you copy/pasted it, but is there any purpose to it?
>
> - a test asserting the claim "Blame output (complex mapping)". This
> title is identical to the test before. I have no idea what this is
> about.
Look at the title:
add 'git blame -e' tests
s/blame/blame -e/
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 3/3] t: mailmap: add simple name translation test
From: Felipe Contreras @ 2012-02-04 21:05 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C Hamano, Marius Storm-Olsen, Jim Meyering
In-Reply-To: <20120204201218.GF22928@burratino>
On Sat, Feb 4, 2012 at 10:12 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>
> Similar comments to the last patch apply here. This time the patch
> is even more mysterious, since it seems to touch a number of test
> assertions, even while I assume not all of them relate to whatever
> this is supposed to check for.
Title: mailmap: add simple name translation test
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH 2/3] t: mailmap: add 'git blame -e' tests
From: Jonathan Nieder @ 2012-02-04 21:13 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Marius Storm-Olsen
In-Reply-To: <CAMP44s3tmiPGgAUakUgoen2aJcsKw4CygtF5f=4x2dxNTrGbGA@mail.gmail.com>
Felipe Contreras wrote:
> On Sat, Feb 4, 2012 at 10:10 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Since I didn't receive a copy of the cover letter or patch 1, I don't
>> know what this is intended to test _for_. Good --- I can more easily
>> convey the reaction of future readers who do not necessarily know the
>> context in which the patch was written (and the commit message does
>> not seem to say).
[...]
> Look at the title:
> add 'git blame -e' tests
>
> s/blame/blame -e/
And? After copy/pasting this particular test with that substitution,
what do we get a test for? What class of problem is it supposed to
catch? I do not think a sentence or two is too much to ask for.
By the way, "I blindly copy/pasted" does not seem like a very sensible
excuse for writing meaningless code (such as the "# git blame" comment
line). Before the code contained one riddle. Afterwards it has two.
Hope that helps,
Jonathan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox