Am 22.08.2016 um 13:22 schrieb Michael Haggerty:
"git blame" already parsed generic diff options from the command line
via diff_opt_parse(), but instead of passing the resulting xdl_opts to
xdi_diff(), it sent its own xdl_opts, which only reflected the values of
the self-parsed options "-w" and "--minimal". Instead, rely on
diff_opt_parse() to parse all of the diff options, including "-w" and
"--minimal", and pass the resulting xdl_opts to xdi_diff().
Sounds useful: It allows more fine-grained control over which whitespace
changes to ignore and which diff algorithm to use. There is a bit of
overlap (e.g. with -b meaning show blank boundaries vs. ignore
whitespace changes), but with your patch blame's own options still take
precedence, so there should be no unpleasant surprises.
Signed-off-by: Michael Haggerty <[email protected]>
---
Somebody who knows more about how diff operations are configured
should please review this. I'm not certain that the change as
implemented won't have other unwanted side-effects, though of course
I checked that the test suite runs correctly.
I don't qualify, but I'll comment anyway..
builtin/blame.c | 11 ++--
t/t4059-diff-indent.sh | 160 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 165 insertions(+), 6 deletions(-)
create mode 100755 t/t4059-diff-indent.sh
This new test doesn't call git blame. Does it belong to a different
commit? And shouldn't the change to blame.c stand on its own, outside
of this series?
diff --git a/builtin/blame.c b/builtin/blame.c
index 7ec7823..cde2d15 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -48,11 +48,12 @@ static int show_root;
static int reverse;
static int blank_boundary;
static int incremental;
-static int xdl_opts;
static int abbrev = -1;
static int no_whole_file_rename;
static int show_progress;
+static struct rev_info revs;
+
static struct date_mode blame_date_mode = { DATE_ISO8601 };
static size_t blame_date_width;
@@ -137,11 +138,12 @@ struct progress_info {
static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b,
xdl_emit_hunk_consume_func_t hunk_func, void *cb_data)
{
- xpparam_t xpp = {0};
+ xpparam_t xpp;
xdemitconf_t xecfg = {0};
xdemitcb_t ecb = {NULL};
- xpp.flags = xdl_opts;
+ memset(&xpp, 0, sizeof(xpp));
+ xpp.flags = revs.diffopt.xdl_opts;
Why call memset instead of using a static initializer? The intent of
this patch is just to change the .flags assignment, isn't it?
xecfg.hunk_func = hunk_func;
ecb.priv = cb_data;
return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);
@@ -2517,7 +2519,6 @@ static int blame_move_callback(const struct option
*option, const char *arg, int
int cmd_blame(int argc, const char **argv, const char *prefix)
{
- struct rev_info revs;
const char *path;
struct scoreboard sb;
struct origin *o;
@@ -2548,8 +2549,6 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
OPT_BIT('l', NULL, &output_option, N_("Show long commit SHA1
(Default: off)"), OUTPUT_LONG_OBJECT_NAME),
OPT_BIT('s', NULL, &output_option, N_("Suppress author name and
timestamp (Default: off)"), OUTPUT_NO_AUTHOR),
OPT_BIT('e', "show-email", &output_option, N_("Show author email
instead of name (Default: off)"), OUTPUT_SHOW_EMAIL),
- OPT_BIT('w', NULL, &xdl_opts, N_("Ignore whitespace
differences"), XDF_IGNORE_WHITESPACE),
- OPT_BIT(0, "minimal", &xdl_opts, N_("Spend extra cycles to find
better match"), XDF_NEED_MINIMAL),
This removes -w and --minimal from blame's short help; diff options
should be mentioned somehow in exchange for that loss. Or perhaps they
should be mentioned in git-rev-list(1)? (git blame -h points to
git-rev-list(1) already.)
Documentation/git-blame.txt needs an update as well.
OPT_STRING('S', NULL, &revs_file, N_("file"), N_("Use revisions from
<file> instead of calling git-rev-list")),
OPT_STRING(0, "contents", &contents_from, N_("file"), N_("Use
<file>'s contents as the final image")),
{ OPTION_CALLBACK, 'C', NULL, &opt, N_("score"), N_("Find line
copies within and across files"), PARSE_OPT_OPTARG, blame_copy_callback },
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
