From 9163399535c43b6764376a280c2551600e8cd5bc Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Mar 2019 04:13:36 -0400 Subject: [PATCH 01/13] revision: drop some unused "revs" parameters There are several internal helpers that take a rev_info struct but don't actually look at it. While one could argue that all helpers in revision.c should take a rev_info struct for consistency, dropping the unused parameter makes it clear that they don't actually depend on any other rev options. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- revision.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/revision.c b/revision.c index eb8e51bc63..695021d6fc 100644 --- a/revision.c +++ b/revision.c @@ -1894,7 +1894,7 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi return 0; } -static void read_pathspec_from_stdin(struct rev_info *revs, struct strbuf *sb, +static void read_pathspec_from_stdin(struct strbuf *sb, struct argv_array *prune) { while (strbuf_getline(sb, stdin) != EOF) @@ -1928,7 +1928,7 @@ static void read_revisions_from_stdin(struct rev_info *revs, die("bad revision '%s'", sb.buf); } if (seen_dashdash) - read_pathspec_from_stdin(revs, &sb, prune); + read_pathspec_from_stdin(&sb, prune); strbuf_release(&sb); warn_on_object_refname_ambiguity = save_warning; @@ -2748,7 +2748,7 @@ static struct merge_simplify_state *locate_simplify_state(struct rev_info *revs, return st; } -static int mark_redundant_parents(struct rev_info *revs, struct commit *commit) +static int mark_redundant_parents(struct commit *commit) { struct commit_list *h = reduce_heads(commit->parents); int i = 0, marked = 0; @@ -2784,7 +2784,7 @@ static int mark_redundant_parents(struct rev_info *revs, struct commit *commit) return marked; } -static int mark_treesame_root_parents(struct rev_info *revs, struct commit *commit) +static int mark_treesame_root_parents(struct commit *commit) { struct commit_list *p; int marked = 0; @@ -2976,8 +2976,8 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c * Detect and simplify both cases. */ if (1 < cnt) { - int marked = mark_redundant_parents(revs, commit); - marked += mark_treesame_root_parents(revs, commit); + int marked = mark_redundant_parents(commit); + marked += mark_treesame_root_parents(commit); if (marked) marked -= leave_one_treesame_to_parent(revs, commit); if (marked) From 7954d365c64fd5cb854cf477aab3c12b8ff43f42 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Mar 2019 04:13:42 -0400 Subject: [PATCH 02/13] log: drop unused rev_info from early output The early output code passes around a rev_info struct but doesn't need it. The setup step only turns on global signal handlers, and the "estimate" step is done completely from the rev->commits list that is passed in separately. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/log.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index ab859f5904..6595471ddf 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -251,7 +251,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix, * This gives a rough estimate for how many commits we * will print out in the list. */ -static int estimate_commit_count(struct rev_info *rev, struct commit_list *list) +static int estimate_commit_count(struct commit_list *list) { int n = 0; @@ -289,7 +289,7 @@ static void log_show_early(struct rev_info *revs, struct commit_list *list) switch (simplify_commit(revs, commit)) { case commit_show: if (show_header) { - int n = estimate_commit_count(revs, list); + int n = estimate_commit_count(list); show_early_header(revs, "incomplete", n); show_header = 0; } @@ -333,7 +333,7 @@ static void early_output(int signal) show_early_output = log_show_early; } -static void setup_early_output(struct rev_info *rev) +static void setup_early_output(void) { struct sigaction sa; @@ -364,7 +364,7 @@ static void setup_early_output(struct rev_info *rev) static void finish_early_output(struct rev_info *rev) { - int n = estimate_commit_count(rev, rev->commits); + int n = estimate_commit_count(rev->commits); signal(SIGALRM, SIG_IGN); show_early_header(rev, "done", n); } @@ -376,7 +376,7 @@ static int cmd_log_walk(struct rev_info *rev) int saved_dcctc = 0, close_file = rev->diffopt.close_file; if (rev->early_output) - setup_early_output(rev); + setup_early_output(); if (prepare_revision_walk(rev)) die(_("revision walk setup failed")); From af117077d38af01f585adb16ded9deccb559b0b6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Mar 2019 04:14:01 -0400 Subject: [PATCH 03/13] log: drop unused "len" from show_tagger() We pass the length of the found "tagger" line to show_tagger(), but it does not use it; instead, it passes the string to pp_user_info(), which reads until newline or NUL. This is OK for our purposes because we always read the object contents into a buffer with an extra NUL (and indeed, our sole caller already relies on this by using starts_with). Let's drop the ignored parameter. And while we're touching the caller, let's use skip_prefix() to avoid a magic number. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/log.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 6595471ddf..35314d12ec 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -490,7 +490,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix) return cmd_log_walk(&rev); } -static void show_tagger(char *buf, int len, struct rev_info *rev) +static void show_tagger(const char *buf, struct rev_info *rev) { struct strbuf out = STRBUF_INIT; struct pretty_print_context pp = {0}; @@ -546,11 +546,11 @@ static int show_tag_object(const struct object_id *oid, struct rev_info *rev) assert(type == OBJ_TAG); while (offset < size && buf[offset] != '\n') { int new_offset = offset + 1; + const char *ident; while (new_offset < size && buf[new_offset++] != '\n') ; /* do nothing */ - if (starts_with(buf + offset, "tagger ")) - show_tagger(buf + offset + 7, - new_offset - offset - 7, rev); + if (skip_prefix(buf + offset, "tagger ", &ident)) + show_tagger(ident, rev); offset = new_offset; } From 5bb1cb6d023fb1087694b8d7148bc6f3f306efc5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Mar 2019 04:14:07 -0400 Subject: [PATCH 04/13] update-index: drop unused prefix_length parameter from do_reupdate() The prefix is always a NUL-terminated string, and we just end up passing it along to parse_pathspec() anyway (which does not even take a length). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/update-index.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/builtin/update-index.c b/builtin/update-index.c index 1b6c42f748..ff5cfd1194 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -724,7 +724,7 @@ static int do_unresolve(int ac, const char **av, } static int do_reupdate(int ac, const char **av, - const char *prefix, int prefix_length) + const char *prefix) { /* Read HEAD and run update-index on paths that are * merged and already different between index and HEAD. @@ -940,8 +940,7 @@ static enum parse_opt_result reupdate_callback( /* consume remaining arguments. */ setup_work_tree(); - *has_errors = do_reupdate(ctx->argc, ctx->argv, - prefix, prefix ? strlen(prefix) : 0); + *has_errors = do_reupdate(ctx->argc, ctx->argv, prefix); if (*has_errors) active_cache_changed = 0; From 04c4c766ec8252674496ab232b63c1860bcbc0a9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Mar 2019 04:14:30 -0400 Subject: [PATCH 05/13] test-date: drop unused "now" parameter from parse_dates() We only need the current time for relative dates like "5 minutes ago", and those are parsed only through approxidate, not the strict parser used by parse_dates(). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/helper/test-date.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/helper/test-date.c b/t/helper/test-date.c index b3253803ac..585347ea48 100644 --- a/t/helper/test-date.c +++ b/t/helper/test-date.c @@ -55,7 +55,7 @@ static void show_dates(const char **argv, const char *format) } } -static void parse_dates(const char **argv, struct timeval *now) +static void parse_dates(const char **argv) { struct strbuf result = STRBUF_INIT; @@ -124,7 +124,7 @@ int cmd__date(int argc, const char **argv) else if (skip_prefix(*argv, "show:", &x)) show_dates(argv+1, x); else if (!strcmp(*argv, "parse")) - parse_dates(argv+1, &now); + parse_dates(argv+1); else if (!strcmp(*argv, "approxidate")) parse_approxidate(argv+1, &now); else if (!strcmp(*argv, "timestamp")) From 664296985c2f05df83da9b859c258d97e6a11ba3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Mar 2019 04:15:07 -0400 Subject: [PATCH 06/13] unpack-trees: drop name_entry from traverse_by_cache_tree() We pull the names from the existing index rather than the tree entry, which is after all the point of this function. Let's drop the unused "names" parameter. Note that we leave the "nr_names" parameter, as it tells us how many trees we are traversing (and thus how many index stages to set up). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- unpack-trees.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/unpack-trees.c b/unpack-trees.c index 22c41a3ba8..79551ab9cc 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -707,7 +707,6 @@ static int index_pos_by_traverse_info(struct name_entry *names, * instead of ODB since we already know what these trees contain. */ static int traverse_by_cache_tree(int pos, int nr_entries, int nr_names, - struct name_entry *names, struct traverse_info *info) { struct cache_entry *src[MAX_UNPACK_TREES + 1] = { NULL, }; @@ -797,7 +796,7 @@ static int traverse_trees_recursive(int n, unsigned long dirmask, * unprocessed entries before 'pos'. */ bottom = o->cache_bottom; - ret = traverse_by_cache_tree(pos, nr_entries, n, names, info); + ret = traverse_by_cache_tree(pos, nr_entries, n, info); o->cache_bottom = bottom; return ret; } From df351c6e676e8eea0defa00ea919ad7da6bd03f1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Mar 2019 04:15:27 -0400 Subject: [PATCH 07/13] unpack-trees: drop unused error_type parameters The verify_clean_subdirectory() helper takes an error_type parameter from the caller, but doesn't actually use it. Instead, when it calls add_rejected_path() it passes NOT_UPTODATE_DIR, its own custom error type which is more specific than what the caller provides. Likewise for verify_clean_submodule(), which always passes WOULD_LOSE_SUBMODULE. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- unpack-trees.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/unpack-trees.c b/unpack-trees.c index 79551ab9cc..5f65ff5804 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1758,7 +1758,6 @@ static void invalidate_ce_path(const struct cache_entry *ce, */ static int verify_clean_submodule(const char *old_sha1, const struct cache_entry *ce, - enum unpack_trees_error_types error_type, struct unpack_trees_options *o) { if (!submodule_from_ce(ce)) @@ -1769,7 +1768,6 @@ static int verify_clean_submodule(const char *old_sha1, } static int verify_clean_subdirectory(const struct cache_entry *ce, - enum unpack_trees_error_types error_type, struct unpack_trees_options *o) { /* @@ -1792,7 +1790,7 @@ static int verify_clean_subdirectory(const struct cache_entry *ce, if (!sub_head && oideq(&oid, &ce->oid)) return 0; return verify_clean_submodule(sub_head ? NULL : oid_to_hex(&oid), - ce, error_type, o); + ce, o); } /* @@ -1888,7 +1886,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype, * files that are in "foo/" we would lose * them. */ - if (verify_clean_subdirectory(ce, error_type, o) < 0) + if (verify_clean_subdirectory(ce, o) < 0) return -1; return 0; } From c5c33504c906f6bf09fe00a1a32c6c4bdb76ed0f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Mar 2019 04:15:48 -0400 Subject: [PATCH 08/13] report_path_error(): drop unused prefix parameter This hasn't been used since 17ddc66e70 (convert report_path_error to take struct pathspec, 2013-07-14), as the names in the struct will have already been prefixed when they were parsed. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/checkout.c | 2 +- builtin/commit.c | 6 +++--- builtin/ls-files.c | 2 +- builtin/submodule--helper.c | 2 +- dir.c | 3 +-- dir.h | 2 +- 6 files changed, 8 insertions(+), 9 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 0e6037b296..72f7110fd8 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -376,7 +376,7 @@ static int checkout_paths(const struct checkout_opts *opts, ps_matched, opts); - if (report_path_error(ps_matched, &opts->pathspec, opts->prefix)) { + if (report_path_error(ps_matched, &opts->pathspec)) { free(ps_matched); return 1; } diff --git a/builtin/commit.c b/builtin/commit.c index f17537474a..a138ff85b0 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -235,7 +235,7 @@ static int commit_index_files(void) * and return the paths that match the given pattern in list. */ static int list_paths(struct string_list *list, const char *with_tree, - const char *prefix, const struct pathspec *pattern) + const struct pathspec *pattern) { int i, ret; char *m; @@ -264,7 +264,7 @@ static int list_paths(struct string_list *list, const char *with_tree, item->util = item; /* better a valid pointer than a fake one */ } - ret = report_path_error(m, pattern, prefix); + ret = report_path_error(m, pattern); free(m); return ret; } @@ -454,7 +454,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix die(_("cannot do a partial commit during a cherry-pick.")); } - if (list_paths(&partial, !current_head ? NULL : "HEAD", prefix, &pathspec)) + if (list_paths(&partial, !current_head ? NULL : "HEAD", &pathspec)) exit(1); discard_cache(); diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 29a8762d46..37a08c714b 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -680,7 +680,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix) if (ps_matched) { int bad; - bad = report_path_error(ps_matched, &pathspec, prefix); + bad = report_path_error(ps_matched, &pathspec); if (bad) fprintf(stderr, "Did you forget to 'git add'?\n"); diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 6bcc4f1bd7..8342b78add 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -348,7 +348,7 @@ static int module_list_compute(int argc, const char **argv, i++; } - if (ps_matched && report_path_error(ps_matched, pathspec, prefix)) + if (ps_matched && report_path_error(ps_matched, pathspec)) result = -1; free(ps_matched); diff --git a/dir.c b/dir.c index b2cabadf25..5286f710cc 100644 --- a/dir.c +++ b/dir.c @@ -502,8 +502,7 @@ int submodule_path_match(const struct index_state *istate, } int report_path_error(const char *ps_matched, - const struct pathspec *pathspec, - const char *prefix) + const struct pathspec *pathspec) { /* * Make sure all pathspec matched; otherwise it is an error. diff --git a/dir.h b/dir.h index e3ec26143d..823bae628b 100644 --- a/dir.h +++ b/dir.h @@ -220,7 +220,7 @@ extern int match_pathspec(const struct index_state *istate, const struct pathspec *pathspec, const char *name, int namelen, int prefix, char *seen, int is_dir); -extern int report_path_error(const char *ps_matched, const struct pathspec *pathspec, const char *prefix); +extern int report_path_error(const char *ps_matched, const struct pathspec *pathspec); extern int within_depth(const char *name, int namelen, int depth, int max_depth); extern int fill_directory(struct dir_struct *dir, From 0f804b0bac39fb696ea74b8dd59549935ec9ca00 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Mar 2019 04:16:14 -0400 Subject: [PATCH 09/13] fetch_pack(): drop unused parameters We don't need the caller of fetch_pack() to pass in "dest", which is the remote URL. Since ba227857d2 (Reduce the number of connects when fetching, 2008-02-04), the caller is responsible for calling git_connect() itself, and our "dest" parameter is unused. That commit also started passing us the resulting "conn" child_process from git_connect(). But likewise, we do not need do anything with it. The descriptors in "fd" are enough for us, and the caller is responsible for cleaning up "conn". We can just drop both parameters. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/fetch-pack.c | 2 +- fetch-pack.c | 3 +-- fetch-pack.h | 3 +-- transport.c | 10 ++++------ 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 153a2bd282..dc1485c8aa 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -234,7 +234,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) BUG("unknown protocol version"); } - ref = fetch_pack(&args, fd, conn, ref, dest, sought, nr_sought, + ref = fetch_pack(&args, fd, ref, sought, nr_sought, &shallow, pack_lockfile_ptr, version); if (pack_lockfile) { printf("lock %s\n", pack_lockfile); diff --git a/fetch-pack.c b/fetch-pack.c index e69993b2eb..8d67d4e362 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1615,9 +1615,8 @@ static int iterate_ref_map(void *cb_data, struct object_id *oid) } struct ref *fetch_pack(struct fetch_pack_args *args, - int fd[], struct child_process *conn, + int fd[], const struct ref *ref, - const char *dest, struct ref **sought, int nr_sought, struct oid_array *shallow, char **pack_lockfile, diff --git a/fetch-pack.h b/fetch-pack.h index 43ec344d95..67f684229a 100644 --- a/fetch-pack.h +++ b/fetch-pack.h @@ -78,9 +78,8 @@ struct fetch_pack_args { * marked as such. */ struct ref *fetch_pack(struct fetch_pack_args *args, - int fd[], struct child_process *conn, + int fd[], const struct ref *ref, - const char *dest, struct ref **sought, int nr_sought, struct oid_array *shallow, diff --git a/transport.c b/transport.c index d0608df5c9..365ea574c7 100644 --- a/transport.c +++ b/transport.c @@ -314,7 +314,6 @@ static int fetch_refs_via_pack(struct transport *transport, int ret = 0; struct git_transport_data *data = transport->data; struct ref *refs = NULL; - char *dest = xstrdup(transport->url); struct fetch_pack_args args; struct ref *refs_tmp = NULL; @@ -356,16 +355,16 @@ static int fetch_refs_via_pack(struct transport *transport, switch (data->version) { case protocol_v2: - refs = fetch_pack(&args, data->fd, data->conn, + refs = fetch_pack(&args, data->fd, refs_tmp ? refs_tmp : transport->remote_refs, - dest, to_fetch, nr_heads, &data->shallow, + to_fetch, nr_heads, &data->shallow, &transport->pack_lockfile, data->version); break; case protocol_v1: case protocol_v0: - refs = fetch_pack(&args, data->fd, data->conn, + refs = fetch_pack(&args, data->fd, refs_tmp ? refs_tmp : transport->remote_refs, - dest, to_fetch, nr_heads, &data->shallow, + to_fetch, nr_heads, &data->shallow, &transport->pack_lockfile, data->version); break; case protocol_unknown_version: @@ -389,7 +388,6 @@ static int fetch_refs_via_pack(struct transport *transport, free_refs(refs_tmp); free_refs(refs); - free(dest); return ret; } From 5205749d2ca81ea1d124ba7d799f4b319084ca68 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Mar 2019 04:16:27 -0400 Subject: [PATCH 10/13] parse-options: drop unused ctx parameter from show_gitcomp() The completion display doesn't actually care about where we are in the parsing. It's generated completely from the set of available options. So we don't need to see the parse-options context struct at all. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- parse-options.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/parse-options.c b/parse-options.c index cec74522e5..ade83a7b20 100644 --- a/parse-options.c +++ b/parse-options.c @@ -523,8 +523,7 @@ static void show_negated_gitcomp(const struct option *opts, int nr_noopts) } } -static int show_gitcomp(struct parse_opt_ctx_t *ctx, - const struct option *opts) +static int show_gitcomp(const struct option *opts) { const struct option *original_opts = opts; int nr_noopts = 0; @@ -603,7 +602,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx, /* lone --git-completion-helper is asked by git-completion.bash */ if (ctx->total == 1 && !strcmp(arg + 1, "-git-completion-helper")) - return show_gitcomp(ctx, options); + return show_gitcomp(options); if (arg[1] != '-') { ctx->opt = arg + 1; From da55ff3d84b9edc635aba4a986de25ec219acd7a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Mar 2019 04:16:36 -0400 Subject: [PATCH 11/13] pretty: drop unused "type" parameter in needs_rfc2047_encoding() The "should we encode" check was split off from add_rfc2047() into its own function in 41dd00bad3 (format-patch: fix rfc2047 address encoding with respect to rfc822 specials, 2012-10-18). But only the "add" half needs to know the rfc2047_type, since it only affects _how_ we encode, not whether we do. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- pretty.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pretty.c b/pretty.c index f496f0f128..f925a014f9 100644 --- a/pretty.c +++ b/pretty.c @@ -343,8 +343,7 @@ static int is_rfc2047_special(char ch, enum rfc2047_type type) return !(isalnum(ch) || ch == '!' || ch == '*' || ch == '+' || ch == '-' || ch == '/'); } -static int needs_rfc2047_encoding(const char *line, int len, - enum rfc2047_type type) +static int needs_rfc2047_encoding(const char *line, int len) { int i; @@ -470,7 +469,7 @@ void pp_user_info(struct pretty_print_context *pp, } strbuf_addstr(sb, "From: "); - if (needs_rfc2047_encoding(namebuf, namelen, RFC2047_ADDRESS)) { + if (needs_rfc2047_encoding(namebuf, namelen)) { add_rfc2047(sb, namebuf, namelen, encoding, RFC2047_ADDRESS); max_length = 76; /* per rfc2047 */ @@ -1728,7 +1727,7 @@ void pp_title_line(struct pretty_print_context *pp, if (pp->print_email_subject) { if (pp->rev) fmt_output_email_subject(sb, pp->rev); - if (needs_rfc2047_encoding(title.buf, title.len, RFC2047_SUBJECT)) + if (needs_rfc2047_encoding(title.buf, title.len)) add_rfc2047(sb, title.buf, title.len, encoding, RFC2047_SUBJECT); else From 9a1180fc304ad9831641e5788e9c8d3dfc10ccdd Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Mar 2019 04:16:42 -0400 Subject: [PATCH 12/13] pretty: drop unused strbuf from parse_padding_placeholder() Unlike other parts of the --pretty user-format expansion, this function is not actually writing to the output, but instead just storing the padding values into a context struct. We don't need to be passed a strbuf at all. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- pretty.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pretty.c b/pretty.c index f925a014f9..ced0485257 100644 --- a/pretty.c +++ b/pretty.c @@ -988,8 +988,7 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8 */ return rest - placeholder; } -static size_t parse_padding_placeholder(struct strbuf *sb, - const char *placeholder, +static size_t parse_padding_placeholder(const char *placeholder, struct format_commit_context *c) { const char *ch = placeholder; @@ -1194,7 +1193,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ case '<': case '>': - return parse_padding_placeholder(sb, placeholder, c); + return parse_padding_placeholder(placeholder, c); } /* these depend on the commit */ From 95be717cd5a5d4956a5152210176e598cf49ec75 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Mar 2019 16:22:15 -0400 Subject: [PATCH 13/13] parse_opt_ref_sorting: always use with NONEG flag The "--sort" parameter of for-each-ref, etc, does not handle negation, and instead returns an error to the parse-options code. But neither piece of code prints anything for the user, which may leave them confused: $ git for-each-ref --no-sort $ echo $? 129 As the comment in the callback function notes, this probably should clear the list, which would make it consistent with other list-like options (i.e., anything that uses OPT_STRING_LIST currently). Unfortunately that's a bit tricky due to the way the ref-filter code works. But in the meantime, let's at least make the error a little less confusing: - switch to using PARSE_OPT_NONEG in the option definition, which will cause the options code to produce a useful message - since this was cut-and-pasted to four different spots, let's define a single OPT_REF_SORT() macro that we can use everywhere - the callback can use BUG_ON_OPT_NEG() to make sure the correct flags are used (incidentally, this also satisfies -Wunused-parameters, since we're now looking at "unset") - expand the comment into a NEEDSWORK to make it clear that the direction is right, but the details need to be worked out Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/branch.c | 3 +-- builtin/for-each-ref.c | 3 +-- builtin/ls-remote.c | 3 +-- builtin/tag.c | 3 +-- ref-filter.c | 9 +++++++-- ref-filter.h | 5 +++++ 6 files changed, 16 insertions(+), 10 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 4c83055730..d4359b33ac 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -644,8 +644,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix) OPT_MERGED(&filter, N_("print only branches that are merged")), OPT_NO_MERGED(&filter, N_("print only branches that are not merged")), OPT_COLUMN(0, "column", &colopts, N_("list branches in columns")), - OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"), - N_("field name to sort on"), &parse_opt_ref_sorting), + OPT_REF_SORT(sorting_tail), { OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"), N_("print only branches of the object"), 0, parse_opt_object_name diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index e931be9ce4..465153e853 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -37,8 +37,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) OPT_INTEGER( 0 , "count", &maxcount, N_("show only matched refs")), OPT_STRING( 0 , "format", &format.format, N_("format"), N_("format to use for the output")), OPT__COLOR(&format.use_color, N_("respect format colors")), - OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"), - N_("field name to sort on"), &parse_opt_ref_sorting), + OPT_REF_SORT(sorting_tail), OPT_CALLBACK(0, "points-at", &filter.points_at, N_("object"), N_("print only refs which points at the given object"), parse_opt_object_name), diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c index 1d7f1f5ce2..6ef519514b 100644 --- a/builtin/ls-remote.c +++ b/builtin/ls-remote.c @@ -67,8 +67,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) OPT_BIT(0, "refs", &flags, N_("do not show peeled tags"), REF_NORMAL), OPT_BOOL(0, "get-url", &get_url, N_("take url..insteadOf into account")), - OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"), - N_("field name to sort on"), &parse_opt_ref_sorting), + OPT_REF_SORT(sorting_tail), OPT_SET_INT_F(0, "exit-code", &status, N_("exit with exit code 2 if no matching refs are found"), 2, PARSE_OPT_NOCOMPLETE), diff --git a/builtin/tag.c b/builtin/tag.c index 02f6bd1279..ad97595fbf 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -412,8 +412,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) OPT_WITHOUT(&filter.no_commit, N_("print only tags that don't contain the commit")), OPT_MERGED(&filter, N_("print only tags that are merged")), OPT_NO_MERGED(&filter, N_("print only tags that are not merged")), - OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"), - N_("field name to sort on"), &parse_opt_ref_sorting), + OPT_REF_SORT(sorting_tail), { OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"), N_("print only tags of the object"), PARSE_OPT_LASTARG_DEFAULT, diff --git a/ref-filter.c b/ref-filter.c index 3aca105307..8d11a94cbd 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2337,8 +2337,13 @@ void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *arg) int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset) { - if (!arg) /* should --no-sort void the list ? */ - return -1; + /* + * NEEDSWORK: We should probably clear the list in this case, but we've + * already munged the global used_atoms list, which would need to be + * undone. + */ + BUG_ON_OPT_NEG(unset); + parse_ref_sorting(opt->value, arg); return 0; } diff --git a/ref-filter.h b/ref-filter.h index 85c8ebc3b9..f1dcff4c6e 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -96,6 +96,11 @@ struct ref_format { #define OPT_MERGED(f, h) _OPT_MERGED_NO_MERGED("merged", f, h) #define OPT_NO_MERGED(f, h) _OPT_MERGED_NO_MERGED("no-merged", f, h) +#define OPT_REF_SORT(var) \ + OPT_CALLBACK_F(0, "sort", (var), \ + N_("key"), N_("field name to sort on"), \ + PARSE_OPT_NONEG, parse_opt_ref_sorting) + /* * API for filtering a set of refs. Based on the type of refs the user * has requested, we iterate through those refs and apply filters