* [take 2] revision parsing made incremental
From: Pierre Habouzit @ 2008-07-08 13:19 UTC (permalink / raw)
To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin
Following Dscho's remarks, I reworked the series to avoid changing
setup_revisions semantics for now, and only exposed the part that groks
options (and keep pseudo revision arguments out).
It indeed makes the series smaller, even if the first patch is quite
long to read, and is just enough for simplifying git-blame in a very
satisfying way.
The series passes the testsuite, has no know blanks issues, and is
pushed to my public repository.
^ permalink raw reply
* [PATCH 2/3] git-blame: migrate to incremental parse-option [1/2]
From: Pierre Habouzit @ 2008-07-08 13:19 UTC (permalink / raw)
To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin, Pierre Habouzit
In-Reply-To: <1215523175-28436-2-git-send-email-madcoder@debian.org>
This step merely moves the parser to an incremental version, still using
parse_revisions.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
builtin-blame.c | 220 +++++++++++++++++++++++++++++-------------------------
1 files changed, 118 insertions(+), 102 deletions(-)
diff --git a/builtin-blame.c b/builtin-blame.c
index cf41511..5e82cd3 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -18,24 +18,16 @@
#include "cache-tree.h"
#include "path-list.h"
#include "mailmap.h"
+#include "parse-options.h"
-static char blame_usage[] =
-"git-blame [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-p] [-w] [-L n,m] [-S <revs-file>] [-M] [-C] [-C] [--contents <filename>] [--incremental] [commit] [--] file\n"
-" -c Use the same output mode as git-annotate (Default: off)\n"
-" -b Show blank SHA-1 for boundary commits (Default: off)\n"
-" -l Show long commit SHA1 (Default: off)\n"
-" --root Do not treat root commits as boundaries (Default: off)\n"
-" -t Show raw timestamp (Default: off)\n"
-" -f, --show-name Show original filename (Default: auto)\n"
-" -n, --show-number Show original linenumber (Default: off)\n"
-" -s Suppress author name and timestamp (Default: off)\n"
-" -p, --porcelain Show in a format designed for machine consumption\n"
-" -w Ignore whitespace differences\n"
-" -L n,m Process only line range n,m, counting from 1\n"
-" -M, -C Find line movements within and across files\n"
-" --incremental Show blame entries as we find them, incrementally\n"
-" --contents file Use <file>'s contents as the final image\n"
-" -S revs-file Use revisions from revs-file instead of calling git-rev-list\n";
+static char blame_usage[] = "git-blame [options] [rev-opts] [rev] [--] file";
+
+static const char *blame_opt_usage[] = {
+ blame_usage,
+ "",
+ "[rev-opts] are documented in git-rev-parse(1)",
+ NULL
+};
static int longest_file;
static int longest_author;
@@ -2219,6 +2211,50 @@ static const char *prepare_initial(struct scoreboard *sb)
return final_commit_name;
}
+static int blame_copy_callback(const struct option *option, const char *arg, int unset)
+{
+ int *opt = option->value;
+
+ /*
+ * -C enables copy from removed files;
+ * -C -C enables copy from existing files, but only
+ * when blaming a new file;
+ * -C -C -C enables copy from existing files for
+ * everybody
+ */
+ if (*opt & PICKAXE_BLAME_COPY_HARDER)
+ *opt |= PICKAXE_BLAME_COPY_HARDEST;
+ if (*opt & PICKAXE_BLAME_COPY)
+ *opt |= PICKAXE_BLAME_COPY_HARDER;
+ *opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;
+
+ if (arg)
+ blame_copy_score = parse_score(arg);
+ return 0;
+}
+
+static int blame_move_callback(const struct option *option, const char *arg, int unset)
+{
+ int *opt = option->value;
+
+ *opt |= PICKAXE_BLAME_MOVE;
+
+ if (arg)
+ blame_move_score = parse_score(arg);
+ return 0;
+}
+
+static int blame_bottomtop_callback(const struct option *option, const char *arg, int unset)
+{
+ const char **bottomtop = option->value;
+ if (!arg)
+ return -1;
+ if (*bottomtop)
+ die("More than one '-L n,m' option given");
+ *bottomtop = arg;
+ return 0;
+}
+
int cmd_blame(int argc, const char **argv, const char *prefix)
{
struct rev_info revs;
@@ -2226,98 +2262,79 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
struct scoreboard sb;
struct origin *o;
struct blame_entry *ent;
- int i, seen_dashdash, unk, opt;
+ int i, seen_dashdash, unk;
long bottom, top, lno;
- int output_option = 0;
- int show_stats = 0;
- const char *revs_file = NULL;
const char *final_commit_name = NULL;
enum object_type type;
- const char *bottomtop = NULL;
- const char *contents_from = NULL;
+
+ static const char *bottomtop = NULL;
+ static int output_option = 0, opt = 0;
+ static int show_stats = 0;
+ static const char *revs_file = NULL;
+ static const char *contents_from = NULL;
+ static const struct option options[] = {
+ OPT_BOOLEAN(0, "incremental", &incremental, "Show blame entries as we find them, incrementally"),
+ OPT_BOOLEAN('b', NULL, &blank_boundary, "Show blank SHA-1 for boundary commits (Default: off)"),
+ OPT_BOOLEAN(0, "root", &show_root, "Do not treat root commits as boundaries (Default: off)"),
+ OPT_BOOLEAN(0, "show-stats", &show_stats, "Show work cost statistics"),
+ OPT_BIT(0, "score-debug", &output_option, "Show output score for blame entries", OUTPUT_SHOW_SCORE),
+ OPT_BIT('f', "show-name", &output_option, "Show original filename (Default: auto)", OUTPUT_SHOW_NAME),
+ OPT_BIT('n', "show-number", &output_option, "Show original linenumber (Default: off)", OUTPUT_SHOW_NUMBER),
+ OPT_BIT('p', "porcelain", &output_option, "Show in a format designed for machine consumption", OUTPUT_PORCELAIN),
+ OPT_BIT('c', NULL, &output_option, "Use the same output mode as git-annotate (Default: off)", OUTPUT_ANNOTATE_COMPAT),
+ OPT_BIT('t', NULL, &output_option, "Show raw timestamp (Default: off)", OUTPUT_RAW_TIMESTAMP),
+ OPT_BIT('l', NULL, &output_option, "Show long commit SHA1 (Default: off)", OUTPUT_LONG_OBJECT_NAME),
+ OPT_BIT('s', NULL, &output_option, "Suppress author name and timestamp (Default: off)", OUTPUT_NO_AUTHOR),
+ OPT_BIT('w', NULL, &xdl_opts, "Ignore whitespace differences", XDF_IGNORE_WHITESPACE),
+ OPT_STRING('S', NULL, &revs_file, "file", "Use revisions from <file> instead of calling git-rev-list"),
+ OPT_STRING(0, "contents", &contents_from, "file", "Use <file>'s contents as the final image"),
+ { OPTION_CALLBACK, 'C', NULL, &opt, "score", "Find line copies within and across files", PARSE_OPT_OPTARG, blame_copy_callback },
+ { OPTION_CALLBACK, 'M', NULL, &opt, "score", "Find line movements within and across files", PARSE_OPT_OPTARG, blame_move_callback },
+ OPT_CALLBACK('L', NULL, &bottomtop, "n,m", "Process only line range n,m, counting from 1", blame_bottomtop_callback),
+ OPT_END()
+ };
+
+ struct parse_opt_ctx_t ctx;
cmd_is_annotate = !strcmp(argv[0], "annotate");
git_config(git_blame_config, NULL);
+ init_revisions(&revs, NULL);
save_commit_buffer = 0;
- opt = 0;
+ parse_options_start(&ctx, argc, argv, PARSE_OPT_KEEP_DASHDASH |
+ PARSE_OPT_KEEP_ARGV0);
+ for (;;) {
+ int n;
+
+ switch (parse_options_step(&ctx, options, blame_opt_usage)) {
+ case PARSE_OPT_HELP:
+ exit(129);
+ case PARSE_OPT_DONE:
+ goto parse_done;
+ }
+
+ if (!strcmp(ctx.argv[0], "--reverse")) {
+ ctx.argv[0] = "--children";
+ reverse = 1;
+ }
+ n = handle_revision_opt(&revs, ctx.argc, ctx.argv,
+ &ctx.cpidx, ctx.out);
+ if (n <= 0) {
+ error("unknown option `%s'", ctx.argv[0]);
+ usage_with_options(blame_opt_usage, options);
+ }
+ ctx.argv += n;
+ ctx.argc -= n;
+ }
+parse_done:
+ argc = parse_options_end(&ctx);
+
seen_dashdash = 0;
for (unk = i = 1; i < argc; i++) {
const char *arg = argv[i];
if (*arg != '-')
break;
- else if (!strcmp("-b", arg))
- blank_boundary = 1;
- else if (!strcmp("--root", arg))
- show_root = 1;
- else if (!strcmp("--reverse", arg)) {
- argv[unk++] = "--children";
- reverse = 1;
- }
- else if (!strcmp(arg, "--show-stats"))
- show_stats = 1;
- else if (!strcmp("-c", arg))
- output_option |= OUTPUT_ANNOTATE_COMPAT;
- else if (!strcmp("-t", arg))
- output_option |= OUTPUT_RAW_TIMESTAMP;
- else if (!strcmp("-l", arg))
- output_option |= OUTPUT_LONG_OBJECT_NAME;
- else if (!strcmp("-s", arg))
- output_option |= OUTPUT_NO_AUTHOR;
- else if (!strcmp("-w", arg))
- xdl_opts |= XDF_IGNORE_WHITESPACE;
- else if (!strcmp("-S", arg) && ++i < argc)
- revs_file = argv[i];
- else if (!prefixcmp(arg, "-M")) {
- opt |= PICKAXE_BLAME_MOVE;
- blame_move_score = parse_score(arg+2);
- }
- else if (!prefixcmp(arg, "-C")) {
- /*
- * -C enables copy from removed files;
- * -C -C enables copy from existing files, but only
- * when blaming a new file;
- * -C -C -C enables copy from existing files for
- * everybody
- */
- if (opt & PICKAXE_BLAME_COPY_HARDER)
- opt |= PICKAXE_BLAME_COPY_HARDEST;
- if (opt & PICKAXE_BLAME_COPY)
- opt |= PICKAXE_BLAME_COPY_HARDER;
- opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;
- blame_copy_score = parse_score(arg+2);
- }
- else if (!prefixcmp(arg, "-L")) {
- if (!arg[2]) {
- if (++i >= argc)
- usage(blame_usage);
- arg = argv[i];
- }
- else
- arg += 2;
- if (bottomtop)
- die("More than one '-L n,m' option given");
- bottomtop = arg;
- }
- else if (!strcmp("--contents", arg)) {
- if (++i >= argc)
- usage(blame_usage);
- contents_from = argv[i];
- }
- else if (!strcmp("--incremental", arg))
- incremental = 1;
- else if (!strcmp("--score-debug", arg))
- output_option |= OUTPUT_SHOW_SCORE;
- else if (!strcmp("-f", arg) ||
- !strcmp("--show-name", arg))
- output_option |= OUTPUT_SHOW_NAME;
- else if (!strcmp("-n", arg) ||
- !strcmp("--show-number", arg))
- output_option |= OUTPUT_SHOW_NUMBER;
- else if (!strcmp("-p", arg) ||
- !strcmp("--porcelain", arg))
- output_option |= OUTPUT_PORCELAIN;
else if (!strcmp("--", arg)) {
seen_dashdash = 1;
i++;
@@ -2364,16 +2381,16 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
if (seen_dashdash) {
/* (1) */
if (argc <= i)
- usage(blame_usage);
+ usage_with_options(blame_opt_usage, options);
path = add_prefix(prefix, argv[i]);
if (i + 1 == argc - 1) {
if (unk != 1)
- usage(blame_usage);
+ usage_with_options(blame_opt_usage, options);
argv[unk++] = argv[i + 1];
}
else if (i + 1 != argc)
/* garbage at end */
- usage(blame_usage);
+ usage_with_options(blame_opt_usage, options);
}
else {
int j;
@@ -2383,7 +2400,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
if (seen_dashdash) {
/* (2) */
if (seen_dashdash + 1 != argc - 1)
- usage(blame_usage);
+ usage_with_options(blame_opt_usage, options);
path = add_prefix(prefix, argv[seen_dashdash + 1]);
for (j = i; j < seen_dashdash; j++)
argv[unk++] = argv[j];
@@ -2391,7 +2408,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
else {
/* (3) */
if (argc <= i)
- usage(blame_usage);
+ usage_with_options(blame_opt_usage, options);
path = add_prefix(prefix, argv[i]);
if (i + 1 == argc - 1) {
final_commit_name = argv[i + 1];
@@ -2405,7 +2422,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
}
}
else if (i != argc - 1)
- usage(blame_usage); /* garbage at end */
+ usage_with_options(blame_opt_usage, options);
setup_work_tree();
if (!has_path_in_work_tree(path))
@@ -2424,7 +2441,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
argv[unk++] = "--"; /* terminate the rev name */
argv[unk] = NULL;
- init_revisions(&revs, NULL);
setup_revisions(unk, argv, &revs, NULL);
memset(&sb, 0, sizeof(sb));
--
1.5.6.2.396.gca539
^ permalink raw reply related
* [PATCH 1/3] revisions: split handle_revision_opt from setup_revisions.
From: Pierre Habouzit @ 2008-07-08 13:19 UTC (permalink / raw)
To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin, Pierre Habouzit
In-Reply-To: <1215523175-28436-1-git-send-email-madcoder@debian.org>
struct rev_info gains two new field:
* .def to store --default argument;
* .show_merge 1-bit field.
handle_revision_opt is able to deal with any revision option, and consumes
them, and leaves revision arguments or pseudo arguments (like --all,
--not, ...) in place.
For now setup_revisions does a pass of handle_revision_opt again so that
code not using it in a parse-opt parser still work the same.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
revision.c | 612 ++++++++++++++++++++++++++++--------------------------------
revision.h | 4 +
2 files changed, 290 insertions(+), 326 deletions(-)
diff --git a/revision.c b/revision.c
index 0191160..3dbb7a8 100644
--- a/revision.c
+++ b/revision.c
@@ -974,6 +974,276 @@ static void add_ignore_packed(struct rev_info *revs, const char *name)
revs->ignore_packed[num] = NULL;
}
+int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
+ int *unkc, const char **unkv)
+{
+ const char *arg = argv[0];
+
+ /* pseudo revision arguments */
+ if (!strcmp(arg, "--all") || !strcmp(arg, "--branches") ||
+ !strcmp(arg, "--tags") || !strcmp(arg, "--remotes") ||
+ !strcmp(arg, "--reflog") || !strcmp(arg, "--not") ||
+ !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk"))
+ {
+ unkv[(*unkc)++] = arg;
+ return 0;
+ }
+
+ if (!prefixcmp(arg, "--max-count=")) {
+ revs->max_count = atoi(arg + 12);
+ }
+ else if (!prefixcmp(arg, "--skip=")) {
+ revs->skip_count = atoi(arg + 7);
+ }
+ /* accept -<digit>, like traditional "head" */
+ else if ((*arg == '-') && isdigit(arg[1])) {
+ revs->max_count = atoi(arg + 1);
+ }
+ else if (!strcmp(arg, "-n")) {
+ if (argc <= 1)
+ return error("-n requires an argument");
+ revs->max_count = atoi(argv[1]);
+ return 2;
+ }
+ else if (!prefixcmp(arg, "-n")) {
+ revs->max_count = atoi(arg + 2);
+ }
+ else if (!prefixcmp(arg, "--max-age=")) {
+ revs->max_age = atoi(arg + 10);
+ }
+ else if (!prefixcmp(arg, "--since=")) {
+ revs->max_age = approxidate(arg + 8);
+ }
+ else if (!prefixcmp(arg, "--after=")) {
+ revs->max_age = approxidate(arg + 8);
+ }
+ else if (!prefixcmp(arg, "--min-age=")) {
+ revs->min_age = atoi(arg + 10);
+ }
+ else if (!prefixcmp(arg, "--before=")) {
+ revs->min_age = approxidate(arg + 9);
+ }
+ else if (!prefixcmp(arg, "--until=")) {
+ revs->min_age = approxidate(arg + 8);
+ }
+ else if (!strcmp(arg, "--first-parent")) {
+ revs->first_parent_only = 1;
+ }
+ else if (!strcmp(arg, "-g") ||
+ !strcmp(arg, "--walk-reflogs")) {
+ init_reflog_walk(&revs->reflog_info);
+ }
+ else if (!strcmp(arg, "--default")) {
+ if (argc <= 1)
+ return error("bad --default argument");
+ revs->def = argv[1];
+ return 2;
+ }
+ else if (!strcmp(arg, "--merge")) {
+ revs->show_merge = 1;
+ }
+ else if (!strcmp(arg, "--topo-order")) {
+ revs->lifo = 1;
+ revs->topo_order = 1;
+ }
+ else if (!strcmp(arg, "--date-order")) {
+ revs->lifo = 0;
+ revs->topo_order = 1;
+ }
+ else if (!prefixcmp(arg, "--early-output")) {
+ int count = 100;
+ switch (arg[14]) {
+ case '=':
+ count = atoi(arg+15);
+ /* Fallthrough */
+ case 0:
+ revs->topo_order = 1;
+ revs->early_output = count;
+ }
+ }
+ else if (!strcmp(arg, "--parents")) {
+ revs->rewrite_parents = 1;
+ revs->print_parents = 1;
+ }
+ else if (!strcmp(arg, "--dense")) {
+ revs->dense = 1;
+ }
+ else if (!strcmp(arg, "--sparse")) {
+ revs->dense = 0;
+ }
+ else if (!strcmp(arg, "--show-all")) {
+ revs->show_all = 1;
+ }
+ else if (!strcmp(arg, "--remove-empty")) {
+ revs->remove_empty_trees = 1;
+ }
+ else if (!strcmp(arg, "--no-merges")) {
+ revs->no_merges = 1;
+ }
+ else if (!strcmp(arg, "--boundary")) {
+ revs->boundary = 1;
+ }
+ else if (!strcmp(arg, "--left-right")) {
+ revs->left_right = 1;
+ }
+ else if (!strcmp(arg, "--cherry-pick")) {
+ revs->cherry_pick = 1;
+ revs->limited = 1;
+ }
+ else if (!strcmp(arg, "--objects")) {
+ revs->tag_objects = 1;
+ revs->tree_objects = 1;
+ revs->blob_objects = 1;
+ }
+ else if (!strcmp(arg, "--objects-edge")) {
+ revs->tag_objects = 1;
+ revs->tree_objects = 1;
+ revs->blob_objects = 1;
+ revs->edge_hint = 1;
+ }
+ else if (!strcmp(arg, "--unpacked")) {
+ revs->unpacked = 1;
+ free(revs->ignore_packed);
+ revs->ignore_packed = NULL;
+ revs->num_ignore_packed = 0;
+ }
+ else if (!prefixcmp(arg, "--unpacked=")) {
+ revs->unpacked = 1;
+ add_ignore_packed(revs, arg+11);
+ }
+ else if (!strcmp(arg, "-r")) {
+ revs->diff = 1;
+ DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
+ }
+ else if (!strcmp(arg, "-t")) {
+ revs->diff = 1;
+ DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
+ DIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE);
+ }
+ else if (!strcmp(arg, "-m")) {
+ revs->ignore_merges = 0;
+ }
+ else if (!strcmp(arg, "-c")) {
+ revs->diff = 1;
+ revs->dense_combined_merges = 0;
+ revs->combine_merges = 1;
+ }
+ else if (!strcmp(arg, "--cc")) {
+ revs->diff = 1;
+ revs->dense_combined_merges = 1;
+ revs->combine_merges = 1;
+ }
+ else if (!strcmp(arg, "-v")) {
+ revs->verbose_header = 1;
+ }
+ else if (!strcmp(arg, "--pretty")) {
+ revs->verbose_header = 1;
+ get_commit_format(arg+8, revs);
+ }
+ else if (!prefixcmp(arg, "--pretty=")) {
+ revs->verbose_header = 1;
+ get_commit_format(arg+9, revs);
+ }
+ else if (!strcmp(arg, "--graph")) {
+ revs->topo_order = 1;
+ revs->rewrite_parents = 1;
+ revs->graph = graph_init(revs);
+ }
+ else if (!strcmp(arg, "--root")) {
+ revs->show_root_diff = 1;
+ }
+ else if (!strcmp(arg, "--no-commit-id")) {
+ revs->no_commit_id = 1;
+ }
+ else if (!strcmp(arg, "--always")) {
+ revs->always_show_header = 1;
+ }
+ else if (!strcmp(arg, "--no-abbrev")) {
+ revs->abbrev = 0;
+ }
+ else if (!strcmp(arg, "--abbrev")) {
+ revs->abbrev = DEFAULT_ABBREV;
+ }
+ else if (!prefixcmp(arg, "--abbrev=")) {
+ revs->abbrev = strtoul(arg + 9, NULL, 10);
+ if (revs->abbrev < MINIMUM_ABBREV)
+ revs->abbrev = MINIMUM_ABBREV;
+ else if (revs->abbrev > 40)
+ revs->abbrev = 40;
+ }
+ else if (!strcmp(arg, "--abbrev-commit")) {
+ revs->abbrev_commit = 1;
+ }
+ else if (!strcmp(arg, "--full-diff")) {
+ revs->diff = 1;
+ revs->full_diff = 1;
+ }
+ else if (!strcmp(arg, "--full-history")) {
+ revs->simplify_history = 0;
+ }
+ else if (!strcmp(arg, "--relative-date")) {
+ revs->date_mode = DATE_RELATIVE;
+ }
+ else if (!strncmp(arg, "--date=", 7)) {
+ revs->date_mode = parse_date_format(arg + 7);
+ }
+ else if (!strcmp(arg, "--log-size")) {
+ revs->show_log_size = 1;
+ }
+ /*
+ * Grepping the commit log
+ */
+ else if (!prefixcmp(arg, "--author=")) {
+ add_header_grep(revs, "author", arg+9);
+ }
+ else if (!prefixcmp(arg, "--committer=")) {
+ add_header_grep(revs, "committer", arg+12);
+ }
+ else if (!prefixcmp(arg, "--grep=")) {
+ add_message_grep(revs, arg+7);
+ }
+ else if (!strcmp(arg, "--extended-regexp") ||
+ !strcmp(arg, "-E")) {
+ if (revs->grep_filter)
+ revs->grep_filter->regflags |= REG_EXTENDED;
+ }
+ else if (!strcmp(arg, "--regexp-ignore-case") ||
+ !strcmp(arg, "-i")) {
+ if (revs->grep_filter)
+ revs->grep_filter->regflags |= REG_ICASE;
+ }
+ else if (!strcmp(arg, "--fixed-strings") ||
+ !strcmp(arg, "-F")) {
+ if (revs->grep_filter)
+ revs->grep_filter->fixed = 1;
+ }
+ else if (!strcmp(arg, "--all-match")) {
+ if (revs->grep_filter)
+ revs->grep_filter->all_match = 1;
+ }
+ else if (!prefixcmp(arg, "--encoding=")) {
+ arg += 11;
+ if (strcmp(arg, "none"))
+ git_log_output_encoding = xstrdup(arg);
+ else
+ git_log_output_encoding = "";
+ }
+ else if (!strcmp(arg, "--reverse")) {
+ revs->reverse ^= 1;
+ }
+ else if (!strcmp(arg, "--children")) {
+ revs->children.name = "children";
+ revs->limited = 1;
+ } else {
+ int opts = diff_opt_parse(&revs->diffopt, argv, argc);
+ if (!opts)
+ unkv[(*unkc)++] = arg;
+ return opts;
+ }
+
+ return 1;
+}
+
/*
* Parse revision information, filling in the "rev_info" structure,
* and removing the used arguments from the argument list.
@@ -983,12 +1253,7 @@ static void add_ignore_packed(struct rev_info *revs, const char *name)
*/
int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def)
{
- int i, flags, seen_dashdash, show_merge;
- const char **unrecognized = argv + 1;
- int left = 1;
- int all_match = 0;
- int regflags = 0;
- int fixed = 0;
+ int i, flags, left, seen_dashdash;
/* First, search for "--" */
seen_dashdash = 0;
@@ -1004,58 +1269,13 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
break;
}
- flags = show_merge = 0;
- for (i = 1; i < argc; i++) {
+ /* Second, deal with arguments and options */
+ flags = 0;
+ for (left = i = 1; i < argc; i++) {
const char *arg = argv[i];
if (*arg == '-') {
int opts;
- if (!prefixcmp(arg, "--max-count=")) {
- revs->max_count = atoi(arg + 12);
- continue;
- }
- if (!prefixcmp(arg, "--skip=")) {
- revs->skip_count = atoi(arg + 7);
- continue;
- }
- /* accept -<digit>, like traditional "head" */
- if ((*arg == '-') && isdigit(arg[1])) {
- revs->max_count = atoi(arg + 1);
- continue;
- }
- if (!strcmp(arg, "-n")) {
- if (argc <= i + 1)
- die("-n requires an argument");
- revs->max_count = atoi(argv[++i]);
- continue;
- }
- if (!prefixcmp(arg, "-n")) {
- revs->max_count = atoi(arg + 2);
- continue;
- }
- if (!prefixcmp(arg, "--max-age=")) {
- revs->max_age = atoi(arg + 10);
- continue;
- }
- if (!prefixcmp(arg, "--since=")) {
- revs->max_age = approxidate(arg + 8);
- continue;
- }
- if (!prefixcmp(arg, "--after=")) {
- revs->max_age = approxidate(arg + 8);
- continue;
- }
- if (!prefixcmp(arg, "--min-age=")) {
- revs->min_age = atoi(arg + 10);
- continue;
- }
- if (!prefixcmp(arg, "--before=")) {
- revs->min_age = approxidate(arg + 9);
- continue;
- }
- if (!prefixcmp(arg, "--until=")) {
- revs->min_age = approxidate(arg + 8);
- continue;
- }
+
if (!strcmp(arg, "--all")) {
handle_refs(revs, flags, for_each_ref);
continue;
@@ -1072,265 +1292,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
handle_refs(revs, flags, for_each_remote_ref);
continue;
}
- if (!strcmp(arg, "--first-parent")) {
- revs->first_parent_only = 1;
- continue;
- }
if (!strcmp(arg, "--reflog")) {
handle_reflog(revs, flags);
continue;
}
- if (!strcmp(arg, "-g") ||
- !strcmp(arg, "--walk-reflogs")) {
- init_reflog_walk(&revs->reflog_info);
- continue;
- }
if (!strcmp(arg, "--not")) {
flags ^= UNINTERESTING;
continue;
}
- if (!strcmp(arg, "--default")) {
- if (++i >= argc)
- die("bad --default argument");
- def = argv[i];
- continue;
- }
- if (!strcmp(arg, "--merge")) {
- show_merge = 1;
- continue;
- }
- if (!strcmp(arg, "--topo-order")) {
- revs->lifo = 1;
- revs->topo_order = 1;
- continue;
- }
- if (!strcmp(arg, "--date-order")) {
- revs->lifo = 0;
- revs->topo_order = 1;
- continue;
- }
- if (!prefixcmp(arg, "--early-output")) {
- int count = 100;
- switch (arg[14]) {
- case '=':
- count = atoi(arg+15);
- /* Fallthrough */
- case 0:
- revs->topo_order = 1;
- revs->early_output = count;
- continue;
- }
- }
- if (!strcmp(arg, "--parents")) {
- revs->rewrite_parents = 1;
- revs->print_parents = 1;
- continue;
- }
- if (!strcmp(arg, "--dense")) {
- revs->dense = 1;
- continue;
- }
- if (!strcmp(arg, "--sparse")) {
- revs->dense = 0;
- continue;
- }
- if (!strcmp(arg, "--show-all")) {
- revs->show_all = 1;
- continue;
- }
- if (!strcmp(arg, "--remove-empty")) {
- revs->remove_empty_trees = 1;
- continue;
- }
- if (!strcmp(arg, "--no-merges")) {
- revs->no_merges = 1;
- continue;
- }
- if (!strcmp(arg, "--boundary")) {
- revs->boundary = 1;
- continue;
- }
- if (!strcmp(arg, "--left-right")) {
- revs->left_right = 1;
- continue;
- }
- if (!strcmp(arg, "--cherry-pick")) {
- revs->cherry_pick = 1;
- revs->limited = 1;
- continue;
- }
- if (!strcmp(arg, "--objects")) {
- revs->tag_objects = 1;
- revs->tree_objects = 1;
- revs->blob_objects = 1;
- continue;
- }
- if (!strcmp(arg, "--objects-edge")) {
- revs->tag_objects = 1;
- revs->tree_objects = 1;
- revs->blob_objects = 1;
- revs->edge_hint = 1;
- continue;
- }
- if (!strcmp(arg, "--unpacked")) {
- revs->unpacked = 1;
- free(revs->ignore_packed);
- revs->ignore_packed = NULL;
- revs->num_ignore_packed = 0;
- continue;
- }
- if (!prefixcmp(arg, "--unpacked=")) {
- revs->unpacked = 1;
- add_ignore_packed(revs, arg+11);
- continue;
- }
- if (!strcmp(arg, "-r")) {
- revs->diff = 1;
- DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
- continue;
- }
- if (!strcmp(arg, "-t")) {
- revs->diff = 1;
- DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
- DIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE);
- continue;
- }
- if (!strcmp(arg, "-m")) {
- revs->ignore_merges = 0;
- continue;
- }
- if (!strcmp(arg, "-c")) {
- revs->diff = 1;
- revs->dense_combined_merges = 0;
- revs->combine_merges = 1;
- continue;
- }
- if (!strcmp(arg, "--cc")) {
- revs->diff = 1;
- revs->dense_combined_merges = 1;
- revs->combine_merges = 1;
- continue;
- }
- if (!strcmp(arg, "-v")) {
- revs->verbose_header = 1;
- continue;
- }
- if (!strcmp(arg, "--pretty")) {
- revs->verbose_header = 1;
- get_commit_format(arg+8, revs);
- continue;
- }
- if (!prefixcmp(arg, "--pretty=")) {
- revs->verbose_header = 1;
- get_commit_format(arg+9, revs);
- continue;
- }
- if (!strcmp(arg, "--graph")) {
- revs->topo_order = 1;
- revs->rewrite_parents = 1;
- revs->graph = graph_init(revs);
- continue;
- }
- if (!strcmp(arg, "--root")) {
- revs->show_root_diff = 1;
- continue;
- }
- if (!strcmp(arg, "--no-commit-id")) {
- revs->no_commit_id = 1;
- continue;
- }
- if (!strcmp(arg, "--always")) {
- revs->always_show_header = 1;
- continue;
- }
- if (!strcmp(arg, "--no-abbrev")) {
- revs->abbrev = 0;
- continue;
- }
- if (!strcmp(arg, "--abbrev")) {
- revs->abbrev = DEFAULT_ABBREV;
- continue;
- }
- if (!prefixcmp(arg, "--abbrev=")) {
- revs->abbrev = strtoul(arg + 9, NULL, 10);
- if (revs->abbrev < MINIMUM_ABBREV)
- revs->abbrev = MINIMUM_ABBREV;
- else if (revs->abbrev > 40)
- revs->abbrev = 40;
- continue;
- }
- if (!strcmp(arg, "--abbrev-commit")) {
- revs->abbrev_commit = 1;
- continue;
- }
- if (!strcmp(arg, "--full-diff")) {
- revs->diff = 1;
- revs->full_diff = 1;
- continue;
- }
- if (!strcmp(arg, "--full-history")) {
- revs->simplify_history = 0;
- continue;
- }
- if (!strcmp(arg, "--relative-date")) {
- revs->date_mode = DATE_RELATIVE;
- continue;
- }
- if (!strncmp(arg, "--date=", 7)) {
- revs->date_mode = parse_date_format(arg + 7);
- continue;
- }
- if (!strcmp(arg, "--log-size")) {
- revs->show_log_size = 1;
- continue;
- }
-
- /*
- * Grepping the commit log
- */
- if (!prefixcmp(arg, "--author=")) {
- add_header_grep(revs, "author", arg+9);
- continue;
- }
- if (!prefixcmp(arg, "--committer=")) {
- add_header_grep(revs, "committer", arg+12);
- continue;
- }
- if (!prefixcmp(arg, "--grep=")) {
- add_message_grep(revs, arg+7);
- continue;
- }
- if (!strcmp(arg, "--extended-regexp") ||
- !strcmp(arg, "-E")) {
- regflags |= REG_EXTENDED;
- continue;
- }
- if (!strcmp(arg, "--regexp-ignore-case") ||
- !strcmp(arg, "-i")) {
- regflags |= REG_ICASE;
- continue;
- }
- if (!strcmp(arg, "--fixed-strings") ||
- !strcmp(arg, "-F")) {
- fixed = 1;
- continue;
- }
- if (!strcmp(arg, "--all-match")) {
- all_match = 1;
- continue;
- }
- if (!prefixcmp(arg, "--encoding=")) {
- arg += 11;
- if (strcmp(arg, "none"))
- git_log_output_encoding = xstrdup(arg);
- else
- git_log_output_encoding = "";
- continue;
- }
- if (!strcmp(arg, "--reverse")) {
- revs->reverse ^= 1;
- continue;
- }
if (!strcmp(arg, "--no-walk")) {
revs->no_walk = 1;
continue;
@@ -1339,19 +1308,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
revs->no_walk = 0;
continue;
}
- if (!strcmp(arg, "--children")) {
- revs->children.name = "children";
- revs->limited = 1;
- continue;
- }
- opts = diff_opt_parse(&revs->diffopt, argv+i, argc-i);
+ opts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);
if (opts > 0) {
i += opts - 1;
continue;
}
- *unrecognized++ = arg;
- left++;
+ if (opts < 0)
+ exit(128);
continue;
}
@@ -1375,21 +1339,18 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
}
}
- if (revs->grep_filter) {
- revs->grep_filter->regflags |= regflags;
- revs->grep_filter->fixed = fixed;
- }
-
- if (show_merge)
+ if (revs->def == NULL)
+ revs->def = def;
+ if (revs->show_merge)
prepare_show_merge(revs);
- if (def && !revs->pending.nr) {
+ if (revs->def && !revs->pending.nr) {
unsigned char sha1[20];
struct object *object;
unsigned mode;
- if (get_sha1_with_mode(def, sha1, &mode))
- die("bad default revision '%s'", def);
- object = get_reference(revs, def, sha1, 0);
- add_pending_object_with_mode(revs, object, def, mode);
+ if (get_sha1_with_mode(revs->def, sha1, &mode))
+ die("bad default revision '%s'", revs->def);
+ object = get_reference(revs, revs->def, sha1, 0);
+ add_pending_object_with_mode(revs, object, revs->def, mode);
}
/* Did the user ask for any diff output? Run the diff! */
@@ -1423,7 +1384,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
die("diff_setup_done failed");
if (revs->grep_filter) {
- revs->grep_filter->all_match = all_match;
compile_grep_patterns(revs->grep_filter);
}
diff --git a/revision.h b/revision.h
index 5b8c56b..cc80fcd 100644
--- a/revision.h
+++ b/revision.h
@@ -26,6 +26,7 @@ struct rev_info {
/* Basic information */
const char *prefix;
+ const char *def;
void *prune_data;
unsigned int early_output;
@@ -66,6 +67,7 @@ struct rev_info {
/* Format info */
unsigned int shown_one:1,
+ show_merge:1,
abbrev_commit:1,
use_terminator:1,
missing_newline:1;
@@ -120,6 +122,8 @@ volatile show_early_output_fn_t show_early_output;
extern void init_revisions(struct rev_info *revs, const char *prefix);
extern int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def);
+extern int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
+ int *unkc, const char **unkv);
extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);
extern int prepare_revision_walk(struct rev_info *revs);
--
1.5.6.2.396.gca539
^ permalink raw reply related
* [PATCH 3/3] git-blame: migrate to incremental parse-option [2/2]
From: Pierre Habouzit @ 2008-07-08 13:19 UTC (permalink / raw)
To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin, Pierre Habouzit
In-Reply-To: <1215523175-28436-3-git-send-email-madcoder@debian.org>
Now use handle_revision_args instead of parse_revisions, and simplify the
handling of old-style arguments a lot thanks to the filtering.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
builtin-blame.c | 130 ++++++++++++++++---------------------------------------
1 files changed, 38 insertions(+), 92 deletions(-)
diff --git a/builtin-blame.c b/builtin-blame.c
index 5e82cd3..99f5140 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -2262,8 +2262,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
struct scoreboard sb;
struct origin *o;
struct blame_entry *ent;
- int i, seen_dashdash, unk;
- long bottom, top, lno;
+ long dashdash_pos, bottom, top, lno;
const char *final_commit_name = NULL;
enum object_type type;
@@ -2301,6 +2300,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
git_config(git_blame_config, NULL);
init_revisions(&revs, NULL);
save_commit_buffer = 0;
+ dashdash_pos = 0;
parse_options_start(&ctx, argc, argv, PARSE_OPT_KEEP_DASHDASH |
PARSE_OPT_KEEP_ARGV0);
@@ -2311,6 +2311,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
case PARSE_OPT_HELP:
exit(129);
case PARSE_OPT_DONE:
+ if (ctx.argv[0])
+ dashdash_pos = ctx.cpidx;
goto parse_done;
}
@@ -2330,20 +2332,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
parse_done:
argc = parse_options_end(&ctx);
- seen_dashdash = 0;
- for (unk = i = 1; i < argc; i++) {
- const char *arg = argv[i];
- if (*arg != '-')
- break;
- else if (!strcmp("--", arg)) {
- seen_dashdash = 1;
- i++;
- break;
- }
- else
- argv[unk++] = arg;
- }
-
if (!blame_move_score)
blame_move_score = BLAME_DEFAULT_MOVE_SCORE;
if (!blame_copy_score)
@@ -2356,92 +2344,50 @@ parse_done:
*
* The remaining are:
*
- * (1) if seen_dashdash, its either
- * "-options -- <path>" or
- * "-options -- <path> <rev>".
- * but the latter is allowed only if there is no
- * options that we passed to revision machinery.
+ * (1) if dashdash_pos != 0, its either
+ * "blame [revisions] -- <path>" or
+ * "blame -- <path> <rev>"
*
- * (2) otherwise, we may have "--" somewhere later and
- * might be looking at the first one of multiple 'rev'
- * parameters (e.g. " master ^next ^maint -- path").
- * See if there is a dashdash first, and give the
- * arguments before that to revision machinery.
- * After that there must be one 'path'.
+ * (2) otherwise, its one of the two:
+ * "blame [revisions] <path>"
+ * "blame <path> <rev>"
*
- * (3) otherwise, its one of the three:
- * "-options <path> <rev>"
- * "-options <rev> <path>"
- * "-options <path>"
- * but again the first one is allowed only if
- * there is no options that we passed to revision
- * machinery.
+ * Note that we must strip out <path> from the arguments: we do not
+ * want the path pruning but we may want "bottom" processing.
*/
-
- if (seen_dashdash) {
- /* (1) */
- if (argc <= i)
- usage_with_options(blame_opt_usage, options);
- path = add_prefix(prefix, argv[i]);
- if (i + 1 == argc - 1) {
- if (unk != 1)
+ if (dashdash_pos) {
+ switch (argc - dashdash_pos - 1) {
+ case 2: /* (1b) */
+ if (argc != 4)
usage_with_options(blame_opt_usage, options);
- argv[unk++] = argv[i + 1];
+ /* reorder for the new way: <rev> -- <path> */
+ argv[1] = argv[3];
+ argv[3] = argv[2];
+ argv[2] = "--";
+ /* FALLTHROUGH */
+ case 1: /* (1a) */
+ path = add_prefix(prefix, argv[--argc]);
+ argv[argc] = NULL;
+ break;
+ default:
+ usage_with_options(blame_opt_usage, options);
}
- else if (i + 1 != argc)
- /* garbage at end */
+ } else {
+ if (argc < 2)
usage_with_options(blame_opt_usage, options);
- }
- else {
- int j;
- for (j = i; !seen_dashdash && j < argc; j++)
- if (!strcmp(argv[j], "--"))
- seen_dashdash = j;
- if (seen_dashdash) {
- /* (2) */
- if (seen_dashdash + 1 != argc - 1)
- usage_with_options(blame_opt_usage, options);
- path = add_prefix(prefix, argv[seen_dashdash + 1]);
- for (j = i; j < seen_dashdash; j++)
- argv[unk++] = argv[j];
+ path = add_prefix(prefix, argv[argc - 1]);
+ if (argc == 3 && !has_path_in_work_tree(path)) { /* (2b) */
+ path = add_prefix(prefix, argv[1]);
+ argv[1] = argv[2];
}
- else {
- /* (3) */
- if (argc <= i)
- usage_with_options(blame_opt_usage, options);
- path = add_prefix(prefix, argv[i]);
- if (i + 1 == argc - 1) {
- final_commit_name = argv[i + 1];
-
- /* if (unk == 1) we could be getting
- * old-style
- */
- if (unk == 1 && !has_path_in_work_tree(path)) {
- path = add_prefix(prefix, argv[i + 1]);
- final_commit_name = argv[i];
- }
- }
- else if (i != argc - 1)
- usage_with_options(blame_opt_usage, options);
+ argv[argc - 1] = "--";
- setup_work_tree();
- if (!has_path_in_work_tree(path))
- die("cannot stat path %s: %s",
- path, strerror(errno));
- }
+ setup_work_tree();
+ if (!has_path_in_work_tree(path))
+ die("cannot stat path %s: %s", path, strerror(errno));
}
- if (final_commit_name)
- argv[unk++] = final_commit_name;
-
- /*
- * Now we got rev and path. We do not want the path pruning
- * but we may want "bottom" processing.
- */
- argv[unk++] = "--"; /* terminate the rev name */
- argv[unk] = NULL;
-
- setup_revisions(unk, argv, &revs, NULL);
+ setup_revisions(argc, argv, &revs, NULL);
memset(&sb, 0, sizeof(sb));
sb.revs = &revs;
--
1.5.6.2.396.gca539
^ permalink raw reply related
* Re: [PATCH] better git-submodule status output
From: Sylvain Joyeux @ 2008-07-08 13:12 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Junio C Hamano, Avery Pennarun, Lars Hjemli, Ping Yin,
Mark Levedahl, git
In-Reply-To: <alpine.DEB.1.00.0807081358481.18205@racer>
> > ... and checkouts the commit registered in the superproject. Therefore
> > neither git-submodule status nor git-submodule summary can give an idea
> > of "what's new in submodules"
>
> Yeah, now I get what you are saying. So you want a "git submodule peek"
> or some such.
That is exactly the idea.
Sylvain
^ permalink raw reply
* Re: [PATCH] better git-submodule status output
From: Johannes Schindelin @ 2008-07-08 13:00 UTC (permalink / raw)
To: Sylvain Joyeux
Cc: Junio C Hamano, Avery Pennarun, Lars Hjemli, Ping Yin,
Mark Levedahl, git
In-Reply-To: <20080708122244.GE14794@joyeux>
Hi,
On Tue, 8 Jul 2008, Sylvain Joyeux wrote:
> [somebody wrote]
>
> > [somebody else wrote]
> >
> > > What I was missing is, given the current discussion, a git-submodule
> > > fetch alike to what Avery was proposing, which fetches all
> > > submodules in one command. Having to do the fetch/update manually
> > > for each submodule is really cumbersome -- I have around 30 modules
> > > in the superproject.
> >
> > Did I not suggest "git submodule update" earlier? AFAIR this updates
> > all submodules that have been initialized.
>
> ... and checkouts the commit registered in the superproject. Therefore
> neither git-submodule status nor git-submodule summary can give an idea
> of "what's new in submodules"
Yeah, now I get what you are saying. So you want a "git submodule peek"
or some such.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 1/6] revisions: refactor init_revisions and setup_revisions.
From: Pierre Habouzit @ 2008-07-08 12:48 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, torvalds, gitster, peff
In-Reply-To: <alpine.DEB.1.00.0807081339270.4319@eeepc-johanness>
[-- Attachment #1: Type: text/plain, Size: 1041 bytes --]
On mar, jui 08, 2008 at 11:43:15 +0000, Johannes Schindelin wrote:
> Hi,
>
> On Tue, 8 Jul 2008, Pierre Habouzit wrote:
>
> > On Tue, Jul 08, 2008 at 10:59:43AM +0000, Johannes Schindelin wrote:
> >
> > > On Tue, 8 Jul 2008, Pierre Habouzit wrote:
> > >
> > > > setup_revisions has been split in two: parse_revisions that does
> > > > (almost) only argument parsing, to be more like what parse-options
> > > > can do, and setup_revisions that does the rest.
> > >
> > > I do not see the sense of this change, except
> >
> > Well, it's required to remove "parse_revisions" at once if we one day
> > reach the goal of having only parse-opt based parsers.
>
> We can do that, then.
>
> We do not need to do that, now.
hmmm right, I may be able to cook a simpler series without that
indeed. Ill repost a new one with this idea.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: stgit error on status command
From: Karl Hasselström @ 2008-07-08 12:25 UTC (permalink / raw)
To: Jon Smirl; +Cc: Git Mailing List, Catalin Marinas
In-Reply-To: <9e4733910807080506k6d51bc98h96a7a7b2bd56d98b@mail.gmail.com>
On 2008-07-08 08:06:04 -0400, Jon Smirl wrote:
> On 7/8/08, Karl Hasselström <kha@treskal.com> wrote:
>
> > Thanks for the report.
> >
> > The problem is that we mis-parse the output of git diff-index when
> > rename detection is on (and it prints more than one filename on
> > one line). This happens if you give stg status the --diff-opts=-M
> > flag, but you didn't -- but it could also happen if you have the
> > stgit.diff-opts config variable set.
>
> I have -M in my config file. If I don't use people on lkml complain
> about renames generating piles of output in the patches. I'm able to
> work around the problem for the moment.
Oh, I'm certainly not suggesting that you shouldn't have -M in your
stgit.diff-opts -- just that dropping it for the moment is a
workaround to your problem.
Hmm. Another workaround is to pass an explicit --diff-opts="" to stg
status; that'll override your stgit.diff-opts, and avoid triggering
the bug.
But with any luck, I'll have this fixed within 4-6 hours or so.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [PATCH] better git-submodule status output
From: Sylvain Joyeux @ 2008-07-08 12:22 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Junio C Hamano, Avery Pennarun, Lars Hjemli, Ping Yin,
Mark Levedahl, git
In-Reply-To: <alpine.DEB.1.00.0807081320220.4319@eeepc-johanness>
> > What I was missing is, given the current discussion, a git-submodule
> > fetch alike to what Avery was proposing, which fetches all submodules in
> > one command. Having to do the fetch/update manually for each submodule
> > is really cumbersome -- I have around 30 modules in the superproject.
>
> Did I not suggest "git submodule update" earlier? AFAIR this updates all
> submodules that have been initialized.
... and checkouts the commit registered in the superproject. Therefore
neither git-submodule status nor git-submodule summary can give an idea
of "what's new in submodules"
Sylvain
^ permalink raw reply
* Re: stgit error on status command
From: Jon Smirl @ 2008-07-08 12:06 UTC (permalink / raw)
To: Karl Hasselström; +Cc: Git Mailing List, Catalin Marinas
In-Reply-To: <20080708070254.GA4129@diana.vm.bytemark.co.uk>
On 7/8/08, Karl Hasselström <kha@treskal.com> wrote:
> Thanks for the report.
>
> The problem is that we mis-parse the output of git diff-index when
> rename detection is on (and it prints more than one filename on one
> line). This happens if you give stg status the --diff-opts=-M flag,
> but you didn't -- but it could also happen if you have the
> stgit.diff-opts config variable set.
I have -M in my config file. If I don't use people on lkml complain
about renames generating piles of output in the patches. I'm able to
work around the problem for the moment.
>
> I'll try to get a patch out tonight. In the mean time, if you like you
> should be able to work around the problem by unsetting
> stgit.diff-opts.
>
>
> --
> Karl Hasselström, kha@treskal.com
> www.treskal.com/kalle
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [PATCH v2 2/4] Add git-sequencer prototype documentation
From: Stephan Beyer @ 2008-07-08 11:49 UTC (permalink / raw)
To: Jakub Narebski
Cc: git, Christian Couder, Daniel Barkalow, Johannes Schindelin,
Junio C Hamano
In-Reply-To: <200807081237.51456.jnareb@gmail.com>
Hi,
On Tue, Jul 08, 2008, Jakub Narebski wrote:
> 1. Splitting a patch
[...]
> Currently I don't see easy way to do this with git-sequencer.
I've needed this, too, and I've added this use case in the EXAMPLES
section that will be included in the next patchset.
Basically it is the same as the rebase documentation says:
during a pause (by "edit" or "pause" insn) reset to HEAD^, add only
those parts to the index you want to have in the first patch and commit
and then repeat this.
When I've experienced this need the first time, I thought about how
sequencer could ease that.
A first idea was something like "take only files X, Y, Z from commit ..."
or a --exclude as patch has.
But often splitting a patch is not done file-wise, but hunk-wise or even
line-wise.
So my second idea was to add an option to "pause" that just invokes
the "reset HEAD^". That makes it easy, I think.
How should this option be called?
> 2. Patch based rebase
>
> git-rebase by default, and for speed, uses git-format-patch / git-am
> pipeline (utilizing '--rebasing' option to git-am to avoid changing
> commit messages, even if they do not follow commit message conventions).
> If you want for plain "git rebase" to use git-sequencer, it should be
> easy to support this "engine"; therefore perhaps it would be good
> to add some equivalent of "git format-patch" to the TODO file format.
Imho the patch generation should be done by git-rebase.
> 3. Checking rebase
>
> Usually when you are interacting with upstream by sending patches by
> email, the last part before sending series of patches is git-rebase
> on top of current work. It would be nice if there were some way to
> have "git rebase" (via git-sequencer) to check that all commits
> (perhaps with some explicitly stated in TODO file exceptions) passes
> pre-commit hook (checking for whitespaces and conflict markers), and
> if possibly also either test suite, or relevant parts of test suite.
>
> So perhaps extending TODO format by "check <script>" or
> "check-all <script>"?
That seems to be useful indeed and reminds me of
"git bisect run <script>".
Perhaps it's also better to call it "run" so that it is a generic
way of running scripts from sequencer and if they fail, sequencer
will pause, if they pass, everything goes on.
What about this?
run [--dir=<path>] [--] <cmd> <args>...::
Run command `<cmd>` with arguments `<args>`.
Pause (conflict-like) if exit status is non-zero.
+
If `<path>` is set, sequencer will change directory to `<path>`
before running the command and change back after exit.
Regards,
Stephan
--
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
^ permalink raw reply
* Re: [PATCH 1/6] revisions: refactor init_revisions and setup_revisions.
From: Johannes Schindelin @ 2008-07-08 11:43 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git, torvalds, gitster, peff
In-Reply-To: <20080708110624.GF19202@artemis.madism.org>
Hi,
On Tue, 8 Jul 2008, Pierre Habouzit wrote:
> On Tue, Jul 08, 2008 at 10:59:43AM +0000, Johannes Schindelin wrote:
>
> > On Tue, 8 Jul 2008, Pierre Habouzit wrote:
> >
> > > setup_revisions has been split in two: parse_revisions that does
> > > (almost) only argument parsing, to be more like what parse-options
> > > can do, and setup_revisions that does the rest.
> >
> > I do not see the sense of this change, except
>
> Well, it's required to remove "parse_revisions" at once if we one day
> reach the goal of having only parse-opt based parsers.
We can do that, then.
We do not need to do that, now.
In the meantime, your patch series of already pretty large patches gets
even larger, which prevents me from being able to review them.
Sad,
Dscho
P.S.: Please do not pull things at me like "it is so convenient to be able
to drop "0, NULL, " for many sites". That is not even funny. This case
does not need more convenience. The other cases do.
^ permalink raw reply
* Re: [PATCH] completion: add branch options --contains --merged --no-merged
From: Johannes Schindelin @ 2008-07-08 11:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, Eric Raible, Git Mailing List, szeder
In-Reply-To: <7vprppvt7a.fsf@gitster.siamese.dyndns.org>
Hi,
On Mon, 7 Jul 2008, Junio C Hamano wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > Eric Raible <raible@gmail.com> wrote:
> >> Signed-off-by: Eric Raible <raible@gmail.com>
> >
> > Trivially-Acked-by: Shawn O. Pearce <spearce@spearce.org>
> >
> > ;-)
> >
> > More completion support that probably should go to maint, as the
> > functionality in git-branch is in 1.5.6 but we (again) forgot to
> > make sure the completion was up-to-date prior to release.
>
> I am actually getting more worried about completion code getting larger
> and larger without its performance impact not being looked at nor
> addressed adequately. In my regular working tree:
>
> $ echo Docu<TAB>
>
> completes "mentation/" instantly, but:
>
> $ git log -- Docu<TAB>
>
> takes about 1.5 to 2 seconds to complete the same.
I noticed that myself, but did not have time to look into it.
It shows two bugs, actually: completions do not care about "--", and
completing refs takes way too long.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git-submodule - register module url if adding in place
From: Johannes Schindelin @ 2008-07-08 11:34 UTC (permalink / raw)
To: Mark Levedahl; +Cc: git
In-Reply-To: <1215484630-3784-1-git-send-email-mlevedahl@gmail.com>
Hi,
On Mon, 7 Jul 2008, Mark Levedahl wrote:
> When adding a new submodule in place, meaning the user created the
> submodule as a git repo in the tree first, we don't go through git
> submodule init to register the module. Thus, the url is not stored in
> .git/config, and no git operation will ever do so. In this case, assume
> the url the user supplies to git add is the one that should be
> registered, and do so.
I agree with Sylvain here, namely that this is too dangerous. Imagine
this very valid scenario:
$ git clone <somewhere> abc
$ git submodule add abc
Bummer.
Yes, happened to me.
So I'd like this to be an error, not something that tries to be helpful,
when it clearly cannot be.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/6] revisions: split the pure option parsing out from parse_revisions.
From: Pierre Habouzit @ 2008-07-08 11:25 UTC (permalink / raw)
To: Johannes Sixt, git, torvalds, gitster, peff
In-Reply-To: <20080708110022.GE19202@artemis.madism.org>
[-- Attachment #1: Type: text/plain, Size: 6070 bytes --]
On Tue, Jul 08, 2008 at 11:00:22AM +0000, Pierre Habouzit wrote:
> On Tue, Jul 08, 2008 at 10:51:13AM +0000, Johannes Sixt wrote:
> > Pierre Habouzit schrieb:
> > > +int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
> > > + int *unkc, const char **unkv)
> > > +{
> > > + const char *arg = argv[0];
> > > +
> > > + /* pseudo revision arguments */
> > > + if (!strcmp(arg, "--all") || !strcmp(arg, "--branches") ||
> > > + !strcmp(arg, "--tags") || !strcmp(arg, "--remotes") ||
> > > + !strcmp(arg, "--reflog") || !strcmp(arg, "--not") ||
> > > + !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk"))
> > > + {
> > > + unkv[*unkc++] = arg;
> >
> > + unkv[(*unkc)++] = arg;
>
> Huh right. Good catch, I wonder why the testsuite failed to see that.
Okay, I know why, there is another bug that was hiding this one,
parse_revisions whas doing argv[left++] = arg itself too, which was
fixing this mistake. I've pushed it again, and here is the fixed patch
3/6 for reviewing.
-----------8<-----------
From d0a062617526128c4ec0245614eb9916b7996c38 Mon Sep 17 00:00:00 2001
From: Pierre Habouzit <madcoder@debian.org>
Date: Tue, 8 Jul 2008 11:00:02 +0200
Subject: [PATCH] revisions: split handle_revision_args from parse_revisions.
This new function is meant to only parse non option revision arguments in
a row. IOW it's meant to parse what remains from a parse-opt filtering of
the argument list, with the knowledge of the "--" position (0 means none).
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
revision.c | 97 +++++++++++++++++++++++++++++++++++------------------------
revision.h | 1 +
2 files changed, 58 insertions(+), 40 deletions(-)
diff --git a/revision.c b/revision.c
index 99b3cc9..6958725 100644
--- a/revision.c
+++ b/revision.c
@@ -1241,39 +1241,15 @@ int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
return 1;
}
-/*
- * Parse revision information, filling in the "rev_info" structure,
- * and removing the used arguments from the argument list.
- *
- * Returns the number of arguments left that weren't recognized
- * (which are also moved to the head of the argument list)
- */
-int parse_revisions(int argc, const char **argv, struct rev_info *revs)
+int handle_revision_args(struct rev_info *revs,
+ int argc, const char **argv, int dashdash_pos)
{
- int i, flags, seen_dashdash;
- const char **unrecognized = argv + 1;
- int left = 1;
+ int i, left, flags = 0;
- /* First, search for "--" */
- seen_dashdash = 0;
- for (i = 1; i < argc; i++) {
+ for (left = i = 1; i < argc; i++) {
const char *arg = argv[i];
- if (strcmp(arg, "--"))
- continue;
- argv[i] = NULL;
- argc = i;
- if (argv[i + 1])
- revs->prune_data = get_pathspec(revs->prefix, argv + i + 1);
- seen_dashdash = 1;
- break;
- }
- flags = 0;
- for (i = 1; i < argc; i++) {
- const char *arg = argv[i];
if (*arg == '-') {
- int opts;
-
if (!strcmp(arg, "--all")) {
handle_refs(revs, flags, for_each_ref);
continue;
@@ -1306,23 +1282,22 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)
revs->no_walk = 0;
continue;
}
-
- opts = handle_revision_opt(revs, argc - i, argv + i, NULL, NULL);
- if (opts > 0) {
- i += opts - 1;
- continue;
+ if (i == dashdash_pos) {
+ argv[i] = NULL;
+ argc = i;
+ if (argv[i + 1])
+ revs->prune_data = get_pathspec(revs->prefix, argv + i + 1);
+ break;
}
- if (opts < 0)
- exit(128);
- *unrecognized++ = arg;
- left++;
+
+ argv[left++] = arg;
continue;
}
- if (handle_revision_arg(arg, revs, flags, seen_dashdash)) {
+ if (handle_revision_arg(arg, revs, flags, dashdash_pos)) {
int j;
- if (seen_dashdash || *arg == '^')
- die("bad revision '%s'", arg);
+ if (dashdash_pos || *arg == '^')
+ return error("bad revision '%s'", arg);
/* If we didn't have a "--":
* (1) all filenames must exist;
@@ -1338,9 +1313,51 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)
break;
}
}
+
return left;
}
+/*
+ * Parse revision information, filling in the "rev_info" structure,
+ * and removing the used arguments from the argument list.
+ *
+ * Returns the number of arguments left that weren't recognized
+ * (which are also moved to the head of the argument list)
+ */
+int parse_revisions(int argc, const char **argv, struct rev_info *revs)
+{
+ int i, left, dashdash_pos = 0;
+
+ /* First, filter out revision options and look for "--" */
+ for (left = i = 1; i < argc; i++) {
+ const char *arg = argv[i];
+ int opts;
+
+ if (arg[0] != '-') {
+ argv[left++] = arg;
+ continue;
+ }
+
+ if (!strcmp(arg, "--")) {
+ dashdash_pos = left;
+ memcpy(argv + left, argv + i, sizeof(*argv) * (argc - i));
+ left += argc - i;
+ break;
+ }
+
+ opts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);
+ if (opts > 0) {
+ i += opts - 1;
+ continue;
+ }
+ if (opts < 0)
+ exit(128);
+ }
+ argv[left] = NULL;
+
+ return handle_revision_args(revs, left, argv, dashdash_pos);
+}
+
void setup_revisions(struct rev_info *revs, const char *def)
{
if (revs->def == NULL)
diff --git a/revision.h b/revision.h
index 46ab713..c0f5df0 100644
--- a/revision.h
+++ b/revision.h
@@ -125,6 +125,7 @@ extern int parse_revisions(int argc, const char **argv, struct rev_info *revs);
extern void setup_revisions(struct rev_info *revs, const char *def);
extern int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
int *unkc, const char **unkv);
+extern int handle_revision_args(struct rev_info *revs, int argc, const char **argv, int dashdash_pos);
extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);
extern int prepare_revision_walk(struct rev_info *revs);
--
1.5.6.2.399.g15e15
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply related
* Re: [PATCH] better git-submodule status output
From: Johannes Schindelin @ 2008-07-08 11:21 UTC (permalink / raw)
To: Sylvain Joyeux
Cc: Junio C Hamano, Avery Pennarun, Lars Hjemli, Ping Yin,
Mark Levedahl, git
In-Reply-To: <20080708080042.GB3628@joyeux>
Hi,
On Tue, 8 Jul 2008, Sylvain Joyeux wrote:
> What I was missing is, given the current discussion, a git-submodule
> fetch alike to what Avery was proposing, which fetches all submodules in
> one command. Having to do the fetch/update manually for each submodule
> is really cumbersome -- I have around 30 modules in the superproject.
Did I not suggest "git submodule update" earlier? AFAIR this updates all
submodules that have been initialized.
Hth,
Dscho
^ permalink raw reply
* Re: [PATCH 1/6] revisions: refactor init_revisions and setup_revisions.
From: Pierre Habouzit @ 2008-07-08 11:06 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, torvalds, gitster, peff
In-Reply-To: <alpine.DEB.1.00.0807081258010.4319@eeepc-johanness>
[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]
On Tue, Jul 08, 2008 at 10:59:43AM +0000, Johannes Schindelin wrote:
> Hi,
>
> On Tue, 8 Jul 2008, Pierre Habouzit wrote:
>
> > setup_revisions has been split in two: parse_revisions that does
> > (almost) only argument parsing, to be more like what parse-options can
> > do, and setup_revisions that does the rest.
>
> I do not see the sense of this change, except
Well, it's required to remove "parse_revisions" at once if we one day
reach the goal of having only parse-opt based parsers.
> - blowing up your patch unnecessarily,
> - making it more inconvenient for users, and
It makes it convenient for parse-opt based parser, and it makes it
convenient for people that don't have arguments to parse (and those are
many).
> - adding an opportunity for bugs to creep in.
>
> In other words, I would like to see the semantics of setup_revisions()
> untouched.
FWIW like said I already submitted that patch under different versions
in the past, and I seem to recall that it was welcomed as it splits the
rev_info final fixups from the argument parsing which are quite
different steps.
FWIW I really like the new semantics better. If you're worried about
the fact that people may be confused about the new semantics, fine,
let's rename it then.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH 2/6] revisions: split the pure option parsing out from parse_revisions.
From: Pierre Habouzit @ 2008-07-08 11:00 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, torvalds, gitster, peff
In-Reply-To: <487346A1.4040006@viscovery.net>
[-- Attachment #1: Type: text/plain, Size: 932 bytes --]
On Tue, Jul 08, 2008 at 10:51:13AM +0000, Johannes Sixt wrote:
> Pierre Habouzit schrieb:
> > +int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
> > + int *unkc, const char **unkv)
> > +{
> > + const char *arg = argv[0];
> > +
> > + /* pseudo revision arguments */
> > + if (!strcmp(arg, "--all") || !strcmp(arg, "--branches") ||
> > + !strcmp(arg, "--tags") || !strcmp(arg, "--remotes") ||
> > + !strcmp(arg, "--reflog") || !strcmp(arg, "--not") ||
> > + !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk"))
> > + {
> > + unkv[*unkc++] = arg;
>
> + unkv[(*unkc)++] = arg;
Huh right. Good catch, I wonder why the testsuite failed to see that.
Anyways I pushed a fix on my public repo.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH 1/6] revisions: refactor init_revisions and setup_revisions.
From: Johannes Schindelin @ 2008-07-08 10:59 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git, torvalds, gitster, peff
In-Reply-To: <1215510964-16664-2-git-send-email-madcoder@debian.org>
Hi,
On Tue, 8 Jul 2008, Pierre Habouzit wrote:
> setup_revisions has been split in two: parse_revisions that does
> (almost) only argument parsing, to be more like what parse-options can
> do, and setup_revisions that does the rest.
I do not see the sense of this change, except
- blowing up your patch unnecessarily,
- making it more inconvenient for users, and
- adding an opportunity for bugs to creep in.
In other words, I would like to see the semantics of setup_revisions()
untouched.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/6] revisions: split the pure option parsing out from parse_revisions.
From: Johannes Sixt @ 2008-07-08 10:51 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git, torvalds, gitster, peff
In-Reply-To: <1215510964-16664-3-git-send-email-madcoder@debian.org>
Pierre Habouzit schrieb:
> +int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
> + int *unkc, const char **unkv)
> +{
> + const char *arg = argv[0];
> +
> + /* pseudo revision arguments */
> + if (!strcmp(arg, "--all") || !strcmp(arg, "--branches") ||
> + !strcmp(arg, "--tags") || !strcmp(arg, "--remotes") ||
> + !strcmp(arg, "--reflog") || !strcmp(arg, "--not") ||
> + !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk"))
> + {
> + unkv[*unkc++] = arg;
+ unkv[(*unkc)++] = arg;
Hm?
-- Hannes
^ permalink raw reply
* Re: Re* [PATCH] builtin-rerere: fix conflict markers parsing
From: Olivier Marin @ 2008-07-08 10:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vwsjwvmlk.fsf_-_@gitster.siamese.dyndns.org>
Junio C Hamano a écrit :
> Junio C Hamano <gitster@pobox.com> writes:
>
> I am wondering if a patch like this on top of your patch may make things
> even safer. The idea is the same as the earlier a1b32fd (git-rerere:
> detect unparsable conflicts, 2008-06-22) to fail rerere unless the markers
> are unambiguous.
>
> Thanks to your isspace(buf[7]), it is slightly less likely that this
> safety triggers on false positives.
>
> Thoughts?
My main concern was the error message that most users will not understand
after a "git rebase --continue", for example. So, I tried to remove it and
let things work as before because rerere seems to work even with ambiguous
cases.
But I think your patch is the right thing to do: safe is better.
Olivier.
^ permalink raw reply
* Re: [PATCH v2 2/4] Add git-sequencer prototype documentation
From: Jakub Narebski @ 2008-07-08 10:37 UTC (permalink / raw)
To: Stephan Beyer
Cc: git, Christian Couder, Daniel Barkalow, Johannes Schindelin,
Junio C Hamano
In-Reply-To: <1215277204-20926-1-git-send-email-s-beyer@gmx.net>
On Sat, 5 July 2008, Stephan Beyer wrote:
> +git-sequencer(1)
> +TODO FILE FORMAT
> +edit <commit>::
> +mark <mark>::
> +merge [options] <commit-ish1> <commit-ish2> ... <commit-ishN>::
> +pick [options] <commit>::
> +patch [options] <file>::
> +pause::
> +ref <ref>::
> +reset <commit-ish>::
> +squash [options] <commit>::
> +squash [options] --from <mark>::
A few comments and ideas:
1. Splitting a patch
I cannot comment well on git-sequencer, as I have started using
StGIT patch management interface instead of git-rebase in times when
there were no "git rebase --interactive". Nevertheless working with
StGIT is a bit similar to working with interactive rebase...
I don't find myself wanting to join two patches into one (to squadh
a commit) perhaps because when I want to add something to a commit
(to a patch) I simply go to this patch, edit files, and refresh the
patch. From time to time however I find myself SPLITTING a patch,
for example extracting something added "by the way"/"while at it"
into separate commit (like late separate better documenting
project_index file format from adding optional description field
to project_index file format).
Currently I don't see easy way to do this with git-sequencer.
2. Patch based rebase
git-rebase by default, and for speed, uses git-format-patch / git-am
pipeline (utilizing '--rebasing' option to git-am to avoid changing
commit messages, even if they do not follow commit message conventions).
If you want for plain "git rebase" to use git-sequencer, it should be
easy to support this "engine"; therefore perhaps it would be good
to add some equivalent of "git format-patch" to the TODO file format.
3. Checking rebase
Usually when you are interacting with upstream by sending patches by
email, the last part before sending series of patches is git-rebase
on top of current work. It would be nice if there were some way to
have "git rebase" (via git-sequencer) to check that all commits
(perhaps with some explicitly stated in TODO file exceptions) passes
pre-commit hook (checking for whitespaces and conflict markers), and
if possibly also either test suite, or relevant parts of test suite.
So perhaps extending TODO format by "check <script>" or
"check-all <script>"?
I'm just not sure if it is worth adding to TOD file format...
--
Jakub Narebski
Poland
^ permalink raw reply
* [PATCH] parse-options: add PARSE_OPT_FAKELASTARG flag.
From: Pierre Habouzit @ 2008-07-08 10:34 UTC (permalink / raw)
To: Junio C Hamano, git, Lars Hjemli
In-Reply-To: <20080708101452.GB19202@artemis.madism.org>
[-- Attachment #1: Type: text/plain, Size: 4482 bytes --]
If you set this for a given flag, and the flag appears without a value on
the command line, then the `defval' is used to fake a new argument.
Note that this flag is meaningless in presence of OPTARG or NOARG flags.
(in the current implementation it will be ignored, but don't rely on it).
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
> (3) inspired from (1) and (2), have a flag for options that says
> "I do take an argument, but if I'm the last option on the
> command line, please fake this argument for me.
>
> I really like (3) more FWIW as it doesn't generate ambiguous
> parsers like (2) would, and it's not horrible like (1). And cherry
> on top it's pretty trivial to implement I think.
And here it is, untested though (in the sense that I didn't test the
new feature, but git is not broken with the patch).
parse-options.c | 55 +++++++++++++++++++++++++++----------------------------
parse-options.h | 1 +
2 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index a90b336..a63485c 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -5,17 +5,6 @@
#define OPT_SHORT 1
#define OPT_UNSET 2
-static inline const char *get_arg(struct parse_opt_ctx_t *p)
-{
- if (p->opt) {
- const char *res = p->opt;
- p->opt = NULL;
- return res;
- }
- p->argc--;
- return *++p->argv;
-}
-
static int opterror(const struct option *opt, const char *reason, int flags)
{
if (flags & OPT_SHORT)
@@ -25,8 +14,24 @@ static int opterror(const struct option *opt, const char *reason, int flags)
return error("option `%s' %s", opt->long_name, reason);
}
+static inline int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,
+ int flags, const char **arg)
+{
+ if (p->opt) {
+ *arg = p->opt;
+ p->opt = NULL;
+ } else if (p->argc) {
+ p->argc--;
+ *arg = *++p->argv;
+ } else if (opt->flags & PARSE_OPT_FAKELASTARG) {
+ *arg = (const char *)opt->defval;
+ } else
+ return opterror(opt, "requires a value", flags);
+ return 0;
+}
+
static int get_value(struct parse_opt_ctx_t *p,
- const struct option *opt, int flags)
+ const struct option *opt, int flags)
{
const char *s, *arg;
const int unset = flags & OPT_UNSET;
@@ -52,7 +57,6 @@ static int get_value(struct parse_opt_ctx_t *p,
}
}
- arg = p->opt ? p->opt : (p->argc > 1 ? p->argv[1] : NULL);
switch (opt->type) {
case OPTION_BIT:
if (unset)
@@ -74,17 +78,12 @@ static int get_value(struct parse_opt_ctx_t *p,
return 0;
case OPTION_STRING:
- if (unset) {
+ if (unset)
*(const char **)opt->value = NULL;
- return 0;
- }
- if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
+ else if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
*(const char **)opt->value = (const char *)opt->defval;
- return 0;
- }
- if (!arg)
- return opterror(opt, "requires a value", flags);
- *(const char **)opt->value = get_arg(p);
+ else
+ return get_arg(p, opt, flags, (const char **)opt->value);
return 0;
case OPTION_CALLBACK:
@@ -94,9 +93,9 @@ static int get_value(struct parse_opt_ctx_t *p,
return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
- if (!arg)
- return opterror(opt, "requires a value", flags);
- return (*opt->callback)(opt, get_arg(p), 0) ? (-1) : 0;
+ if (get_arg(p, opt, flags, &arg))
+ return -1;
+ return (*opt->callback)(opt, arg, 0) ? (-1) : 0;
case OPTION_INTEGER:
if (unset) {
@@ -107,9 +106,9 @@ static int get_value(struct parse_opt_ctx_t *p,
*(int *)opt->value = opt->defval;
return 0;
}
- if (!arg)
- return opterror(opt, "requires a value", flags);
- *(int *)opt->value = strtol(get_arg(p), (char **)&s, 10);
+ if (get_arg(p, opt, flags, &arg))
+ return -1;
+ *(int *)opt->value = strtol(arg, (char **)&s, 10);
if (*s)
return opterror(opt, "expects a numerical value", flags);
return 0;
diff --git a/parse-options.h b/parse-options.h
index c5f0b4b..6e9edd1 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -28,6 +28,7 @@ enum parse_opt_option_flags {
PARSE_OPT_NOARG = 2,
PARSE_OPT_NONEG = 4,
PARSE_OPT_HIDDEN = 8,
+ PARSE_OPT_FAKELASTARG = 16,
};
struct option;
--
1.5.6.2.398.g3c3f1.dirty
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply related
* Re: What should "git branch --merged master" do?
From: Pierre Habouzit @ 2008-07-08 10:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Lars Hjemli
In-Reply-To: <7v8wwcx446.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 3075 bytes --]
On Tue, Jul 08, 2008 at 06:49:13AM +0000, Junio C Hamano wrote:
> e8b404c (git-branch: add support for --merged and --no-merged, 2008-04-17)
> introduced "git branch --merged" to show the branches that are contained
> within the current HEAD. I.e. the ones that you can say "git branch -d"
> and do not have to say "-D" to delete.
>
> Currently "git branch --merged master" fails with a message:
>
> fatal: A branch named 'master' already exists.
>
> but --merged and its opposite --no-merged are in spirit very similar to
> another option --contains in that it is meant to be used as a way to
> influence which branches are _reported_, and not about creating a new
> branch.
>
> Perhaps we should change them to default to HEAD (to retain the current
> behaviour) but take a commit, and show branches that are merged to the
> commit or not yet fully merged to the commit, respectively?
>
> Incidentally, "git branch --with" fails without the mandatory commit
> argument, and if we are going to do the above, we probably should default
> the argument to HEAD as well.
>
> Here is an attempt to update --with but I am not happy with it.
>
> The patch makes
>
> $ git branch --contains
>
> work as expected, but breaks
>
> $ git branch --contains master
>
> You need to spell "git branch --contains=master" instead. Which means it
> is a regression and cannot be applied. I suspect I am not using
> parse_options() in an optimal way, but I did not find a way for my
> callback to tell "I consumed the next parameter and it was mine" or "The
> next one is not my optional parameter" back to the parse_options(), so
> probably parse_options() need to be fixed to update this without
> regression, I suspect.
I actually see three ways.
(1) quick and dirty: if we see at argv[argc - 1]
--contains/--merge/--no-merge/... we do:
argv[argc] = "HEAD" and use parse-options with a mandatory
argument for --merge/--contains/... Okay this leaves the issue of
the fact that we must NULL-terminate argv, but that's trivial
(even if quite despisable).
(2) big update of parse-options: we refactor callbacks so that they
can decide if they take their argument or not. This removes the
need for --long-opt=foo for options with optional arguments, but
the refactor will be heavy. That's more or less what you suggest
in your last § but I don't like it for many reasons so I wont
enter the details.
(3) inspired from (1) and (2), have a flag for options that says "I do
take an argument, but if I'm the last option on the command line,
please fake this argument for me.
I really like (3) more FWIW as it doesn't generate ambiguous parsers
like (2) would, and it's not horrible like (1). And cherry on top it's
pretty trivial to implement I think.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: Migration of builtin-blame to parse-option
From: Pierre Habouzit @ 2008-07-08 10:02 UTC (permalink / raw)
To: git; +Cc: torvalds, gitster, peff
In-Reply-To: <1215510964-16664-1-git-send-email-madcoder@debian.org>
[-- Attachment #1: Type: text/plain, Size: 522 bytes --]
While rebasing this branch on the latest next, I had a conflict for
the patch 1 that I fixed, and I also fixed spacing issues in patches 4
and 6. The fixed series is on my public git repo[0], but it's really
like this one, so I don't see the point in flooding again with it ;)
[0] git://git.madism.org/~madcoder/git.git#ph/parseopt
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ 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