* [PATCH v3 0/2] Reduce parse-options.o dependencies @ 2011-08-11 9:15 Dmitry Ivankov 2011-08-11 9:15 ` [PATCH v3 1/2] parse-options: export opterr, optbug Dmitry Ivankov 2011-08-11 9:15 ` [PATCH v3 2/2] Reduce parse-options.o dependencies Dmitry Ivankov 0 siblings, 2 replies; 6+ messages in thread From: Dmitry Ivankov @ 2011-08-11 9:15 UTC (permalink / raw) To: git; +Cc: Jonathan Nieder, David Barr, Dmitry Ivankov This is a reroll of [1]. The main purpose is to make parse-options.o more self-contained so that it at least doesn't pull any extlibs. Immediate usage is for stuff in contrib (svn-fe). Distant application is that some day we could publish this small library separately. Mostly no changes since [1], just commit message line wrapping, fixup for a Makefile typo and rebase on top of master (new parse_opt_string_list moved to parse-options-cb.c too). [1] http://thread.gmane.org/gmane.comp.version-control.git/176318/focus=176574 Dmitry Ivankov (2): parse-options: export opterr, optbug Reduce parse-options.o dependencies Makefile | 3 +- abspath.c | 28 ++++++++++++ parse-options-cb.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++ parse-options.c | 125 +--------------------------------------------------- parse-options.h | 2 + setup.c | 28 ------------ 6 files changed, 159 insertions(+), 152 deletions(-) create mode 100644 parse-options-cb.c -- 1.7.3.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] parse-options: export opterr, optbug 2011-08-11 9:15 [PATCH v3 0/2] Reduce parse-options.o dependencies Dmitry Ivankov @ 2011-08-11 9:15 ` Dmitry Ivankov 2011-08-11 10:42 ` Jonathan Nieder 2011-08-11 9:15 ` [PATCH v3 2/2] Reduce parse-options.o dependencies Dmitry Ivankov 1 sibling, 1 reply; 6+ messages in thread From: Dmitry Ivankov @ 2011-08-11 9:15 UTC (permalink / raw) To: git; +Cc: Jonathan Nieder, David Barr, Dmitry Ivankov opterror and optbug functions are used by some of parsing routines in parse-options.c to report errors and bugs respectively. Export these functions to allow more custom parsing routines to use them in a uniform way. Signed-off-by: Dmitry Ivankov <divanorama@gmail.com> --- parse-options.c | 4 ++-- parse-options.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/parse-options.c b/parse-options.c index 879ea82..7b061af 100644 --- a/parse-options.c +++ b/parse-options.c @@ -12,14 +12,14 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx, #define OPT_SHORT 1 #define OPT_UNSET 2 -static int optbug(const struct option *opt, const char *reason) +int optbug(const struct option *opt, const char *reason) { if (opt->long_name) return error("BUG: option '%s' %s", opt->long_name, reason); return error("BUG: switch '%c' %s", opt->short_name, reason); } -static int opterror(const struct option *opt, const char *reason, int flags) +int opterror(const struct option *opt, const char *reason, int flags) { if (flags & OPT_SHORT) return error("switch `%c' %s", opt->short_name, reason); diff --git a/parse-options.h b/parse-options.h index 05eb09b..59e0b52 100644 --- a/parse-options.h +++ b/parse-options.h @@ -165,6 +165,8 @@ extern NORETURN void usage_msg_opt(const char *msg, const char * const *usagestr, const struct option *options); +extern int optbug(const struct option *opt, const char *reason); +extern int opterror(const struct option *opt, const char *reason, int flags); /*----- incremental advanced APIs -----*/ enum { -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] parse-options: export opterr, optbug 2011-08-11 9:15 ` [PATCH v3 1/2] parse-options: export opterr, optbug Dmitry Ivankov @ 2011-08-11 10:42 ` Jonathan Nieder 0 siblings, 0 replies; 6+ messages in thread From: Jonathan Nieder @ 2011-08-11 10:42 UTC (permalink / raw) To: Dmitry Ivankov; +Cc: git, David Barr, Stephen Boyd (+cc: Stephen) Dmitry Ivankov wrote: > opterror and optbug functions are used by some of parsing routines > in parse-options.c to report errors and bugs respectively. > > Export these functions to allow more custom parsing routines to use > them in a uniform way. In other words, exposing opterror() allows custom option types to behave more like the built-in ones when producing messages like "option `<opt>' expects a numerical value". What should they pass in the "flags" argument? Does this deserve a mention in the "Option Callbacks" section of Documentation/technical/api-parse-options.txt? Would opterror() be enough? I don't see any current users of optbug outside of parse_options_check() (which is part of low-level machinery). Aside from that, seems sensible. [quoting in full for Stephen's convenience. One quick comment below.] > > Signed-off-by: Dmitry Ivankov <divanorama@gmail.com> > --- > parse-options.c | 4 ++-- > parse-options.h | 2 ++ > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/parse-options.c b/parse-options.c > index 879ea82..7b061af 100644 > --- a/parse-options.c > +++ b/parse-options.c > @@ -12,14 +12,14 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx, > #define OPT_SHORT 1 > #define OPT_UNSET 2 > > -static int optbug(const struct option *opt, const char *reason) > +int optbug(const struct option *opt, const char *reason) > { > if (opt->long_name) > return error("BUG: option '%s' %s", opt->long_name, reason); > return error("BUG: switch '%c' %s", opt->short_name, reason); > } > > -static int opterror(const struct option *opt, const char *reason, int flags) > +int opterror(const struct option *opt, const char *reason, int flags) > { > if (flags & OPT_SHORT) > return error("switch `%c' %s", opt->short_name, reason); > diff --git a/parse-options.h b/parse-options.h > index 05eb09b..59e0b52 100644 > --- a/parse-options.h > +++ b/parse-options.h > @@ -165,6 +165,8 @@ extern NORETURN void usage_msg_opt(const char *msg, > const char * const *usagestr, > const struct option *options); > > +extern int optbug(const struct option *opt, const char *reason); > +extern int opterror(const struct option *opt, const char *reason, int flags); > /*----- incremental advanced APIs -----*/ A blank line above the comment would make this more readable. > > enum { > -- Except as noted above, Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Some (though not all) of the possible changes noted above implemented below, plus an example caller (which made reading the patch a little easier). What do you think? builtin/read-tree.c | 2 +- parse-options.c | 2 +- parse-options.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git c/builtin/read-tree.c i/builtin/read-tree.c index df6c4c88..6013090c 100644 --- c/builtin/read-tree.c +++ i/builtin/read-tree.c @@ -53,7 +53,7 @@ static int exclude_per_directory_cb(const struct option *opt, const char *arg, opts = (struct unpack_trees_options *)opt->value; if (opts->dir) - die("more than one --exclude-per-directory given."); + return opterror(opt, "can only be supplied once", 0); dir = xcalloc(1, sizeof(*opts->dir)); dir->flags |= DIR_SHOW_IGNORED; diff --git c/parse-options.c i/parse-options.c index 7b061afc..777611b1 100644 --- c/parse-options.c +++ i/parse-options.c @@ -12,7 +12,7 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx, #define OPT_SHORT 1 #define OPT_UNSET 2 -int optbug(const struct option *opt, const char *reason) +static int optbug(const struct option *opt, const char *reason) { if (opt->long_name) return error("BUG: option '%s' %s", opt->long_name, reason); diff --git c/parse-options.h i/parse-options.h index 59e0b524..6d31ad3a 100644 --- c/parse-options.h +++ i/parse-options.h @@ -165,8 +165,8 @@ extern NORETURN void usage_msg_opt(const char *msg, const char * const *usagestr, const struct option *options); -extern int optbug(const struct option *opt, const char *reason); extern int opterror(const struct option *opt, const char *reason, int flags); + /*----- incremental advanced APIs -----*/ enum { ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] Reduce parse-options.o dependencies 2011-08-11 9:15 [PATCH v3 0/2] Reduce parse-options.o dependencies Dmitry Ivankov 2011-08-11 9:15 ` [PATCH v3 1/2] parse-options: export opterr, optbug Dmitry Ivankov @ 2011-08-11 9:15 ` Dmitry Ivankov 2011-08-11 11:04 ` Jonathan Nieder 1 sibling, 1 reply; 6+ messages in thread From: Dmitry Ivankov @ 2011-08-11 9:15 UTC (permalink / raw) To: git; +Cc: Jonathan Nieder, David Barr, Dmitry Ivankov Currently parse-options.o pulls quite a big bunch of dependencies. his complicates it's usage in contrib/ because it pulls external dependencies and it also increases executables size. Split off less generic and more internal to git part of parse-options.c to parse-options-cb.c. Move prefix_filename function from setup.c to abspath.c. abspath.o and wrapper.o pull each other, so it's unlikely to increase the dependencies. It was a dependency of parse-options.o that pulled many others. Now parse-options.o pulls just abspath.o, ctype.o, strbuf.o, usage.o, wrapper.o, libc directly and strlcpy.o indirectly. Signed-off-by: Dmitry Ivankov <divanorama@gmail.com> --- Makefile | 3 +- abspath.c | 28 ++++++++++++ parse-options-cb.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++ parse-options.c | 121 -------------------------------------------------- setup.c | 28 ------------ 5 files changed, 155 insertions(+), 150 deletions(-) create mode 100644 parse-options-cb.c diff --git a/Makefile b/Makefile index 62ad0c2..7d47bdb 100644 --- a/Makefile +++ b/Makefile @@ -642,6 +642,7 @@ LIB_OBJS += pack-revindex.o LIB_OBJS += pack-write.o LIB_OBJS += pager.o LIB_OBJS += parse-options.o +LIB_OBJS += parse-options-cb.o LIB_OBJS += patch-delta.o LIB_OBJS += patch-ids.o LIB_OBJS += path.o @@ -2204,7 +2205,7 @@ test-delta$X: diff-delta.o patch-delta.o test-line-buffer$X: vcs-svn/lib.a -test-parse-options$X: parse-options.o +test-parse-options$X: parse-options.o parse-options-cb.o test-string-pool$X: vcs-svn/lib.a diff --git a/abspath.c b/abspath.c index 37287f8..f04ac18 100644 --- a/abspath.c +++ b/abspath.c @@ -139,3 +139,31 @@ const char *absolute_path(const char *path) } return buf; } + +/* + * Unlike prefix_path, this should be used if the named file does + * not have to interact with index entry; i.e. name of a random file + * on the filesystem. + */ +const char *prefix_filename(const char *pfx, int pfx_len, const char *arg) +{ + static char path[PATH_MAX]; +#ifndef WIN32 + if (!pfx_len || is_absolute_path(arg)) + return arg; + memcpy(path, pfx, pfx_len); + strcpy(path + pfx_len, arg); +#else + char *p; + /* don't add prefix to absolute paths, but still replace '\' by '/' */ + if (is_absolute_path(arg)) + pfx_len = 0; + else if (pfx_len) + memcpy(path, pfx, pfx_len); + strcpy(path + pfx_len, arg); + for (p = path + pfx_len; *p; p++) + if (*p == '\\') + *p = '/'; +#endif + return path; +} diff --git a/parse-options-cb.c b/parse-options-cb.c new file mode 100644 index 0000000..c248f66 --- /dev/null +++ b/parse-options-cb.c @@ -0,0 +1,125 @@ +#include "git-compat-util.h" +#include "parse-options.h" +#include "cache.h" +#include "commit.h" +#include "color.h" +#include "string-list.h" + +/*----- some often used options -----*/ + +int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset) +{ + int v; + + if (!arg) { + v = unset ? 0 : DEFAULT_ABBREV; + } else { + v = strtol(arg, (char **)&arg, 10); + if (*arg) + return opterror(opt, "expects a numerical value", 0); + if (v && v < MINIMUM_ABBREV) + v = MINIMUM_ABBREV; + else if (v > 40) + v = 40; + } + *(int *)(opt->value) = v; + return 0; +} + +int parse_opt_approxidate_cb(const struct option *opt, const char *arg, + int unset) +{ + *(unsigned long *)(opt->value) = approxidate(arg); + return 0; +} + +int parse_opt_color_flag_cb(const struct option *opt, const char *arg, + int unset) +{ + int value; + + if (!arg) + arg = unset ? "never" : (const char *)opt->defval; + value = git_config_colorbool(NULL, arg, -1); + if (value < 0) + return opterror(opt, + "expects \"always\", \"auto\", or \"never\"", 0); + *(int *)opt->value = value; + return 0; +} + +int parse_opt_verbosity_cb(const struct option *opt, const char *arg, + int unset) +{ + int *target = opt->value; + + if (unset) + /* --no-quiet, --no-verbose */ + *target = 0; + else if (opt->short_name == 'v') { + if (*target >= 0) + (*target)++; + else + *target = 1; + } else { + if (*target <= 0) + (*target)--; + else + *target = -1; + } + return 0; +} + +int parse_opt_with_commit(const struct option *opt, const char *arg, int unset) +{ + unsigned char sha1[20]; + struct commit *commit; + + if (!arg) + return -1; + if (get_sha1(arg, sha1)) + return error("malformed object name %s", arg); + commit = lookup_commit_reference(sha1); + if (!commit) + return error("no such commit %s", arg); + commit_list_insert(commit, opt->value); + return 0; +} + +int parse_opt_tertiary(const struct option *opt, const char *arg, int unset) +{ + int *target = opt->value; + *target = unset ? 2 : 1; + return 0; +} + +int parse_options_concat(struct option *dst, size_t dst_size, struct option *src) +{ + int i, j; + + for (i = 0; i < dst_size; i++) + if (dst[i].type == OPTION_END) + break; + for (j = 0; i < dst_size; i++, j++) { + dst[i] = src[j]; + if (src[j].type == OPTION_END) + return 0; + } + return -1; +} + +int parse_opt_string_list(const struct option *opt, const char *arg, int unset) +{ + struct string_list *v = opt->value; + + if (unset) { + string_list_clear(v, 0); + return 0; + } + + if (!arg) + return -1; + + string_list_append(v, xstrdup(arg)); + return 0; +} diff --git a/parse-options.c b/parse-options.c index 7b061af..503ab5d 100644 --- a/parse-options.c +++ b/parse-options.c @@ -3,7 +3,6 @@ #include "cache.h" #include "commit.h" #include "color.h" -#include "string-list.h" static int parse_options_usage(struct parse_opt_ctx_t *ctx, const char * const *usagestr, @@ -584,123 +583,3 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx, return usage_with_options_internal(ctx, usagestr, opts, 0, err); } - -/*----- some often used options -----*/ -#include "cache.h" - -int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset) -{ - int v; - - if (!arg) { - v = unset ? 0 : DEFAULT_ABBREV; - } else { - v = strtol(arg, (char **)&arg, 10); - if (*arg) - return opterror(opt, "expects a numerical value", 0); - if (v && v < MINIMUM_ABBREV) - v = MINIMUM_ABBREV; - else if (v > 40) - v = 40; - } - *(int *)(opt->value) = v; - return 0; -} - -int parse_opt_approxidate_cb(const struct option *opt, const char *arg, - int unset) -{ - *(unsigned long *)(opt->value) = approxidate(arg); - return 0; -} - -int parse_opt_color_flag_cb(const struct option *opt, const char *arg, - int unset) -{ - int value; - - if (!arg) - arg = unset ? "never" : (const char *)opt->defval; - value = git_config_colorbool(NULL, arg, -1); - if (value < 0) - return opterror(opt, - "expects \"always\", \"auto\", or \"never\"", 0); - *(int *)opt->value = value; - return 0; -} - -int parse_opt_verbosity_cb(const struct option *opt, const char *arg, - int unset) -{ - int *target = opt->value; - - if (unset) - /* --no-quiet, --no-verbose */ - *target = 0; - else if (opt->short_name == 'v') { - if (*target >= 0) - (*target)++; - else - *target = 1; - } else { - if (*target <= 0) - (*target)--; - else - *target = -1; - } - return 0; -} - -int parse_opt_with_commit(const struct option *opt, const char *arg, int unset) -{ - unsigned char sha1[20]; - struct commit *commit; - - if (!arg) - return -1; - if (get_sha1(arg, sha1)) - return error("malformed object name %s", arg); - commit = lookup_commit_reference(sha1); - if (!commit) - return error("no such commit %s", arg); - commit_list_insert(commit, opt->value); - return 0; -} - -int parse_opt_tertiary(const struct option *opt, const char *arg, int unset) -{ - int *target = opt->value; - *target = unset ? 2 : 1; - return 0; -} - -int parse_options_concat(struct option *dst, size_t dst_size, struct option *src) -{ - int i, j; - - for (i = 0; i < dst_size; i++) - if (dst[i].type == OPTION_END) - break; - for (j = 0; i < dst_size; i++, j++) { - dst[i] = src[j]; - if (src[j].type == OPTION_END) - return 0; - } - return -1; -} - -int parse_opt_string_list(const struct option *opt, const char *arg, int unset) -{ - struct string_list *v = opt->value; - - if (unset) { - string_list_clear(v, 0); - return 0; - } - - if (!arg) - return -1; - - string_list_append(v, xstrdup(arg)); - return 0; -} diff --git a/setup.c b/setup.c index 5ea5502..3463819 100644 --- a/setup.c +++ b/setup.c @@ -40,34 +40,6 @@ char *prefix_path(const char *prefix, int len, const char *path) return sanitized; } -/* - * Unlike prefix_path, this should be used if the named file does - * not have to interact with index entry; i.e. name of a random file - * on the filesystem. - */ -const char *prefix_filename(const char *pfx, int pfx_len, const char *arg) -{ - static char path[PATH_MAX]; -#ifndef WIN32 - if (!pfx_len || is_absolute_path(arg)) - return arg; - memcpy(path, pfx, pfx_len); - strcpy(path + pfx_len, arg); -#else - char *p; - /* don't add prefix to absolute paths, but still replace '\' by '/' */ - if (is_absolute_path(arg)) - pfx_len = 0; - else if (pfx_len) - memcpy(path, pfx, pfx_len); - strcpy(path + pfx_len, arg); - for (p = path + pfx_len; *p; p++) - if (*p == '\\') - *p = '/'; -#endif - return path; -} - int check_filename(const char *prefix, const char *arg) { const char *name; -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] Reduce parse-options.o dependencies 2011-08-11 9:15 ` [PATCH v3 2/2] Reduce parse-options.o dependencies Dmitry Ivankov @ 2011-08-11 11:04 ` Jonathan Nieder 2011-08-11 23:35 ` Junio C Hamano 0 siblings, 1 reply; 6+ messages in thread From: Jonathan Nieder @ 2011-08-11 11:04 UTC (permalink / raw) To: Dmitry Ivankov; +Cc: git, David Barr, Stephen Boyd (+cc: Stephen) Dmitry Ivankov wrote: > Currently parse-options.o pulls quite a big bunch of dependencies. > his complicates it's usage in contrib/ because it pulls external > dependencies and it also increases executables size. > > Split off less generic and more internal to git part of > parse-options.c to parse-options-cb.c. > > Move prefix_filename function from setup.c to abspath.c. abspath.o > and wrapper.o pull each other, so it's unlikely to increase the > dependencies. It was a dependency of parse-options.o that pulled > many others. > > Now parse-options.o pulls just abspath.o, ctype.o, strbuf.o, usage.o, > wrapper.o, libc directly and strlcpy.o indirectly. So, in other words, currently linking to parse-options involves linking to git's object access machinery and diff machinery, hence libz, libssl for SHA-1, libpcre, etc. That is a waste of space, startup time, and build complexity for simple programs that do not need access to git's object db. This patch does two things to address that: - option callbacks which freely use the git object db and other facilities move to a separate parse-options-cb.o translation unit, so simple programs can avoid them; - prefix_filename, which is used to support OPTION_FILENAME, moves from setup.c to abspath.c, so it can be used without pulling in unrelated git machinery. The result is, as you say, that use of parse-options.o only pulls in abspath, ctype, strbuf, usage, wrapper, and -lc --- which is to say, just utility functions --- and you can build a program using parse-options with a simple commandline like cc -o test-program -Wall -W -O2 test-program.c libgit.a Okay, on to the patch itself (comments sparsely scattered within). > > Signed-off-by: Dmitry Ivankov <divanorama@gmail.com> > --- > Makefile | 3 +- > abspath.c | 28 ++++++++++++ > parse-options-cb.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > parse-options.c | 121 -------------------------------------------------- > setup.c | 28 ------------ > 5 files changed, 155 insertions(+), 150 deletions(-) > create mode 100644 parse-options-cb.c > > diff --git a/Makefile b/Makefile > index 62ad0c2..7d47bdb 100644 > --- a/Makefile > +++ b/Makefile > @@ -642,6 +642,7 @@ LIB_OBJS += pack-revindex.o > LIB_OBJS += pack-write.o > LIB_OBJS += pager.o > LIB_OBJS += parse-options.o > +LIB_OBJS += parse-options-cb.o > LIB_OBJS += patch-delta.o > LIB_OBJS += patch-ids.o > LIB_OBJS += path.o > @@ -2204,7 +2205,7 @@ test-delta$X: diff-delta.o patch-delta.o > > test-line-buffer$X: vcs-svn/lib.a > > -test-parse-options$X: parse-options.o > +test-parse-options$X: parse-options.o parse-options-cb.o > > test-string-pool$X: vcs-svn/lib.a > > diff --git a/abspath.c b/abspath.c > index 37287f8..f04ac18 100644 > --- a/abspath.c > +++ b/abspath.c > @@ -139,3 +139,31 @@ const char *absolute_path(const char *path) > } > return buf; > } > + > +/* > + * Unlike prefix_path, this should be used if the named file does > + * not have to interact with index entry; i.e. name of a random file > + * on the filesystem. > + */ > +const char *prefix_filename(const char *pfx, int pfx_len, const char *arg) > +{ > + static char path[PATH_MAX]; > +#ifndef WIN32 > + if (!pfx_len || is_absolute_path(arg)) > + return arg; > + memcpy(path, pfx, pfx_len); > + strcpy(path + pfx_len, arg); > +#else > + char *p; > + /* don't add prefix to absolute paths, but still replace '\' by '/' */ > + if (is_absolute_path(arg)) > + pfx_len = 0; > + else if (pfx_len) > + memcpy(path, pfx, pfx_len); > + strcpy(path + pfx_len, arg); > + for (p = path + pfx_len; *p; p++) > + if (*p == '\\') > + *p = '/'; > +#endif > + return path; > +} > diff --git a/parse-options-cb.c b/parse-options-cb.c > new file mode 100644 > index 0000000..c248f66 > --- /dev/null > +++ b/parse-options-cb.c > @@ -0,0 +1,125 @@ > +#include "git-compat-util.h" > +#include "parse-options.h" > +#include "cache.h" Style: Files in git tend to use only one of "git-compat-util.h", "cache.h", or "builtin.h" and put it at the top. So in this case, it should probably use just "cache.h". > +#include "commit.h" > +#include "color.h" > +#include "string-list.h" > + > +/*----- some often used options -----*/ > + > +int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset) > +{ > + int v; > + > + if (!arg) { > + v = unset ? 0 : DEFAULT_ABBREV; > + } else { > + v = strtol(arg, (char **)&arg, 10); > + if (*arg) > + return opterror(opt, "expects a numerical value", 0); > + if (v && v < MINIMUM_ABBREV) > + v = MINIMUM_ABBREV; > + else if (v > 40) > + v = 40; > + } > + *(int *)(opt->value) = v; > + return 0; > +} > + > +int parse_opt_approxidate_cb(const struct option *opt, const char *arg, > + int unset) > +{ > + *(unsigned long *)(opt->value) = approxidate(arg); > + return 0; > +} > + > +int parse_opt_color_flag_cb(const struct option *opt, const char *arg, > + int unset) > +{ > + int value; > + > + if (!arg) > + arg = unset ? "never" : (const char *)opt->defval; > + value = git_config_colorbool(NULL, arg, -1); > + if (value < 0) > + return opterror(opt, > + "expects \"always\", \"auto\", or \"never\"", 0); > + *(int *)opt->value = value; > + return 0; > +} > + > +int parse_opt_verbosity_cb(const struct option *opt, const char *arg, > + int unset) > +{ > + int *target = opt->value; > + > + if (unset) > + /* --no-quiet, --no-verbose */ > + *target = 0; > + else if (opt->short_name == 'v') { > + if (*target >= 0) > + (*target)++; > + else > + *target = 1; > + } else { > + if (*target <= 0) > + (*target)--; > + else > + *target = -1; > + } > + return 0; > +} > + > +int parse_opt_with_commit(const struct option *opt, const char *arg, int unset) > +{ > + unsigned char sha1[20]; > + struct commit *commit; > + > + if (!arg) > + return -1; > + if (get_sha1(arg, sha1)) > + return error("malformed object name %s", arg); > + commit = lookup_commit_reference(sha1); > + if (!commit) > + return error("no such commit %s", arg); > + commit_list_insert(commit, opt->value); > + return 0; > +} > + > +int parse_opt_tertiary(const struct option *opt, const char *arg, int unset) > +{ > + int *target = opt->value; > + *target = unset ? 2 : 1; > + return 0; > +} > + > +int parse_options_concat(struct option *dst, size_t dst_size, struct option *src) > +{ > + int i, j; > + > + for (i = 0; i < dst_size; i++) > + if (dst[i].type == OPTION_END) > + break; > + for (j = 0; i < dst_size; i++, j++) { > + dst[i] = src[j]; > + if (src[j].type == OPTION_END) > + return 0; > + } > + return -1; > +} > + > +int parse_opt_string_list(const struct option *opt, const char *arg, int unset) > +{ > + struct string_list *v = opt->value; > + > + if (unset) { > + string_list_clear(v, 0); > + return 0; > + } > + > + if (!arg) > + return -1; > + > + string_list_append(v, xstrdup(arg)); > + return 0; > +} > diff --git a/parse-options.c b/parse-options.c > index 7b061af..503ab5d 100644 > --- a/parse-options.c > +++ b/parse-options.c > @@ -3,7 +3,6 @@ > #include "cache.h" > #include "commit.h" > #include "color.h" > -#include "string-list.h" > > static int parse_options_usage(struct parse_opt_ctx_t *ctx, > const char * const *usagestr, > @@ -584,123 +583,3 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx, > return usage_with_options_internal(ctx, usagestr, opts, 0, err); > } > > - > -/*----- some often used options -----*/ > -#include "cache.h" > - > -int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset) [... snip code moved verbatim ...] > -} > diff --git a/setup.c b/setup.c > index 5ea5502..3463819 100644 > --- a/setup.c > +++ b/setup.c > @@ -40,34 +40,6 @@ char *prefix_path(const char *prefix, int len, const char *path) > return sanitized; > } > > -/* > - * Unlike prefix_path, this should be used if the named file does > - * not have to interact with index entry; i.e. name of a random file > - * on the filesystem. > - */ > -const char *prefix_filename(const char *pfx, int pfx_len, const char *arg) > -{ > - static char path[PATH_MAX]; > -#ifndef WIN32 > - if (!pfx_len || is_absolute_path(arg)) > - return arg; > - memcpy(path, pfx, pfx_len); > - strcpy(path + pfx_len, arg); > -#else > - char *p; > - /* don't add prefix to absolute paths, but still replace '\' by '/' */ > - if (is_absolute_path(arg)) > - pfx_len = 0; > - else if (pfx_len) > - memcpy(path, pfx, pfx_len); > - strcpy(path + pfx_len, arg); > - for (p = path + pfx_len; *p; p++) > - if (*p == '\\') > - *p = '/'; > -#endif > - return path; > -} > - > int check_filename(const char *prefix, const char *arg) > { > const char *name; > -- > 1.7.3.4 > Except for the commit message, and with or without the #include tweak mentioned above, Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Thanks. This seems to be in pretty good shape (and as mentioned before, gets closer to fulfillment of a longstanding wish :)). ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] Reduce parse-options.o dependencies 2011-08-11 11:04 ` Jonathan Nieder @ 2011-08-11 23:35 ` Junio C Hamano 0 siblings, 0 replies; 6+ messages in thread From: Junio C Hamano @ 2011-08-11 23:35 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Dmitry Ivankov, git, David Barr, Stephen Boyd Jonathan Nieder <jrnieder@gmail.com> writes: >> diff --git a/parse-options-cb.c b/parse-options-cb.c >> new file mode 100644 >> index 0000000..c248f66 >> --- /dev/null >> +++ b/parse-options-cb.c >> @@ -0,0 +1,125 @@ >> +#include "git-compat-util.h" >> +#include "parse-options.h" >> +#include "cache.h" > > Style: Files in git tend to use only one of "git-compat-util.h", > "cache.h", or "builtin.h" and put it at the top. So in this case, it > should probably use just "cache.h". This needs a bit of clarification. The compatibility rule actually is to have git-compat-util.h at the very beginning. As cache.h is a very widely used header almost everybody that needs to access the internals include, it includes git-compat-util.h as its first thing to include. So if this is an old file that existed before git-compat-util.h, then it is perfectly fine to start it with #include "cache.h" #include "parse-options.h" Otherwise the three-line include above is also just fine. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-08-11 23:35 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-08-11 9:15 [PATCH v3 0/2] Reduce parse-options.o dependencies Dmitry Ivankov 2011-08-11 9:15 ` [PATCH v3 1/2] parse-options: export opterr, optbug Dmitry Ivankov 2011-08-11 10:42 ` Jonathan Nieder 2011-08-11 9:15 ` [PATCH v3 2/2] Reduce parse-options.o dependencies Dmitry Ivankov 2011-08-11 11:04 ` Jonathan Nieder 2011-08-11 23:35 ` Junio C Hamano
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.