Brandon Williams <bmwill@xxxxxxxxxx> writes:
> Factor out the logic for processing shallow, deepen, deepen_since, and
> deepen_not lines into their own functions to simplify the
> 'receive_needs()' function in addition to making it easier to reuse some
> of this logic when implementing protocol_v2.
These little functions that still require their incoming data to
begin with fixed prefixes feels a bit strange way to refactor the
logic for later reuse (when I imagine "reuse", the first use case
that comes to my mind is "this data source our new code reads from
gives the same data as the old 'shallow' packet used to give, but in
a different syntax"---so I'd restructure the code in such a way that
the caller figures out the syntax part and the called helper just
groks the "information contents" unwrapped from the surface syntax;
the syntax may be different in the new codepath but once unwrapped,
the "information contents" to be processed would not be different
hence we can reuse the helper).
IOW, I would have expected the caller to be not like this:
> - if (skip_prefix(line, "shallow ", &arg)) {
> - struct object_id oid;
> - struct object *object;
> - if (get_oid_hex(arg, &oid))
> - die("invalid shallow line: %s", line);
> - object = parse_object(&oid);
> - if (!object)
> - continue;
> - if (object->type != OBJ_COMMIT)
> - die("invalid shallow object %s", oid_to_hex(&oid));
> - if (!(object->flags & CLIENT_SHALLOW)) {
> - object->flags |= CLIENT_SHALLOW;
> - add_object_array(object, NULL, &shallows);
> - }
> + if (process_shallow(line, &shallows))
> continue;
> + if (process_deepen(line, &depth))
> continue;
...
but more like
if (skip_prefix(line, "shallow ", &arg) {
process_shallow(arg, &shallows);
continue;
}
if (skip_prefix(line, "deepen ", &arg) {
process_deepen(arg, &depth);
continue;
}
...
I need to defer the final judgment until I see how they are used,
though. It's not too big a deal either way---it just felt "not
quite right" to me.
