On Wed, Mar 15, 2017 at 02:09:33PM -0700, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> writes:
>
> > This actually drops the last caller for for_each_branch_ref(). I'm not
> > sure if we shoulder consider cleaning up the proliferation of
> > for_each_ref() helpers.
>
> That is certainly a good thing to do (but outside this series).
>
> I am wondering if "git diff" could have chosen a better way to
> arrange deleted and inserted lines. The first two if statements
> in the preimage corresponds to the new opt-with-value(branches)
> thing, the next two are repalced by opt-with-value(tags), an
> odd-man-out "--glob=" thing is replaced with handle_ref_opt(NULL),
> and then two "--remotes" thing are paired with the final
> opt-with-value(remotes) thing.
>
> That would break the usual expectation of seeing all "-" first and
> then "+" when there is no intervening " " context lines, so such an
> output might break tools, though (I do not think "git apply" would
> choke).
I think in this case you could still show it better with context lines
between. Something like:
@@ -749,42 +758,20 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
for_each_ref_in("refs/bisect/good", anti_reference, NULL);
continue;
}
- if (skip_prefix(arg, "--branches=", &arg)) {
- for_each_glob_ref_in(show_reference, arg,
- "refs/heads/", NULL);
- clear_ref_exclusion(&ref_excludes);
- continue;
- }
- if (!strcmp(arg, "--branches")) {
- for_each_branch_ref(show_reference, NULL);
- clear_ref_exclusion(&ref_excludes);
+ if (opt_with_value(arg, "--branches", &arg)) {
+ handle_ref_opt(arg, "refs/heads/");
continue;
}
- if (skip_prefix(arg, "--tags=", &arg)) {
- for_each_glob_ref_in(show_reference, arg,
- "refs/tags/", NULL);
- clear_ref_exclusion(&ref_excludes);
- if (!strcmp(arg, "--tags")) {
- for_each_tag_ref(show_reference, NULL);
- clear_ref_exclusion(&ref_excludes);
+ if (opt_with_value(arg, "--tags", &arg)) {
+ handle_ref_opt(arg, "refs/tags/");
continue;
}
if (skip_prefix(arg, "--glob=", &arg)) {
- for_each_glob_ref(show_reference, arg, NULL);
- clear_ref_exclusion(&ref_excludes);
+ handle_ref_opt(arg, NULL);
continue;
}
- if (skip_prefix(arg, "--remotes=", &arg)) {
- for_each_glob_ref_in(show_reference, arg,
- "refs/remotes/", NULL);
- clear_ref_exclusion(&ref_excludes);
- if (!strcmp(arg, "--remotes")) {
- for_each_remote_ref(show_reference, NULL);
- clear_ref_exclusion(&ref_excludes);
+ if (opt_with_value(arg, "--remotes", &arg)) {
+ handle_ref_opt(arg, "refs/remotes/");
continue;
}
if (skip_prefix(arg, "--exclude=", &arg)) {
I have no idea what heuristic might generate that diff, though. Using
--patience and --histogram does provided different results than Myers
for this case, but none produces the output above. The other two try to
push the new "--branches" lines up as a replacement for "--branches=",
but then you get a clump of "--branches", "--tags=", and "--tags".
-Peff