Re: [PATCH 2/4] ref-filter: add empty values to technical fields
2018-07-10 1:39 GMT+03:00 Junio C Hamano :
> Olga Telezhnaya writes:
>
>> Atoms like "align" or "end" do not have string representation.
>> Earlier we had to go and parse whole object with a hope that we
>> could fill their string representations. It's easier to fill them
>> with an empty string before we start to work with whole object.
>>
>> Signed-off-by: Olga Telezhnaia
>> ---
>
> Just being curious, but is there any meaningful relationship between
> what was labelled as SOURCE_NONE in the previous step and what this
> step calls "technical fields"? Things like "upstream" (which is not
> affected by the contents of the object, but is affected by the ref
> in question) and "if" (which merely exists to construct the language
> syntax) would fall into quite different category, so one might be
> subset/superset of the other, but I am wondering if we can take
> advantage of more table-driven approach taken by the previous step.
Sorry that it was not clear enough.
SOURCE_NONE fields are the fields that do not require object info.
By "technical fields" in this particular commit I mean fields that
will never be filled. So, it's a good idea to fill such fields with an
empty string and do not take them into account later (when we are
thinking whether we need to get and parse an object or not). I guess
we do not need to mark these "technical fields" in a special way: we
don't need this information. Moreover, some of the fields that I
filled with an empty string here, sometimes have non-empty
representation, and I changed nothing for such cases.
>
>
>> ref-filter.c | 8 +++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/ref-filter.c b/ref-filter.c
>> index 8611c24fd57d1..27733ef013bed 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -1497,6 +1497,7 @@ static int populate_value(struct ref_array_item *ref,
>> struct strbuf *err)
>> refname = get_symref(atom, ref);
>> else if (starts_with(name, "upstream")) {
>> const char *branch_name;
>> + v->s = "";
>> /* only local branches may have an upstream */
>> if (!skip_prefix(ref->refname, "refs/heads/",
>>_name))
>> @@ -1509,6 +1510,7 @@ static int populate_value(struct ref_array_item *ref,
>> struct strbuf *err)
>> continue;
>> } else if (atom->u.remote_ref.push) {
>> const char *branch_name;
>> + v->s = "";
>> if (!skip_prefix(ref->refname, "refs/heads/",
>>_name))
>> continue;
>> @@ -1549,22 +1551,26 @@ static int populate_value(struct ref_array_item
>> *ref, struct strbuf *err)
>> continue;
>> } else if (starts_with(name, "align")) {
>> v->handler = align_atom_handler;
>> + v->s = "";
>> continue;
>> } else if (!strcmp(name, "end")) {
>> v->handler = end_atom_handler;
>> + v->s = "";
>> continue;
>> } else if (starts_with(name, "if")) {
>> const char *s;
>> -
>> + v->s = "";
>> if (skip_prefix(name, "if:", ))
>> v->s = xstrdup(s);
>> v->handler = if_atom_handler;
>> continue;
>> } else if (!strcmp(name, "then")) {
>> v->handler = then_atom_handler;
>> + v->s = "";
>> continue;
>> } else if (!strcmp(name, "else")) {
>> v->handler = else_atom_handler;
>> + v->s = "";
>> continue;
>> } else
>> continue;
>>
>> --
>> https://github.com/git/git/pull/520
Re: [PATCH 2/4] ref-filter: add empty values to technical fields
Olga Telezhnaya writes:
> Atoms like "align" or "end" do not have string representation.
> Earlier we had to go and parse whole object with a hope that we
> could fill their string representations. It's easier to fill them
> with an empty string before we start to work with whole object.
>
> Signed-off-by: Olga Telezhnaia
> ---
Just being curious, but is there any meaningful relationship between
what was labelled as SOURCE_NONE in the previous step and what this
step calls "technical fields"? Things like "upstream" (which is not
affected by the contents of the object, but is affected by the ref
in question) and "if" (which merely exists to construct the language
syntax) would fall into quite different category, so one might be
subset/superset of the other, but I am wondering if we can take
advantage of more table-driven approach taken by the previous step.
> ref-filter.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 8611c24fd57d1..27733ef013bed 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1497,6 +1497,7 @@ static int populate_value(struct ref_array_item *ref,
> struct strbuf *err)
> refname = get_symref(atom, ref);
> else if (starts_with(name, "upstream")) {
> const char *branch_name;
> + v->s = "";
> /* only local branches may have an upstream */
> if (!skip_prefix(ref->refname, "refs/heads/",
>_name))
> @@ -1509,6 +1510,7 @@ static int populate_value(struct ref_array_item *ref,
> struct strbuf *err)
> continue;
> } else if (atom->u.remote_ref.push) {
> const char *branch_name;
> + v->s = "";
> if (!skip_prefix(ref->refname, "refs/heads/",
>_name))
> continue;
> @@ -1549,22 +1551,26 @@ static int populate_value(struct ref_array_item *ref,
> struct strbuf *err)
> continue;
> } else if (starts_with(name, "align")) {
> v->handler = align_atom_handler;
> + v->s = "";
> continue;
> } else if (!strcmp(name, "end")) {
> v->handler = end_atom_handler;
> + v->s = "";
> continue;
> } else if (starts_with(name, "if")) {
> const char *s;
> -
> + v->s = "";
> if (skip_prefix(name, "if:", ))
> v->s = xstrdup(s);
> v->handler = if_atom_handler;
> continue;
> } else if (!strcmp(name, "then")) {
> v->handler = then_atom_handler;
> + v->s = "";
> continue;
> } else if (!strcmp(name, "else")) {
> v->handler = else_atom_handler;
> + v->s = "";
> continue;
> } else
> continue;
>
> --
> https://github.com/git/git/pull/520
[PATCH 2/4] ref-filter: add empty values to technical fields
Atoms like "align" or "end" do not have string representation.
Earlier we had to go and parse whole object with a hope that we
could fill their string representations. It's easier to fill them
with an empty string before we start to work with whole object.
Signed-off-by: Olga Telezhnaia
---
ref-filter.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/ref-filter.c b/ref-filter.c
index 8611c24fd57d1..27733ef013bed 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1497,6 +1497,7 @@ static int populate_value(struct ref_array_item *ref,
struct strbuf *err)
refname = get_symref(atom, ref);
else if (starts_with(name, "upstream")) {
const char *branch_name;
+ v->s = "";
/* only local branches may have an upstream */
if (!skip_prefix(ref->refname, "refs/heads/",
_name))
@@ -1509,6 +1510,7 @@ static int populate_value(struct ref_array_item *ref,
struct strbuf *err)
continue;
} else if (atom->u.remote_ref.push) {
const char *branch_name;
+ v->s = "";
if (!skip_prefix(ref->refname, "refs/heads/",
_name))
continue;
@@ -1549,22 +1551,26 @@ static int populate_value(struct ref_array_item *ref,
struct strbuf *err)
continue;
} else if (starts_with(name, "align")) {
v->handler = align_atom_handler;
+ v->s = "";
continue;
} else if (!strcmp(name, "end")) {
v->handler = end_atom_handler;
+ v->s = "";
continue;
} else if (starts_with(name, "if")) {
const char *s;
-
+ v->s = "";
if (skip_prefix(name, "if:", ))
v->s = xstrdup(s);
v->handler = if_atom_handler;
continue;
} else if (!strcmp(name, "then")) {
v->handler = then_atom_handler;
+ v->s = "";
continue;
} else if (!strcmp(name, "else")) {
v->handler = else_atom_handler;
+ v->s = "";
continue;
} else
continue;
--
https://github.com/git/git/pull/520
