Merge branch 'rs/strbuf-expand-step'

Code clean-up around strbuf_expand() API.

* rs/strbuf-expand-step:
  strbuf: simplify strbuf_expand_literal_cb()
  replace strbuf_expand() with strbuf_expand_step()
  replace strbuf_expand_dict_cb() with strbuf_expand_step()
  strbuf: factor out strbuf_expand_step()
  pretty: factor out expand_separator()

Author: gitster

builtin/branch.c

@@ -366,17 +366,8 @@ static const char *quote_literal_for_format(const char *s)
 	static struct strbuf buf = STRBUF_INIT;
 
 	strbuf_reset(&buf);
-	while (*s) {
-		const char *ep = strchrnul(s, '%');
-		if (s < ep)
-			strbuf_add(&buf, s, ep - s);
-		if (*ep == '%') {
-			strbuf_addstr(&buf, "%%");
-			s = ep + 1;
-		} else {
-			s = ep;
-		}
-	}
+	while (strbuf_expand_step(&buf, &s))
+		strbuf_addstr(&buf, "%%");
 	return buf.buf;
 }
 

builtin/cat-file.c

@@ -309,10 +309,8 @@ static int is_atom(const char *atom, const char *s, int slen)
 }
 
 static void expand_atom(struct strbuf *sb, const char *atom, int len,
-			void *vdata)
+			struct expand_data *data)
 {
-	struct expand_data *data = vdata;
-
 	if (is_atom("objectname", atom, len)) {
 		if (!data->mark_query)
 			strbuf_addstr(sb, oid_to_hex(&data->oid));
@@ -346,19 +344,21 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len,
 		die("unknown format element: %.*s", len, atom);
 }
 
-static size_t expand_format(struct strbuf *sb, const char *start, void *data)
+static void expand_format(struct strbuf *sb, const char *start,
+			  struct expand_data *data)
 {
-	const char *end;
-
-	if (*start != '(')
-		return 0;
-	end = strchr(start + 1, ')');
-	if (!end)
-		die("format element '%s' does not end in ')'", start);
-
-	expand_atom(sb, start + 1, end - start - 1, data);
-
-	return end - start + 1;
+	while (strbuf_expand_step(sb, &start)) {
+		const char *end;
+
+		if (skip_prefix(start, "%", &start) || *start != '(')
+			strbuf_addch(sb, '%');
+		else if (!(end = strchr(start + 1, ')')))
+			die("format element '%s' does not end in ')'", start);
+		else {
+			expand_atom(sb, start + 1, end - start - 1, data);
+			start = end + 1;
+		}
+	}
 }
 
 static void batch_write(struct batch_options *opt, const void *data, int len)
@@ -495,7 +495,7 @@ static void batch_object_write(const char *obj_name,
 	if (!opt->format) {
 		print_default_format(scratch, data, opt);
 	} else {
-		strbuf_expand(scratch, opt->format, expand_format, data);
+		expand_format(scratch, opt->format, data);
 		strbuf_addch(scratch, opt->output_delim);
 	}
 
@@ -773,9 +773,8 @@ static int batch_objects(struct batch_options *opt)
 	 */
 	memset(&data, 0, sizeof(data));
 	data.mark_query = 1;
-	strbuf_expand(&output,
+	expand_format(&output,
 		      opt->format ? opt->format : DEFAULT_FORMAT,
-		      expand_format,
 		      &data);
 	data.mark_query = 0;
 	strbuf_release(&output);

builtin/ls-files.c

@@ -264,74 +264,57 @@ static void expand_objectsize(struct strbuf *line, const struct object_id *oid,
 		strbuf_addstr(line, "-");
 	}
 }
-struct show_index_data {
-	const char *pathname;
-	struct index_state *istate;
-	const struct cache_entry *ce;
-};
-
-static size_t expand_show_index(struct strbuf *sb, const char *start,
-				void *context)
-{
-	struct show_index_data *data = context;
-	const char *end;
-	const char *p;
-	size_t len = strbuf_expand_literal_cb(sb, start, NULL);
-	struct stat st;
-
-	if (len)
-		return len;
-	if (*start != '(')
-		die(_("bad ls-files format: element '%s' "
-		      "does not start with '('"), start);
-
-	end = strchr(start + 1, ')');
-	if (!end)
-		die(_("bad ls-files format: element '%s' "
-		      "does not end in ')'"), start);
-
-	len = end - start + 1;
-	if (skip_prefix(start, "(objectmode)", &p))
-		strbuf_addf(sb, "%06o", data->ce->ce_mode);
-	else if (skip_prefix(start, "(objectname)", &p))
-		strbuf_add_unique_abbrev(sb, &data->ce->oid, abbrev);
-	else if (skip_prefix(start, "(objecttype)", &p))
-		strbuf_addstr(sb, type_name(object_type(data->ce->ce_mode)));
-	else if (skip_prefix(start, "(objectsize:padded)", &p))
-		expand_objectsize(sb, &data->ce->oid, object_type(data->ce->ce_mode), 1);
-	else if (skip_prefix(start, "(objectsize)", &p))
-		expand_objectsize(sb, &data->ce->oid, object_type(data->ce->ce_mode), 0);
-	else if (skip_prefix(start, "(stage)", &p))
-		strbuf_addf(sb, "%d", ce_stage(data->ce));
-	else if (skip_prefix(start, "(eolinfo:index)", &p))
-		strbuf_addstr(sb, S_ISREG(data->ce->ce_mode) ?
-			      get_cached_convert_stats_ascii(data->istate,
-			      data->ce->name) : "");
-	else if (skip_prefix(start, "(eolinfo:worktree)", &p))
-		strbuf_addstr(sb, !lstat(data->pathname, &st) &&
-			      S_ISREG(st.st_mode) ?
-			      get_wt_convert_stats_ascii(data->pathname) : "");
-	else if (skip_prefix(start, "(eolattr)", &p))
-		strbuf_addstr(sb, get_convert_attr_ascii(data->istate,
-			      data->pathname));
-	else if (skip_prefix(start, "(path)", &p))
-		write_name_to_buf(sb, data->pathname);
-	else
-		die(_("bad ls-files format: %%%.*s"), (int)len, start);
-
-	return len;
-}
 
 static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,
 			const char *format, const char *fullname) {
-	struct show_index_data data = {
-		.pathname = fullname,
-		.istate = repo->index,
-		.ce = ce,
-	};
 	struct strbuf sb = STRBUF_INIT;
 
-	strbuf_expand(&sb, format, expand_show_index, &data);
+	while (strbuf_expand_step(&sb, &format)) {
+		const char *end;
+		size_t len;
+		struct stat st;
+
+		if (skip_prefix(format, "%", &format))
+			strbuf_addch(&sb, '%');
+		else if ((len = strbuf_expand_literal(&sb, format)))
+			format += len;
+		else if (*format != '(')
+			die(_("bad ls-files format: element '%s' "
+			      "does not start with '('"), format);
+		else if (!(end = strchr(format + 1, ')')))
+			die(_("bad ls-files format: element '%s' "
+			      "does not end in ')'"), format);
+		else if (skip_prefix(format, "(objectmode)", &format))
+			strbuf_addf(&sb, "%06o", ce->ce_mode);
+		else if (skip_prefix(format, "(objectname)", &format))
+			strbuf_add_unique_abbrev(&sb, &ce->oid, abbrev);
+		else if (skip_prefix(format, "(objecttype)", &format))
+			strbuf_addstr(&sb, type_name(object_type(ce->ce_mode)));
+		else if (skip_prefix(format, "(objectsize:padded)", &format))
+			expand_objectsize(&sb, &ce->oid,
+					  object_type(ce->ce_mode), 1);
+		else if (skip_prefix(format, "(objectsize)", &format))
+			expand_objectsize(&sb, &ce->oid,
+					  object_type(ce->ce_mode), 0);
+		else if (skip_prefix(format, "(stage)", &format))
+			strbuf_addf(&sb, "%d", ce_stage(ce));
+		else if (skip_prefix(format, "(eolinfo:index)", &format))
+			strbuf_addstr(&sb, S_ISREG(ce->ce_mode) ?
+				      get_cached_convert_stats_ascii(repo->index,
+				      ce->name) : "");
+		else if (skip_prefix(format, "(eolinfo:worktree)", &format))
+			strbuf_addstr(&sb, !lstat(fullname, &st) &&
+				      S_ISREG(st.st_mode) ?
+				      get_wt_convert_stats_ascii(fullname) : "");
+		else if (skip_prefix(format, "(eolattr)", &format))
+			strbuf_addstr(&sb, get_convert_attr_ascii(repo->index,
+								 fullname));
+		else if (skip_prefix(format, "(path)", &format))
+			write_name_to_buf(&sb, fullname);
+		else
+			die(_("bad ls-files format: %%%.*s"),
+			    (int)(end - format + 1), format);
+	}
 	strbuf_addch(&sb, line_terminator);
 	fwrite(sb.buf, sb.len, 1, stdout);
 	strbuf_release(&sb);

builtin/ls-tree.c

@@ -55,63 +55,6 @@ struct ls_tree_options {
 	const char *format;
 };
 
-struct show_tree_data {
-	struct ls_tree_options *options;
-	unsigned mode;
-	enum object_type type;
-	const struct object_id *oid;
-	const char *pathname;
-	struct strbuf *base;
-};
-
-static size_t expand_show_tree(struct strbuf *sb, const char *start,
-			       void *context)
-{
-	struct show_tree_data *data = context;
-	struct ls_tree_options *options = data->options;
-	const char *end;
-	const char *p;
-	unsigned int errlen;
-	size_t len = strbuf_expand_literal_cb(sb, start, NULL);
-
-	if (len)
-		return len;
-	if (*start != '(')
-		die(_("bad ls-tree format: element '%s' does not start with '('"), start);
-
-	end = strchr(start + 1, ')');
-	if (!end)
-		die(_("bad ls-tree format: element '%s' does not end in ')'"), start);
-
-	len = end - start + 1;
-	if (skip_prefix(start, "(objectmode)", &p)) {
-		strbuf_addf(sb, "%06o", data->mode);
-	} else if (skip_prefix(start, "(objecttype)", &p)) {
-		strbuf_addstr(sb, type_name(data->type));
-	} else if (skip_prefix(start, "(objectsize:padded)", &p)) {
-		expand_objectsize(sb, data->oid, data->type, 1);
-	} else if (skip_prefix(start, "(objectsize)", &p)) {
-		expand_objectsize(sb, data->oid, data->type, 0);
-	} else if (skip_prefix(start, "(objectname)", &p)) {
-		strbuf_add_unique_abbrev(sb, data->oid, options->abbrev);
-	} else if (skip_prefix(start, "(path)", &p)) {
-		const char *name = data->base->buf;
-		const char *prefix = options->chomp_prefix ? options->ls_tree_prefix : NULL;
-		struct strbuf sbuf = STRBUF_INIT;
-		size_t baselen = data->base->len;
-
-		strbuf_addstr(data->base, data->pathname);
-		name = relative_path(data->base->buf, prefix, &sbuf);
-		quote_c_style(name, sb, NULL, 0);
-		strbuf_setlen(data->base, baselen);
-		strbuf_release(&sbuf);
-	} else {
-		errlen = (unsigned long)len;
-		die(_("bad ls-tree format: %%%.*s"), errlen, start);
-	}
-	return len;
-}
-
 static int show_recursive(struct ls_tree_options *options, const char *base,
 			  size_t baselen, const char *pathname)
 {
@@ -150,14 +93,7 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,
 	int recurse = 0;
 	struct strbuf sb = STRBUF_INIT;
 	enum object_type type = object_type(mode);
-	struct show_tree_data cb_data = {
-		.options = options,
-		.mode = mode,
-		.type = type,
-		.oid = oid,
-		.pathname = pathname,
-		.base = base,
-	};
+	const char *format = options->format;
 
 	if (type == OBJ_TREE && show_recursive(options, base->buf, base->len, pathname))
 		recurse = READ_TREE_RECURSIVE;
@@ -166,7 +102,46 @@ static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,
 	if (type == OBJ_BLOB && (options->ls_options & LS_TREE_ONLY))
 		return 0;
 
-	strbuf_expand(&sb, options->format, expand_show_tree, &cb_data);
+	while (strbuf_expand_step(&sb, &format)) {
+		const char *end;
+		size_t len;
+
+		if (skip_prefix(format, "%", &format))
+			strbuf_addch(&sb, '%');
+		else if ((len = strbuf_expand_literal(&sb, format)))
+			format += len;
+		else if (*format != '(')
+			die(_("bad ls-tree format: element '%s' "
+			      "does not start with '('"), format);
+		else if (!(end = strchr(format + 1, ')')))
+			die(_("bad ls-tree format: element '%s' "
+			      "does not end in ')'"), format);
+		else if (skip_prefix(format, "(objectmode)", &format))
+			strbuf_addf(&sb, "%06o", mode);
+		else if (skip_prefix(format, "(objecttype)", &format))
+			strbuf_addstr(&sb, type_name(type));
+		else if (skip_prefix(format, "(objectsize:padded)", &format))
+			expand_objectsize(&sb, oid, type, 1);
+		else if (skip_prefix(format, "(objectsize)", &format))
+			expand_objectsize(&sb, oid, type, 0);
+		else if (skip_prefix(format, "(objectname)", &format))
+			strbuf_add_unique_abbrev(&sb, oid, options->abbrev);
+		else if (skip_prefix(format, "(path)", &format)) {
+			const char *name;
+			const char *prefix = options->chomp_prefix ?
+					     options->ls_tree_prefix : NULL;
+			struct strbuf sbuf = STRBUF_INIT;
+			size_t baselen = base->len;
+
+			strbuf_addstr(base, pathname);
+			name = relative_path(base->buf, prefix, &sbuf);
+			quote_c_style(name, &sb, NULL, 0);
+			strbuf_setlen(base, baselen);
+			strbuf_release(&sbuf);
+		} else
+			die(_("bad ls-tree format: %%%.*s"),
+			    (int)(end - format + 1), format);
+	}
 	strbuf_addch(&sb, options->null_termination ? '\0' : '\n');
 	fwrite(sb.buf, sb.len, 1, stdout);
 	strbuf_release(&sb);

convert.c

@@ -634,23 +634,21 @@ static int filter_buffer_or_fd(int in UNUSED, int out, void *data)
 	 */
 	struct child_process child_process = CHILD_PROCESS_INIT;
 	struct filter_params *params = (struct filter_params *)data;
+	const char *format = params->cmd;
 	int write_err, status;
 
 	/* apply % substitution to cmd */
 	struct strbuf cmd = STRBUF_INIT;
-	struct strbuf path = STRBUF_INIT;
-	struct strbuf_expand_dict_entry dict[] = {
-		{ "f", NULL, },
-		{ NULL, NULL, },
-	};
-
-	/* quote the path to preserve spaces, etc. */
-	sq_quote_buf(&path, params->path);
-	dict[0].value = path.buf;
 
-	/* expand all %f with the quoted path */
-	strbuf_expand(&cmd, params->cmd, strbuf_expand_dict_cb, &dict);
-	strbuf_release(&path);
+	/* expand all %f with the quoted path; quote to preserve space, etc. */
+	while (strbuf_expand_step(&cmd, &format)) {
+		if (skip_prefix(format, "%", &format))
+			strbuf_addch(&cmd, '%');
+		else if (skip_prefix(format, "f", &format))
+			sq_quote_buf(&cmd, params->path);
+		else
+			strbuf_addch(&cmd, '%');
+	}
 
 	strvec_push(&child_process.args, cmd.buf);
 	child_process.use_shell = 1;