* [PATCH v9 1/4] usage: make error functions a stack
@ 2014-07-15 23:32 Jacob Keller
2014-07-15 23:32 ` [PATCH v9 2/4] tag: fix --sort tests to use cat<<-\EOF format Jacob Keller
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Jacob Keller @ 2014-07-15 23:32 UTC (permalink / raw)
To: git; +Cc: Jacob Keller
Rename set_error_routine to be push_error_routine, and add a new
pop_error_routine. This allows temporary modifications of the error
routine over a small block of code.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Renamed set_error_routine to push_error_routine in order to match the
pop_error_routine.
git-compat-util.h | 3 ++-
run-command.c | 2 +-
usage.c | 32 ++++++++++++++++++++++++++++----
3 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index 9de318071083..b650e3cb6cdc 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -343,7 +343,8 @@ static inline int const_error(void)
#endif
extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
-extern void set_error_routine(void (*routine)(const char *err, va_list params));
+extern void push_error_routine(void (*routine)(const char *err, va_list params));
+extern void pop_error_routine(void);
extern void set_die_is_recursing_routine(int (*routine)(void));
extern int starts_with(const char *str, const char *prefix);
diff --git a/run-command.c b/run-command.c
index be07d4ad335b..a611b819c25d 100644
--- a/run-command.c
+++ b/run-command.c
@@ -360,7 +360,7 @@ fail_pipe:
set_cloexec(child_err);
}
set_die_routine(die_child);
- set_error_routine(error_child);
+ push_error_routine(error_child);
close(notify_pipe[0]);
set_cloexec(notify_pipe[1]);
diff --git a/usage.c b/usage.c
index ed146453cabe..3fe26771f7e6 100644
--- a/usage.c
+++ b/usage.c
@@ -57,18 +57,42 @@ static int die_is_recursing_builtin(void)
* (ugh), so keep things static. */
static NORETURN_PTR void (*usage_routine)(const char *err, va_list params) = usage_builtin;
static NORETURN_PTR void (*die_routine)(const char *err, va_list params) = die_builtin;
-static void (*error_routine)(const char *err, va_list params) = error_builtin;
static void (*warn_routine)(const char *err, va_list params) = warn_builtin;
static int (*die_is_recursing)(void) = die_is_recursing_builtin;
+struct error_func_list {
+ void (*func)(const char *, va_list);
+ struct error_func_list *next;
+};
+static struct error_func_list default_error_func = { error_builtin };
+static struct error_func_list *error_funcs = &default_error_func;
+
void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params))
{
die_routine = routine;
}
-void set_error_routine(void (*routine)(const char *err, va_list params))
+/* push error routine onto the error function stack */
+void push_error_routine(void (*routine)(const char *err, va_list params))
{
- error_routine = routine;
+ struct error_func_list *efl = xmalloc(sizeof(*efl));
+ efl->func = routine;
+ efl->next = error_funcs;
+ error_funcs = efl;
+}
+
+/* pop a single error routine off of the error function stack, thus reverting
+ * to previous error. Should always be paired with a push_error_routine */
+void pop_error_routine(void)
+{
+ struct error_func_list *efl = error_funcs;
+
+ assert(error_funcs != &default_error_func);
+
+ if (efl->next) {
+ error_funcs = efl->next;
+ free(efl);
+ }
}
void set_die_is_recursing_routine(int (*routine)(void))
@@ -144,7 +168,7 @@ int error(const char *err, ...)
va_list params;
va_start(params, err);
- error_routine(err, params);
+ error_funcs->func(err, params);
va_end(params);
return -1;
}
--
2.0.1.475.g9b8d714
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v9 2/4] tag: fix --sort tests to use cat<<-\EOF format
2014-07-15 23:32 [PATCH v9 1/4] usage: make error functions a stack Jacob Keller
@ 2014-07-15 23:32 ` Jacob Keller
2014-07-15 23:32 ` [PATCH v9 3/4] tag: update parsing to be more precise regarding errors Jacob Keller
2014-07-15 23:32 ` [PATCH v9 4/4] tag: support configuring --sort via .gitconfig Jacob Keller
2 siblings, 0 replies; 11+ messages in thread
From: Jacob Keller @ 2014-07-15 23:32 UTC (permalink / raw)
To: git; +Cc: Jacob Keller
The --sort tests should use the better format for >expect to maintain
indenting and ensure that no substitution is occurring. This makes
parsing and understanding the tests a bit easier.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
t/t7004-tag.sh | 40 ++++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index e4ab0f5b6419..66010b0e7066 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1385,41 +1385,41 @@ test_expect_success 'lexical sort' '
git tag foo1.6 &&
git tag foo1.10 &&
git tag -l --sort=refname "foo*" >actual &&
- cat >expect <<EOF &&
-foo1.10
-foo1.3
-foo1.6
-EOF
+ cat >expect <<-\EOF &&
+ foo1.10
+ foo1.3
+ foo1.6
+ EOF
test_cmp expect actual
'
test_expect_success 'version sort' '
git tag -l --sort=version:refname "foo*" >actual &&
- cat >expect <<EOF &&
-foo1.3
-foo1.6
-foo1.10
-EOF
+ cat >expect <<-\EOF &&
+ foo1.3
+ foo1.6
+ foo1.10
+ EOF
test_cmp expect actual
'
test_expect_success 'reverse version sort' '
git tag -l --sort=-version:refname "foo*" >actual &&
- cat >expect <<EOF &&
-foo1.10
-foo1.6
-foo1.3
-EOF
+ cat >expect <<-\EOF &&
+ foo1.10
+ foo1.6
+ foo1.3
+ EOF
test_cmp expect actual
'
test_expect_success 'reverse lexical sort' '
git tag -l --sort=-refname "foo*" >actual &&
- cat >expect <<EOF &&
-foo1.6
-foo1.3
-foo1.10
-EOF
+ cat >expect <<-\EOF &&
+ foo1.6
+ foo1.3
+ foo1.10
+ EOF
test_cmp expect actual
'
--
2.0.1.475.g9b8d714
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v9 3/4] tag: update parsing to be more precise regarding errors
2014-07-15 23:32 [PATCH v9 1/4] usage: make error functions a stack Jacob Keller
2014-07-15 23:32 ` [PATCH v9 2/4] tag: fix --sort tests to use cat<<-\EOF format Jacob Keller
@ 2014-07-15 23:32 ` Jacob Keller
2014-07-15 23:32 ` [PATCH v9 4/4] tag: support configuring --sort via .gitconfig Jacob Keller
2 siblings, 0 replies; 11+ messages in thread
From: Jacob Keller @ 2014-07-15 23:32 UTC (permalink / raw)
To: git; +Cc: Jacob Keller, Jeff King
Update the parsing of sort string specifications so that it is able to
properly detect errors in the function type and allowed atoms.
Cc: Jeff King <peff@peff.net>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
builtin/tag.c | 55 +++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 41 insertions(+), 14 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index ef765563388c..7d82526e76be 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -522,24 +522,51 @@ static int parse_opt_points_at(const struct option *opt __attribute__((unused)),
static int parse_opt_sort(const struct option *opt, const char *arg, int unset)
{
int *sort = opt->value;
- int flags = 0;
+ char *value, *separator, *type, *atom;
+ int flags = 0, function = 0, err = 0;
- if (*arg == '-') {
+ /* skip the '-' prefix for reverse sort order first */
+ if (skip_prefix(arg, "-", &arg))
flags |= REVERSE_SORT;
- arg++;
+
+ /* duplicate string so we can modify it in place */
+ value = xstrdup(arg);
+
+ /* determine the sort function and the sorting atom */
+ separator = strchr(value, ':');
+ if (separator) {
+ /* split the string at the separator with a NULL byte */
+ *separator = '\0';
+ type = value;
+ atom = separator + 1;
+ } else {
+ /* we have no separator, so assume the whole string is the * atom */
+ type = NULL;
+ atom = value;
}
- if (starts_with(arg, "version:")) {
- *sort = VERCMP_SORT;
- arg += 8;
- } else if (starts_with(arg, "v:")) {
- *sort = VERCMP_SORT;
- arg += 2;
+
+ if (type) {
+ if (!strcmp(type, "version") || !strcmp(type, "v"))
+ function = VERCMP_SORT;
+ else {
+ err = error(_("unsupported sort function '%s'"), type);
+ goto abort;
+ }
+
} else
- *sort = STRCMP_SORT;
- if (strcmp(arg, "refname"))
- die(_("unsupported sort specification %s"), arg);
- *sort |= flags;
- return 0;
+ function = STRCMP_SORT;
+
+ /* for now, only the refname is a valid atom */
+ if (atom && strcmp(atom, "refname")) {
+ err = error(_("unsupported sort specification '%s'"), atom);
+ goto abort;
+ }
+
+ *sort = (flags | function);
+
+abort:
+ free(value);
+ return err;
}
int cmd_tag(int argc, const char **argv, const char *prefix)
--
2.0.1.475.g9b8d714
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v9 4/4] tag: support configuring --sort via .gitconfig
2014-07-15 23:32 [PATCH v9 1/4] usage: make error functions a stack Jacob Keller
2014-07-15 23:32 ` [PATCH v9 2/4] tag: fix --sort tests to use cat<<-\EOF format Jacob Keller
2014-07-15 23:32 ` [PATCH v9 3/4] tag: update parsing to be more precise regarding errors Jacob Keller
@ 2014-07-15 23:32 ` Jacob Keller
2014-07-15 23:42 ` Jeff King
2 siblings, 1 reply; 11+ messages in thread
From: Jacob Keller @ 2014-07-15 23:32 UTC (permalink / raw)
To: git; +Cc: Jacob Keller, Jeff King
Add support for configuring default sort specification for git tags.
Command line option will override the configured value. Both methods use
the same syntax. Make use of (set/pop)_error_routine to temporarily
modify error reporting when parsing as a configuration option.
Cc: Jeff King <peff@peff.net>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Documentation/config.txt | 5 ++
Documentation/git-tag.txt | 5 +-
builtin/tag.c | 124 ++++++++++++++++++++++++++++------------------
t/t7004-tag.sh | 36 ++++++++++++++
4 files changed, 120 insertions(+), 50 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1d718bdb9662..c55c22ab7be9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2354,6 +2354,11 @@ submodule.<name>.ignore::
"--ignore-submodules" option. The 'git submodule' commands are not
affected by this setting.
+tag.sort::
+ This variable controls the sort ordering of tags when displayed by
+ linkgit:git-tag[1]. Without the "--sort=<value>" option provided, the
+ value of this variable will be used as the default.
+
tar.umask::
This variable can be used to restrict the permission bits of
tar archive entries. The default is 0002, which turns off the
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index b424a1bc48bb..320908369f06 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -99,7 +99,9 @@ OPTIONS
Sort in a specific order. Supported type is "refname"
(lexicographic order), "version:refname" or "v:refname" (tag
names are treated as versions). Prepend "-" to reverse sort
- order.
+ order. When this option is not given, the sort order defaults to the
+ value configured for the 'tag.sort' variable if it exists, or
+ lexicographic order otherwise. See linkgit:git-config[1].
--column[=<options>]::
--no-column::
@@ -317,6 +319,7 @@ include::date-formats.txt[]
SEE ALSO
--------
linkgit:git-check-ref-format[1].
+linkgit:git-config[1].
GIT
---
diff --git a/builtin/tag.c b/builtin/tag.c
index 7d82526e76be..091536c61eab 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -32,6 +32,8 @@ static const char * const git_tag_usage[] = {
#define SORT_MASK 0x7fff
#define REVERSE_SORT 0x8000
+static int tag_sort;
+
struct tag_filter {
const char **patterns;
int lines;
@@ -346,9 +348,76 @@ static const char tag_template_nocleanup[] =
"Lines starting with '%c' will be kept; you may remove them"
" yourself if you want to.\n");
+static int parse_sort_string(const char *arg, int *sort)
+{
+ char *value, *separator, *type, *atom;
+ int flags = 0, function = 0, err = 0;
+
+ /* skip the '-' prefix for reverse sort order first */
+ if (skip_prefix(arg, "-", &arg))
+ flags |= REVERSE_SORT;
+
+ /* duplicate string so we can modify it in place */
+ value = xstrdup(arg);
+
+ /* determine the sort function and the sorting atom */
+ separator = strchr(value, ':');
+ if (separator) {
+ /* split the string at the separator with a NULL byte */
+ *separator = '\0';
+ type = value;
+ atom = separator + 1;
+ } else {
+ /* we have no separator, so assume the whole string is the * atom */
+ type = NULL;
+ atom = value;
+ }
+
+ if (type) {
+ if (!strcmp(type, "version") || !strcmp(type, "v"))
+ function = VERCMP_SORT;
+ else {
+ err = error(_("unsupported sort function '%s'"), type);
+ goto abort;
+ }
+
+ } else
+ function = STRCMP_SORT;
+
+ /* for now, only the refname is a valid atom */
+ if (atom && strcmp(atom, "refname")) {
+ err = error(_("unsupported sort specification '%s'"), atom);
+ goto abort;
+ }
+
+ *sort = (flags | function);
+
+abort:
+ free(value);
+ return err;
+}
+
+static void error_bad_sort_config(const char *err, va_list params)
+{
+ vreportf("warning: tag.sort has ", err, params);
+}
+
static int git_tag_config(const char *var, const char *value, void *cb)
{
- int status = git_gpg_config(var, value, cb);
+ int status;
+
+ if (!strcmp(var, "tag.sort")) {
+ if (!value)
+ return config_error_nonbool(var);
+
+ push_error_routine(error_bad_sort_config);
+ parse_sort_string(value, &tag_sort);
+ pop_error_routine();
+
+ return 0;
+ }
+
+ status = git_gpg_config(var, value, cb);
if (status)
return status;
if (starts_with(var, "column."))
@@ -522,51 +591,8 @@ static int parse_opt_points_at(const struct option *opt __attribute__((unused)),
static int parse_opt_sort(const struct option *opt, const char *arg, int unset)
{
int *sort = opt->value;
- char *value, *separator, *type, *atom;
- int flags = 0, function = 0, err = 0;
- /* skip the '-' prefix for reverse sort order first */
- if (skip_prefix(arg, "-", &arg))
- flags |= REVERSE_SORT;
-
- /* duplicate string so we can modify it in place */
- value = xstrdup(arg);
-
- /* determine the sort function and the sorting atom */
- separator = strchr(value, ':');
- if (separator) {
- /* split the string at the separator with a NULL byte */
- *separator = '\0';
- type = value;
- atom = separator + 1;
- } else {
- /* we have no separator, so assume the whole string is the * atom */
- type = NULL;
- atom = value;
- }
-
- if (type) {
- if (!strcmp(type, "version") || !strcmp(type, "v"))
- function = VERCMP_SORT;
- else {
- err = error(_("unsupported sort function '%s'"), type);
- goto abort;
- }
-
- } else
- function = STRCMP_SORT;
-
- /* for now, only the refname is a valid atom */
- if (atom && strcmp(atom, "refname")) {
- err = error(_("unsupported sort specification '%s'"), atom);
- goto abort;
- }
-
- *sort = (flags | function);
-
-abort:
- free(value);
- return err;
+ return parse_sort_string(arg, sort);
}
int cmd_tag(int argc, const char **argv, const char *prefix)
@@ -579,7 +605,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct create_tag_options opt;
char *cleanup_arg = NULL;
int annotate = 0, force = 0, lines = -1;
- int cmdmode = 0, sort = 0;
+ int cmdmode = 0;
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
@@ -605,7 +631,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
OPT__FORCE(&force, N_("replace the tag if exists")),
OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
{
- OPTION_CALLBACK, 0, "sort", &sort, N_("type"), N_("sort tags"),
+ OPTION_CALLBACK, 0, "sort", &tag_sort, N_("type"), N_("sort tags"),
PARSE_OPT_NONEG, parse_opt_sort
},
@@ -661,9 +687,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
copts.padding = 2;
run_column_filter(colopts, &copts);
}
- if (lines != -1 && sort)
+ if (lines != -1 && tag_sort)
die(_("--sort and -n are incompatible"));
- ret = list_tags(argv, lines == -1 ? 0 : lines, with_commit, sort);
+ ret = list_tags(argv, lines == -1 ? 0 : lines, with_commit, tag_sort);
if (column_active(colopts))
stop_column_filter();
return ret;
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 66010b0e7066..036665308841 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1423,6 +1423,42 @@ test_expect_success 'reverse lexical sort' '
test_cmp expect actual
'
+test_expect_success 'configured lexical sort' '
+ git config tag.sort "v:refname" &&
+ git tag -l "foo*" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.3
+ foo1.6
+ foo1.10
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'option override configured sort' '
+ git tag -l --sort=-refname "foo*" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.6
+ foo1.3
+ foo1.10
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'invalid sort parameter on command line' '
+ test_must_fail git tag -l --sort=notvalid "foo*" >actual
+'
+
+test_expect_success 'invalid sort parameter in configuratoin' '
+ git config tag.sort "v:notvalid" &&
+ git tag -l "foo*" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.10
+ foo1.3
+ foo1.6
+ EOF
+ test_cmp expect actual
+'
+
run_with_limited_stack () {
(ulimit -s 64 && "$@")
}
--
2.0.1.475.g9b8d714
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v9 4/4] tag: support configuring --sort via .gitconfig
2014-07-15 23:32 ` [PATCH v9 4/4] tag: support configuring --sort via .gitconfig Jacob Keller
@ 2014-07-15 23:42 ` Jeff King
2014-07-15 23:46 ` Keller, Jacob E
0 siblings, 1 reply; 11+ messages in thread
From: Jeff King @ 2014-07-15 23:42 UTC (permalink / raw)
To: Jacob Keller; +Cc: git
On Tue, Jul 15, 2014 at 04:32:59PM -0700, Jacob Keller wrote:
> +static void error_bad_sort_config(const char *err, va_list params)
> +{
> + vreportf("warning: tag.sort has ", err, params);
> +}
This feels a little like an abuse of the "prefix" field of vreportf, but
as you probably saw in my "for fun" patch, doing it right means
formatting into a buffer and then reformatting that (which we're
already doing again in vreportf, but less flexibly). I dunno.
At any rate, this should be marked for translation, no?
-Peff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v9 4/4] tag: support configuring --sort via .gitconfig
2014-07-15 23:42 ` Jeff King
@ 2014-07-15 23:46 ` Keller, Jacob E
2014-07-16 17:59 ` Junio C Hamano
0 siblings, 1 reply; 11+ messages in thread
From: Keller, Jacob E @ 2014-07-15 23:46 UTC (permalink / raw)
To: peff@peff.net; +Cc: git@vger.kernel.org
On Tue, 2014-07-15 at 19:42 -0400, Jeff King wrote:
> On Tue, Jul 15, 2014 at 04:32:59PM -0700, Jacob Keller wrote:
>
> > +static void error_bad_sort_config(const char *err, va_list params)
> > +{
> > + vreportf("warning: tag.sort has ", err, params);
> > +}
>
> This feels a little like an abuse of the "prefix" field of vreportf, but
> as you probably saw in my "for fun" patch, doing it right means
> formatting into a buffer and then reformatting that (which we're
> already doing again in vreportf, but less flexibly). I dunno.
>
> At any rate, this should be marked for translation, no?
>
> -Peff
Oh, yes it probably should. It's definitely a little bit abusive of the
prefix field, but that seemed easier.
Thanks,
Jake
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v9 4/4] tag: support configuring --sort via .gitconfig
2014-07-15 23:46 ` Keller, Jacob E
@ 2014-07-16 17:59 ` Junio C Hamano
2014-07-16 19:51 ` Keller, Jacob E
2014-07-16 21:13 ` Keller, Jacob E
0 siblings, 2 replies; 11+ messages in thread
From: Junio C Hamano @ 2014-07-16 17:59 UTC (permalink / raw)
To: Keller, Jacob E; +Cc: peff@peff.net, git@vger.kernel.org
"Keller, Jacob E" <jacob.e.keller@intel.com> writes:
> On Tue, 2014-07-15 at 19:42 -0400, Jeff King wrote:
>> On Tue, Jul 15, 2014 at 04:32:59PM -0700, Jacob Keller wrote:
>>
>> > +static void error_bad_sort_config(const char *err, va_list params)
>> > +{
>> > + vreportf("warning: tag.sort has ", err, params);
>> > +}
>>
>> This feels a little like an abuse of the "prefix" field of vreportf, but
>> as you probably saw in my "for fun" patch, doing it right means
>> formatting into a buffer and then reformatting that (which we're
>> already doing again in vreportf, but less flexibly). I dunno.
>>
>> At any rate, this should be marked for translation, no?
>>
>> -Peff
>
> Oh, yes it probably should. It's definitely a little bit abusive of the
> prefix field, but that seemed easier.
And i18n would automatically mean this will not work, no? There is
no guarantee that the translation of "warning: " will be followed
directly by the translation of "tag.sort has" without any words from
variable part in all languages.
After all, it seems to me that the one in
http://thread.gmane.org/gmane.comp.version-control.git/253346
struck the right balance among various abuses; let's use the error
reporter from that version, instead of going down this rabbit hole.
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v9 4/4] tag: support configuring --sort via .gitconfig
2014-07-16 17:59 ` Junio C Hamano
@ 2014-07-16 19:51 ` Keller, Jacob E
2014-07-16 21:13 ` Keller, Jacob E
1 sibling, 0 replies; 11+ messages in thread
From: Keller, Jacob E @ 2014-07-16 19:51 UTC (permalink / raw)
To: gitster@pobox.com; +Cc: git@vger.kernel.org, peff@peff.net
On Wed, 2014-07-16 at 10:59 -0700, Junio C Hamano wrote:
> "Keller, Jacob E" <jacob.e.keller@intel.com> writes:
>
> > On Tue, 2014-07-15 at 19:42 -0400, Jeff King wrote:
> >> On Tue, Jul 15, 2014 at 04:32:59PM -0700, Jacob Keller wrote:
> >>
> >> > +static void error_bad_sort_config(const char *err, va_list params)
> >> > +{
> >> > + vreportf("warning: tag.sort has ", err, params);
> >> > +}
> >>
> >> This feels a little like an abuse of the "prefix" field of vreportf, but
> >> as you probably saw in my "for fun" patch, doing it right means
> >> formatting into a buffer and then reformatting that (which we're
> >> already doing again in vreportf, but less flexibly). I dunno.
> >>
> >> At any rate, this should be marked for translation, no?
> >>
> >> -Peff
> >
> > Oh, yes it probably should. It's definitely a little bit abusive of the
> > prefix field, but that seemed easier.
>
> And i18n would automatically mean this will not work, no? There is
> no guarantee that the translation of "warning: " will be followed
> directly by the translation of "tag.sort has" without any words from
> variable part in all languages.
>
> After all, it seems to me that the one in
>
> http://thread.gmane.org/gmane.comp.version-control.git/253346
>
> struck the right balance among various abuses; let's use the error
> reporter from that version, instead of going down this rabbit hole.
>
> Thanks.
>
>
>
This is fine with me :)
Thanks,
Jake
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v9 4/4] tag: support configuring --sort via .gitconfig
2014-07-16 17:59 ` Junio C Hamano
2014-07-16 19:51 ` Keller, Jacob E
@ 2014-07-16 21:13 ` Keller, Jacob E
2014-07-16 21:40 ` Junio C Hamano
1 sibling, 1 reply; 11+ messages in thread
From: Keller, Jacob E @ 2014-07-16 21:13 UTC (permalink / raw)
To: gitster@pobox.com; +Cc: git@vger.kernel.org, peff@peff.net
On Wed, 2014-07-16 at 10:59 -0700, Junio C Hamano wrote:
> "Keller, Jacob E" <jacob.e.keller@intel.com> writes:
>
> > On Tue, 2014-07-15 at 19:42 -0400, Jeff King wrote:
> >> On Tue, Jul 15, 2014 at 04:32:59PM -0700, Jacob Keller wrote:
> >>
> >> > +static void error_bad_sort_config(const char *err, va_list params)
> >> > +{
> >> > + vreportf("warning: tag.sort has ", err, params);
> >> > +}
> >>
> >> This feels a little like an abuse of the "prefix" field of vreportf, but
> >> as you probably saw in my "for fun" patch, doing it right means
> >> formatting into a buffer and then reformatting that (which we're
> >> already doing again in vreportf, but less flexibly). I dunno.
> >>
> >> At any rate, this should be marked for translation, no?
> >>
> >> -Peff
> >
> > Oh, yes it probably should. It's definitely a little bit abusive of the
> > prefix field, but that seemed easier.
>
> And i18n would automatically mean this will not work, no? There is
> no guarantee that the translation of "warning: " will be followed
> directly by the translation of "tag.sort has" without any words from
> variable part in all languages.
>
> After all, it seems to me that the one in
>
> http://thread.gmane.org/gmane.comp.version-control.git/253346
>
> struck the right balance among various abuses; let's use the error
> reporter from that version, instead of going down this rabbit hole.
>
> Thanks.
>
>
>
So is that patch series version acceptable then? Or should I spin one
again that is in that vein?
Thanks,
Jake
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v9 4/4] tag: support configuring --sort via .gitconfig
2014-07-16 21:13 ` Keller, Jacob E
@ 2014-07-16 21:40 ` Junio C Hamano
2014-07-16 21:42 ` Keller, Jacob E
0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2014-07-16 21:40 UTC (permalink / raw)
To: Keller, Jacob E; +Cc: git@vger.kernel.org, peff@peff.net
"Keller, Jacob E" <jacob.e.keller@intel.com> writes:
>> After all, it seems to me that the one in
>>
>> http://thread.gmane.org/gmane.comp.version-control.git/253346
>>
>> struck the right balance among various abuses; let's use the error
>> reporter from that version, instead of going down this rabbit hole.
>>
>> Thanks.
>
> So is that patch series version acceptable then? Or should I spin one
> again that is in that vein?
I do not offhand know what other changes you had in v9 since
$gmane/253346, so I'll leave it up to you. If the only difference
is the error reporting codepath, and you are happy with what I have
in my tree
$ git log -p --reverse -3 a93d886b9
then we can proceed with that version. Have there been any issues
raised on that older version other than error reporting?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v9 4/4] tag: support configuring --sort via .gitconfig
2014-07-16 21:40 ` Junio C Hamano
@ 2014-07-16 21:42 ` Keller, Jacob E
0 siblings, 0 replies; 11+ messages in thread
From: Keller, Jacob E @ 2014-07-16 21:42 UTC (permalink / raw)
To: gitster@pobox.com; +Cc: git@vger.kernel.org, peff@peff.net
On Wed, 2014-07-16 at 14:40 -0700, Junio C Hamano wrote:
> "Keller, Jacob E" <jacob.e.keller@intel.com> writes:
>
> >> After all, it seems to me that the one in
> >>
> >> http://thread.gmane.org/gmane.comp.version-control.git/253346
> >>
> >> struck the right balance among various abuses; let's use the error
> >> reporter from that version, instead of going down this rabbit hole.
> >>
> >> Thanks.
> >
> > So is that patch series version acceptable then? Or should I spin one
> > again that is in that vein?
>
> I do not offhand know what other changes you had in v9 since
> $gmane/253346, so I'll leave it up to you. If the only difference
> is the error reporting codepath, and you are happy with what I have
> in my tree
>
> $ git log -p --reverse -3 a93d886b9
>
> then we can proceed with that version. Have there been any issues
> raised on that older version other than error reporting?
>
>
>
>
I'll double check.
Thanks,
Jake
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-07-16 21:42 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-15 23:32 [PATCH v9 1/4] usage: make error functions a stack Jacob Keller
2014-07-15 23:32 ` [PATCH v9 2/4] tag: fix --sort tests to use cat<<-\EOF format Jacob Keller
2014-07-15 23:32 ` [PATCH v9 3/4] tag: update parsing to be more precise regarding errors Jacob Keller
2014-07-15 23:32 ` [PATCH v9 4/4] tag: support configuring --sort via .gitconfig Jacob Keller
2014-07-15 23:42 ` Jeff King
2014-07-15 23:46 ` Keller, Jacob E
2014-07-16 17:59 ` Junio C Hamano
2014-07-16 19:51 ` Keller, Jacob E
2014-07-16 21:13 ` Keller, Jacob E
2014-07-16 21:40 ` Junio C Hamano
2014-07-16 21:42 ` Keller, Jacob E
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).