Hi Dscho,

On 08/10, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schinde...@gmx.de>
>
> [..]
> 
> @@ -13,15 +14,38 @@ NULL
>  int cmd_range_diff(int argc, const char **argv, const char *prefix)
>  {
>       int creation_factor = 60;
> +     struct diff_options diffopt = { NULL };
>       struct option options[] = {
>               OPT_INTEGER(0, "creation-factor", &creation_factor,
>                           N_("Percentage by which creation is weighted")),
>               OPT_END()
>       };
> -     int res = 0;
> +     int i, j, res = 0;
>       struct strbuf range1 = STRBUF_INIT, range2 = STRBUF_INIT;
>  
> +     git_config(git_diff_ui_config, NULL);
> +
> +     diff_setup(&diffopt);
> +     diffopt.output_format = DIFF_FORMAT_PATCH;
> +
>       argc = parse_options(argc, argv, NULL, options,
> +                          builtin_range_diff_usage, PARSE_OPT_KEEP_UNKNOWN |
> +                          PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0);
> +
> +     for (i = j = 1; i < argc && strcmp("--", argv[i]); ) {
> +             int c = diff_opt_parse(&diffopt, argv + i, argc - i, prefix);
> +
> +             if (!c)
> +                     argv[j++] = argv[i++];
> +             else
> +                     i += c;
> +     }

I don't think this handles "--" quite as would be expected.  Trying to
use "git range-diff -- js/range-diff-v4...HEAD" I get:

    $ ./git range-diff -- js/range-diff-v4...HEAD
    error: need two commit ranges
    usage: git range-diff [<options>] <old-base>..<old-tip> 
<new-base>..<new-tip>
       or: git range-diff [<options>] <old-tip>...<new-tip>
       or: git range-diff [<options>] <base> <old-tip> <new-tip>

        --creation-factor <n>
                              Percentage by which creation is weighted
        --no-dual-color       color both diff and diff-between-diffs

while what I would have expected is to actually get a range diff.
This happens because after we break out of the loop we don't add the
actual ranges to argv, but just skip them instead.

I think something like the following should be squashed in to this
patch.

--->8---
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index ef3ba22e29..132574c57a 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -53,6 +53,11 @@ int cmd_range_diff(int argc, const char **argv, const char 
*prefix)
                else
                        i += c;
        }
+       if (i < argc && !strcmp("--", argv[i])) {
+               i++; j++;
+               while (i < argc)
+                       argv[j++] = argv[i++];
+       }
        argc = j;
        diff_setup_done(&diffopt);
 
--->8---

> +     argc = j;
> +     diff_setup_done(&diffopt);
> +
> +     /* Make sure that there are no unparsed options */
> +     argc = parse_options(argc, argv, NULL,
> +                          options + ARRAY_SIZE(options) - 1, /* OPT_END */
>                            builtin_range_diff_usage, 0);
>  
>       if (argc == 2) {
> @@ -59,7 +83,8 @@ int cmd_range_diff(int argc, const char **argv, const char 
> *prefix)
>               usage_with_options(builtin_range_diff_usage, options);
>       }
>  
> -     res = show_range_diff(range1.buf, range2.buf, creation_factor);
> +     res = show_range_diff(range1.buf, range2.buf, creation_factor,
> +                           &diffopt);
>  
>       strbuf_release(&range1);
>       strbuf_release(&range2);
> diff --git a/range-diff.c b/range-diff.c
> index 2d94200d3..71883a4b7 100644
> --- a/range-diff.c
> +++ b/range-diff.c
> @@ -6,6 +6,7 @@
>  #include "hashmap.h"
>  #include "xdiff-interface.h"
>  #include "linear-assignment.h"
> +#include "diffcore.h"
>  
>  struct patch_util {
>       /* For the search for an exact match */
> @@ -258,7 +259,31 @@ static const char *short_oid(struct patch_util *util)
>       return find_unique_abbrev(&util->oid, DEFAULT_ABBREV);
>  }
>  
> -static void output(struct string_list *a, struct string_list *b)
> +static struct diff_filespec *get_filespec(const char *name, const char *p)
> +{
> +     struct diff_filespec *spec = alloc_filespec(name);
> +
> +     fill_filespec(spec, &null_oid, 0, 0644);
> +     spec->data = (char *)p;
> +     spec->size = strlen(p);
> +     spec->should_munmap = 0;
> +     spec->is_stdin = 1;
> +
> +     return spec;
> +}
> +
> +static void patch_diff(const char *a, const char *b,
> +                           struct diff_options *diffopt)
> +{
> +     diff_queue(&diff_queued_diff,
> +                get_filespec("a", a), get_filespec("b", b));
> +
> +     diffcore_std(diffopt);
> +     diff_flush(diffopt);
> +}
> +
> +static void output(struct string_list *a, struct string_list *b,
> +                struct diff_options *diffopt)
>  {
>       int i = 0, j = 0;
>  
> @@ -300,6 +325,9 @@ static void output(struct string_list *a, struct 
> string_list *b)
>                       printf("%d: %s ! %d: %s\n",
>                              b_util->matching + 1, short_oid(a_util),
>                              j + 1, short_oid(b_util));
> +                     if (!(diffopt->output_format & DIFF_FORMAT_NO_OUTPUT))
> +                             patch_diff(a->items[b_util->matching].string,
> +                                        b->items[j].string, diffopt);
>                       a_util->shown = 1;
>                       j++;
>               }
> @@ -307,7 +335,7 @@ static void output(struct string_list *a, struct 
> string_list *b)
>  }
>  
>  int show_range_diff(const char *range1, const char *range2,
> -                 int creation_factor)
> +                 int creation_factor, struct diff_options *diffopt)
>  {
>       int res = 0;
>  
> @@ -322,7 +350,7 @@ int show_range_diff(const char *range1, const char 
> *range2,
>       if (!res) {
>               find_exact_matches(&branch1, &branch2);
>               get_correspondences(&branch1, &branch2, creation_factor);
> -             output(&branch1, &branch2);
> +             output(&branch1, &branch2, diffopt);
>       }
>  
>       string_list_clear(&branch1, 1);
> diff --git a/range-diff.h b/range-diff.h
> index 7b6eef303..2407d46a3 100644
> --- a/range-diff.h
> +++ b/range-diff.h
> @@ -1,7 +1,9 @@
>  #ifndef RANGE_DIFF_H
>  #define RANGE_DIFF_H
>  
> +#include "diff.h"
> +
>  int show_range_diff(const char *range1, const char *range2,
> -                 int creation_factor);
> +                 int creation_factor, struct diff_options *diffopt);
>  
>  #endif
> -- 
> gitgitgadget
> 

Reply via email to