Re: [PATCH] for-each-ref: add option to omit newlines
Junio C Hamano gitster at pobox.com writes: I would rather see us go in the direction to add -z output option, which is what everybody else that produces NUL terminated entries in our suite of subcommands does. I agree that -z would help in this case and I very much appreciate that option when using diff --name-only, ls-files, etc. However, when specifying a format string it's just a matter of ending the format string in '%00' and you're good to go. But then you get the null byte *and* a newline. And with your proposal there would be no way of saying you want neither. I expected the output to be formatted according to a (repetition of) the format string, not some variation of it that I couldn't opt out of. But I see that git-log also shows this behavior and already has a -z option, so I guess that's fairly ingrained. Maybe it's just me? In that case I've no problem with throwing in the towel. Øsse -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] for-each-ref: add option to omit newlines
Øystein Walle oys...@gmail.com writes: However, when specifying a format string it's just a matter of ending the format string in '%00' and you're good to go. But then you get the null byte *and* a newline. And with your proposal there would be no way of saying you want neither. I very well understand that. All other commands that support -z to give you NUL terminated output do not consider that a downside. Why should for-each-ref be special? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] for-each-ref: add option to omit newlines
Junio C Hamano gitster at pobox.com writes: I very well understand that. All other commands that support -z to give you NUL terminated output do not consider that a downside. Why should for-each-ref be special? After I discovered log also has this there is nothing special about for-each-ref any longer, so my patch as-is would only make things less consistent. What is special is that they give you the option of supplying a format string. ls-files, diff and others print a specific list of items (paths, shas, ...) and there's no question about how they are presented other than the delimiter between each item, to which a selection of either a newline or a null byte is plenty. With log, for-each-ref and rev-list (any others?) that sort of breaks down. With the format string you're given the power to make the command print basically anything you like, however you like; no longer only a question of mere delimiters. It only makes sense then (to me, at least) that the command does not meddle with the format the user has chosen. Maybe it's all subjective... I'm okay with just leaving things as they are. There are ways around it. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] for-each-ref: add option to omit newlines
Øystein Walle oys...@gmail.com writes:
Maybe it's all subjective... I'm okay with just leaving things as they
are.
Lack of -z in for-each-ref can be called an inconsistency that
already exists you may want to fix in any case.
As an extension to that, I would not be fundamentally against a new
option, e.g. --terminiator=7, that causes us to use putchar(7)
instead of putchar('\n') or putchar('\0') to terminate each records.
At that point, -z becomes a synonym for --terminator=0.
And --terminator='' might even be a natural extension to that
option to cause us not to call any putchar() there.
If we were to do that, we should do them for all commands that let
you use -z, not just for-each-ref, for consistency reasons, I
would think.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] for-each-ref: add option to omit newlines
Even when having specified a format string for-each-ref still prints a
newline after printing each ref according to the format. This breaks
splitting the output on newlines, for example.
Signed-off-by: Øystein Walle oys...@gmail.com
---
I was somewhat surprised by this behaviour; I expected to be in full control of
the output but I could not get rid of the newlines.
As far as I'm *personally* concerned the line putchar('\n'); could just as
well be removed completely and an '\n' appended to the default format string.
But since this command (and this behaviour) as been around since 2006 I assume
that it's in Git's best interest to not change the default behaviour. Hence the
introduction of this option. Although I was taken aback by this behaviour, is
it patch-worthy? The fix isn't very pretty.
On to the patch itself: I contemplated putting '\n' in the default format and
removing it if -n was given, which would get rid of the need to pass an exta
argument to show_ref(). But that means we would need to *insert it* when a
format is given and -n is not...
I've changed how the default format is assigned so that we can check if it's
NULL after option parsing to spit out an error. Alternatively we could just
allow it.
Documentation/git-for-each-ref.txt | 3 +++
builtin/for-each-ref.c | 20 +++-
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index 4240875..805914a 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -62,6 +62,9 @@ OPTIONS
the specified host language. This is meant to produce
a scriptlet that can directly be `eval`ed.
+-n::
+--no-newlines::
+ Do not print a newline after printing each formatted ref.
FIELD NAMES
---
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 51798b4..32d6b12 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -998,7 +998,8 @@ static void emit(const char *cp, const char *ep)
}
}
-static void show_ref(struct refinfo *info, const char *format, int quote_style)
+static void show_ref(struct refinfo *info, const char *format, int quote_style,
+int no_newlines)
{
const char *cp, *sp, *ep;
@@ -1023,7 +1024,8 @@ static void show_ref(struct refinfo *info, const char
*format, int quote_style)
resetv.s = color;
print_value(resetv, quote_style);
}
- putchar('\n');
+ if (!no_newlines)
+ putchar('\n');
}
static struct ref_sort *default_sort(void)
@@ -1067,9 +1069,9 @@ static char const * const for_each_ref_usage[] = {
int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
{
int i, num_refs;
- const char *format = %(objectname) %(objecttype)\t%(refname);
+ const char *format = NULL;
struct ref_sort *sort = NULL, **sort_tail = sort;
- int maxcount = 0, quote_style = 0;
+ int maxcount = 0, quote_style = 0, no_newlines = 0;
struct refinfo **refs;
struct grab_ref_cbdata cbdata;
@@ -1082,6 +1084,8 @@ int cmd_for_each_ref(int argc, const char **argv, const
char *prefix)
N_(quote placeholders suitably for python),
QUOTE_PYTHON),
OPT_BIT(0 , tcl, quote_style,
N_(quote placeholders suitably for tcl), QUOTE_TCL),
+ OPT_BOOL('n' , no-newlines, no_newlines,
+ N_(do not output a newline after each ref)),
OPT_GROUP(),
OPT_INTEGER( 0 , count, maxcount, N_(show only n matched
refs)),
@@ -1100,6 +1104,12 @@ int cmd_for_each_ref(int argc, const char **argv, const
char *prefix)
error(more than one quoting style?);
usage_with_options(for_each_ref_usage, opts);
}
+ if (no_newlines !format) {
+ error(--no-newlines without --format does not make sense);
+ usage_with_options(for_each_ref_usage, opts);
+ }
+ if (!format)
+ format = %(objectname) %(objecttype)\t%(refname);
if (verify_format(format))
usage_with_options(for_each_ref_usage, opts);
@@ -1120,6 +1130,6 @@ int cmd_for_each_ref(int argc, const char **argv, const
char *prefix)
if (!maxcount || num_refs maxcount)
maxcount = num_refs;
for (i = 0; i maxcount; i++)
- show_ref(refs[i], format, quote_style);
+ show_ref(refs[i], format, quote_style, no_newlines);
return 0;
}
--
1.8.5
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] for-each-ref: add option to omit newlines
Øystein Walle oystwa at gmail.com writes: splitting the output on newlines, for example. Ugh, I mean on null bytes, naturally. Øsse. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] for-each-ref: add option to omit newlines
Øystein Walle oys...@gmail.com writes:
On to the patch itself: I contemplated putting '\n' in the default format and
removing it if -n was given, which would get rid of the need to pass an exta
argument to show_ref(). But that means we would need to *insert it* when a
format is given and -n is not...
I would rather see us go in the direction to add -z output option,
which is what everybody else that produces NUL terminated entries in
our suite of subcommands does.
IOW, something along with this line (untested).
builtin/for-each-ref.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 51798b4..2c8cac8 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -94,6 +94,7 @@ static const char **used_atom;
static cmp_type *used_atom_type;
static int used_atom_cnt, need_tagged, need_symref;
static int need_color_reset_at_eol;
+static int line_termination = '\n';
/*
* Used to parse format string and sort specifiers
@@ -1023,7 +1024,7 @@ static void show_ref(struct refinfo *info, const char
*format, int quote_style)
resetv.s = color;
print_value(resetv, quote_style);
}
- putchar('\n');
+ putchar(line_termination);
}
static struct ref_sort *default_sort(void)
@@ -1088,6 +1089,9 @@ int cmd_for_each_ref(int argc, const char **argv, const
char *prefix)
OPT_STRING( 0 , format, format, N_(format), N_(format to
use for the output)),
OPT_CALLBACK(0 , sort, sort_tail, N_(key),
N_(field name to sort on), opt_parse_sort),
+ OPT_SET_INT('z', NULL, line_termination,
+ N_(Use NUL instead of LF to end each output
records),
+ '\0'),
OPT_END(),
};
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
