[PATCH] [RFC] Add a new config. option for skipping merges in git-log

2015-03-16 Thread Koosha Khajehmoogahi
This patch adds a 'showmerges' config. option for git-log.
This option determines whether the log should contain merge
commits or not. In essence, if this option is set to false,
git-log will be run as 'git-log --no-merges'.

To force git-log to show merges even if 'log.showmerges' is
set, we use --include-merges command line option.

Signed-off-by: Koosha Khajehmoogahi koosha.kha...@gmail.com
---
 Documentation/config.txt | 3 +++
 builtin/log.c| 9 +
 revision.c   | 2 ++
 revision.h   | 1 +
 4 files changed, 15 insertions(+)

This is the third time I send this patch as it seems it didn't
get delivered even after one hour!

Please help me with this patch. It seems that my --include-merges
command-line option does not have have any effect on the behavior
of git-log. I don't know why the value of force_show_merges is
always 0!

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1530255..7775b8c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1735,6 +1735,9 @@ log.showroot::
Tools like linkgit:git-log[1] or linkgit:git-whatchanged[1], which
normally hide the root commit will now show it. True by default.
 
+log.showmerges::
+   If true, merges will be shown in the log list. True by default.
+
 log.mailmap::
If true, makes linkgit:git-log[1], linkgit:git-show[1], and
linkgit:git-whatchanged[1] assume `--use-mailmap`.
diff --git a/builtin/log.c b/builtin/log.c
index dd8f3fc..867bcf2 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -31,6 +31,7 @@ static const char *default_date_mode = NULL;
 
 static int default_abbrev_commit;
 static int default_show_root = 1;
+static int default_max_parents = -1;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config;
@@ -108,6 +109,8 @@ static void cmd_log_init_defaults(struct rev_info *rev)
rev-diffopt.stat_graph_width = -1; /* respect statGraphWidth config */
rev-abbrev_commit = default_abbrev_commit;
rev-show_root_diff = default_show_root;
+   if (rev-force_show_merges == 0)
+   rev-max_parents = default_max_parents;
rev-subject_prefix = fmt_patch_subject_prefix;
DIFF_OPT_SET(rev-diffopt, ALLOW_TEXTCONV);
 
@@ -390,6 +393,12 @@ static int git_log_config(const char *var, const char 
*value, void *cb)
default_show_root = git_config_bool(var, value);
return 0;
}
+
+   if (!strcmp(var, log.showmerges)) {
+   default_max_parents = git_config_bool(var, value) ? -1 : 1;
+   return 0;
+   }
+
if (skip_prefix(var, color.decorate., slot_name))
return parse_decorate_color_config(var, slot_name, value);
if (!strcmp(var, log.mailmap)) {
diff --git a/revision.c b/revision.c
index 66520c6..e7073b8 100644
--- a/revision.c
+++ b/revision.c
@@ -1804,6 +1804,8 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
revs-min_parents = 2;
} else if (!strcmp(arg, --no-merges)) {
revs-max_parents = 1;
+   } else if (!strcmp(arg, --include-merges)) {
+   revs-force_show_merges = 1;
} else if (starts_with(arg, --min-parents=)) {
revs-min_parents = atoi(arg+14);
} else if (starts_with(arg, --no-min-parents)) {
diff --git a/revision.h b/revision.h
index 0ea8b4e..f496472 100644
--- a/revision.h
+++ b/revision.h
@@ -145,6 +145,7 @@ struct rev_info {
unsigned inttrack_linear:1,
track_first_time:1,
linear:1;
+   unsigned int force_show_merges:1;
 
enum date_mode date_mode;
 
-- 
1.9.1







--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] [RFC] Add a new config. option for skipping merges in git-log

2015-03-16 Thread Koosha Khajehmoogahi

This patch adds a 'showmerges' config. option for git-log.
This option determines whether the log should contain merge
commits or not. In essence, if this option is set to false,
git-log will be run as 'git-log --no-merges'.

To force git-log to show merges even if 'log.showmerges' is
set, we use --include-merges command line option.

Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de
---
 Documentation/config.txt | 3 +++
 builtin/log.c| 9 +
 revision.c   | 2 ++
 revision.h   | 1 +
 4 files changed, 15 insertions(+)



Please help me with this patch. It seems that my --include-merges
command-line option does not have have any effect on the behavior
of git-log. I don't know why the value of force_show_merges is
always 0!

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1530255..7775b8c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1735,6 +1735,9 @@ log.showroot::
Tools like linkgit:git-log[1] or linkgit:git-whatchanged[1], which
normally hide the root commit will now show it. True by default.
 
+log.showmerges::
+   If true, merges will be shown in the log list. True by default.
+
 log.mailmap::
If true, makes linkgit:git-log[1], linkgit:git-show[1], and
linkgit:git-whatchanged[1] assume `--use-mailmap`.
diff --git a/builtin/log.c b/builtin/log.c
index dd8f3fc..867bcf2 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -31,6 +31,7 @@ static const char *default_date_mode = NULL;
 
 static int default_abbrev_commit;
 static int default_show_root = 1;
+static int default_max_parents = -1;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config;
@@ -108,6 +109,8 @@ static void cmd_log_init_defaults(struct rev_info *rev)
rev-diffopt.stat_graph_width = -1; /* respect statGraphWidth config */
rev-abbrev_commit = default_abbrev_commit;
rev-show_root_diff = default_show_root;
+   if (rev-force_show_merges == 0)
+   rev-max_parents = default_max_parents;
rev-subject_prefix = fmt_patch_subject_prefix;
DIFF_OPT_SET(rev-diffopt, ALLOW_TEXTCONV);
 
@@ -390,6 +393,12 @@ static int git_log_config(const char *var, const char 
*value, void *cb)
default_show_root = git_config_bool(var, value);
return 0;
}
+
+   if (!strcmp(var, log.showmerges)) {
+   default_max_parents = git_config_bool(var, value) ? -1 : 1;
+   return 0;
+   }
+
if (skip_prefix(var, color.decorate., slot_name))
return parse_decorate_color_config(var, slot_name, value);
if (!strcmp(var, log.mailmap)) {
diff --git a/revision.c b/revision.c
index 66520c6..e7073b8 100644
--- a/revision.c
+++ b/revision.c
@@ -1804,6 +1804,8 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
revs-min_parents = 2;
} else if (!strcmp(arg, --no-merges)) {
revs-max_parents = 1;
+   } else if (!strcmp(arg, --include-merges)) {
+   revs-force_show_merges = 1;
} else if (starts_with(arg, --min-parents=)) {
revs-min_parents = atoi(arg+14);
} else if (starts_with(arg, --no-min-parents)) {
diff --git a/revision.h b/revision.h
index 0ea8b4e..f496472 100644
--- a/revision.h
+++ b/revision.h
@@ -145,6 +145,7 @@ struct rev_info {
unsigned inttrack_linear:1,
track_first_time:1,
linear:1;
+   unsigned int force_show_merges:1;
 
enum date_mode date_mode;
 
-- 
1.9.1



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] [RFC] Add a new config. option for skipping merges in git-log

2015-03-16 Thread Koosha Khajehmoogahi

This patch adds a 'showmerges' config. option for git-log.
This option determines whether the log should contain merge
commits or not. In essence, if this option is set to false,
git-log will be run as 'git-log --no-merges'.

To force git-log to show merges even if 'log.showmerges' is
set, we use --include-merges command line option.

Signed-off-by: Koosha Khajehmoogahi koo...@posteo.de
---
 Documentation/config.txt | 3 +++
 builtin/log.c| 9 +
 revision.c   | 2 ++
 revision.h   | 1 +
 4 files changed, 15 insertions(+)

This is the second time I send this patch as it seems it didn't
get delivered even after one hour!

Please help me with this patch. It seems that my --include-merges
command-line option does not have have any effect on the behavior
of git-log. I don't know why the value of force_show_merges is
always 0!

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1530255..7775b8c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1735,6 +1735,9 @@ log.showroot::
Tools like linkgit:git-log[1] or linkgit:git-whatchanged[1], which
normally hide the root commit will now show it. True by default.
 
+log.showmerges::
+   If true, merges will be shown in the log list. True by default.
+
 log.mailmap::
If true, makes linkgit:git-log[1], linkgit:git-show[1], and
linkgit:git-whatchanged[1] assume `--use-mailmap`.
diff --git a/builtin/log.c b/builtin/log.c
index dd8f3fc..867bcf2 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -31,6 +31,7 @@ static const char *default_date_mode = NULL;
 
 static int default_abbrev_commit;
 static int default_show_root = 1;
+static int default_max_parents = -1;
 static int decoration_style;
 static int decoration_given;
 static int use_mailmap_config;
@@ -108,6 +109,8 @@ static void cmd_log_init_defaults(struct rev_info *rev)
rev-diffopt.stat_graph_width = -1; /* respect statGraphWidth config */
rev-abbrev_commit = default_abbrev_commit;
rev-show_root_diff = default_show_root;
+   if (rev-force_show_merges == 0)
+   rev-max_parents = default_max_parents;
rev-subject_prefix = fmt_patch_subject_prefix;
DIFF_OPT_SET(rev-diffopt, ALLOW_TEXTCONV);
 
@@ -390,6 +393,12 @@ static int git_log_config(const char *var, const char 
*value, void *cb)
default_show_root = git_config_bool(var, value);
return 0;
}
+
+   if (!strcmp(var, log.showmerges)) {
+   default_max_parents = git_config_bool(var, value) ? -1 : 1;
+   return 0;
+   }
+
if (skip_prefix(var, color.decorate., slot_name))
return parse_decorate_color_config(var, slot_name, value);
if (!strcmp(var, log.mailmap)) {
diff --git a/revision.c b/revision.c
index 66520c6..e7073b8 100644
--- a/revision.c
+++ b/revision.c
@@ -1804,6 +1804,8 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
revs-min_parents = 2;
} else if (!strcmp(arg, --no-merges)) {
revs-max_parents = 1;
+   } else if (!strcmp(arg, --include-merges)) {
+   revs-force_show_merges = 1;
} else if (starts_with(arg, --min-parents=)) {
revs-min_parents = atoi(arg+14);
} else if (starts_with(arg, --no-min-parents)) {
diff --git a/revision.h b/revision.h
index 0ea8b4e..f496472 100644
--- a/revision.h
+++ b/revision.h
@@ -145,6 +145,7 @@ struct rev_info {
unsigned inttrack_linear:1,
track_first_time:1,
linear:1;
+   unsigned int force_show_merges:1;
 
enum date_mode date_mode;
 
-- 
1.9.1





--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] [RFC] Add a new config. option for skipping merges in git-log

2015-03-16 Thread Junio C Hamano
Koosha Khajehmoogahi koo...@posteo.de writes:

 This patch adds a 'showmerges' config. option for git-log.
 This option determines whether the log should contain merge
 commits or not. In essence, if this option is set to false,
 git-log will be run as 'git-log --no-merges'.

 To force git-log to show merges even if 'log.showmerges' is
 set, we use --include-merges command line option.

Yuck.

I agree that there is currently no way to revert the setting that is
touched by --merges and --no-merges back to the default, but I think
the right way to fix that is by streamlining these two options,
instead of piling yet another kludge --include-merges on top.

When we think about possible canned selection modes:

 (do we show merge commits?) * (do we show non-merge commits?)

we have four combinations.  Answering the above two questions with
No/No would end up showing nothing, which is meaningless, so that
leaves us three choices (of course, the user could choose to futz
directly with min/max-parents to select only Octopus merges, but
that is a more advanced/exotic usage).

Wouldn't it make more sense to spell which selection mode the user
wants with:

git log --merges=selection-mode

by naming the three meaningful selection modes with short and sweet
names?  Perhaps

default? show? both? -- show both merge commits and non-merge commits
only -- show only merge commits
skip -- show only non-merge commits

or something?

Now, as I always say, I am not great at naming things, so do not
take these names as the final suggestion, but I think you got the
idea.

Of course, then the traditional --merges option can be kept as a
short-hand for --merges=only, and --no-merges would become a
short-hand for --merges=skip.

Once we have done that streamlining of the command line options, it
will naturally follow that log.merges = show | only | skip would
be a useful configuration option.

I doubt we need an extra bit in rev_info to implement such a syntax
sugar.

 diff --git a/revision.h b/revision.h
 index 0ea8b4e..f496472 100644
 --- a/revision.h
 +++ b/revision.h
 @@ -145,6 +145,7 @@ struct rev_info {
   unsigned inttrack_linear:1,
   track_first_time:1,
   linear:1;
 + unsigned int force_show_merges:1;
  
   enum date_mode date_mode;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] [RFC] Add a new config. option for skipping merges in git-log

2015-03-16 Thread Koosha Khajehmoogahi


On 03/16/2015 09:50 PM, Junio C Hamano wrote:
 The command line overrides the config, no?  If you set up what the
 command line defaults to from the config, let the command line
 parser do whatever it wants to do, and do nothing else after the
 command line parser returns, wouldn't that be sufficient?
 
 Puzzled...
 

Yes, the command line overrides the config. But, the config and command
line parameters are parsed in two different files. The question is how
you would read the config in revision.c while parsing the command line
when there is no function in revision.c for that?

Sorry if I look baffled.
-- 
Koosha
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] [RFC] Add a new config. option for skipping merges in git-log

2015-03-16 Thread Junio C Hamano
Koosha Khajehmoogahi koo...@posteo.de writes:

 On 03/16/2015 09:50 PM, Junio C Hamano wrote:
 The command line overrides the config, no?  If you set up what the
 command line defaults to from the config, let the command line
 parser do whatever it wants to do, and do nothing else after the
 command line parser returns, wouldn't that be sufficient?
 
 Puzzled...
 

 Yes, the command line overrides the config. But, the config and command
 line parameters are parsed in two different files. The question is how
 you would read the config in revision.c while parsing the command line
 when there is no function in revision.c for that?

What I had in mind was along the line of the attached diff.

If I were doing this new feature, it would be in two-patch series,
whose first patch [1/2] would be just the revision.[ch] to give
these three canned selection options (we obviously need to update
the documentation and also add tests to make sure the new option
behaves sensibly when used alone, and in conjunction with existing
--merges and --no-merges options).  The second patch [2/2] would
teach git_log_config() to read the new configuration value and keep
it in a file-scope static variable in builtin/log.c, and then call
parse_merges_opt() with that value in the codepath somewhere after
init_revisions() is called on rev and before setup_revisions() is
called.

Note that the addition I made to cmd_log() below is for illustration
purposes only; I have no strong opinion on whether it is at the
right place in the codepath (it is one place that is between
init_revisions() and setup_revisions(), but it may not be the best
such place).  The call may want to be made a lot later in the
codepath, possibly inside cmd_log_init() instead of cmd_log(), for
example.  The choice depends on how widely you would want this new
configuration be honored, which I didn't think too deeply.  The
questions you would need to answer before deciding where is the best
place to make the call include:

 - Should git whatchanged also pay attention to it?
 - What about git show?

etc.


 builtin/log.c |  5 +
 revision.c| 20 
 revision.h|  2 ++
 3 files changed, 27 insertions(+)

diff --git a/builtin/log.c b/builtin/log.c
index dd8f3fc..11a191d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -36,6 +36,7 @@ static int decoration_given;
 static int use_mailmap_config;
 static const char *fmt_patch_subject_prefix = PATCH;
 static const char *fmt_pretty;
+static const char *log_merges;
 
 static const char * const builtin_log_usage[] = {
N_(git log [options] [revision range] [[--] path...]),
@@ -397,6 +398,8 @@ static int git_log_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
+   if (!strcmp(var, log.merges))
+   return git_config_string(log_merges, var, value);
if (grep_config(var, value, cb)  0)
return -1;
if (git_gpg_config(var, value, cb)  0)
@@ -628,6 +631,8 @@ int cmd_log(int argc, const char **argv, const char *prefix)
 
init_revisions(rev, prefix);
rev.always_show_header = 1;
+   if (log_merges  parse_merges_opt(rev, log_merges))
+   die(unknown config value for log.merges: %s, log_merges);
memset(opt, 0, sizeof(opt));
opt.def = HEAD;
opt.revarg_opt = REVARG_COMMITTISH;
diff --git a/revision.c b/revision.c
index dbee26b..2fa37b0 100644
--- a/revision.c
+++ b/revision.c
@@ -1678,6 +1678,23 @@ static void add_message_grep(struct rev_info *revs, 
const char *pattern)
add_grep(revs, pattern, GREP_PATTERN_BODY);
 }
 
+int parse_merges_opt(struct rev_info *revs, const char *param)
+{
+   if (!strcmp(param, both)) {
+   revs-min_parents = 0;
+   revs-max_parents = -1;
+   } else if (!strcmp(param, only)) {
+   revs-min_parents = 2;
+   revs-max_parents = -1;
+   } else if (!strcmp(param, skip)) {
+   revs-min_parents = 0;
+   revs-max_parents = 1;
+   } else {
+   return -1;
+   }
+   return 0;
+}
+
 static int handle_revision_opt(struct rev_info *revs, int argc, const char 
**argv,
   int *unkc, const char **unkv)
 {
@@ -1812,6 +1829,9 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
revs-max_parents = atoi(arg+14);
} else if (starts_with(arg, --no-max-parents)) {
revs-max_parents = -1;
+   } else if (starts_with(arg, --merges=)) {
+   if (parse_merges_opt(revs, arg + 9))
+   die(unknown option: %s, arg);
} else if (!strcmp(arg, --boundary)) {
revs-boundary = 1;
} else if (!strcmp(arg, --left-right)) {
diff --git a/revision.h b/revision.h
index 0ea8b4e..640589c 100644
--- a/revision.h
+++ b/revision.h
@@ -240,6 +240,8 @@ extern int setup_revisions(int argc, const char **argv, 
struct 

Re: [PATCH] [RFC] Add a new config. option for skipping merges in git-log

2015-03-16 Thread Koosha Khajehmoogahi

On 03/16/2015 06:53 PM, Junio C Hamano wrote:
 Koosha Khajehmoogahi koo...@posteo.de writes:
 
 This patch adds a 'showmerges' config. option for git-log.
 This option determines whether the log should contain merge
 commits or not. In essence, if this option is set to false,
 git-log will be run as 'git-log --no-merges'.

 To force git-log to show merges even if 'log.showmerges' is
 set, we use --include-merges command line option.
 
 Yuck.
 
 I agree that there is currently no way to revert the setting that is
 touched by --merges and --no-merges back to the default, but I think
 the right way to fix that is by streamlining these two options,
 instead of piling yet another kludge --include-merges on top.
 
 When we think about possible canned selection modes:
 
  (do we show merge commits?) * (do we show non-merge commits?)
 
 we have four combinations.  Answering the above two questions with
 No/No would end up showing nothing, which is meaningless, so that
 leaves us three choices (of course, the user could choose to futz
 directly with min/max-parents to select only Octopus merges, but
 that is a more advanced/exotic usage).
 
 Wouldn't it make more sense to spell which selection mode the user
 wants with:
 
   git log --merges=selection-mode
 
 by naming the three meaningful selection modes with short and sweet
 names?  Perhaps
 
 default? show? both? -- show both merge commits and non-merge commits
 only -- show only merge commits
 skip -- show only non-merge commits
 
 or something?
 
 Now, as I always say, I am not great at naming things, so do not
 take these names as the final suggestion, but I think you got the
 idea.
 
 Of course, then the traditional --merges option can be kept as a
 short-hand for --merges=only, and --no-merges would become a
 short-hand for --merges=skip.
 
 Once we have done that streamlining of the command line options, it
 will naturally follow that log.merges = show | only | skip would
 be a useful configuration option.
 
 I doubt we need an extra bit in rev_info to implement such a syntax
 sugar.
 
 diff --git a/revision.h b/revision.h
 index 0ea8b4e..f496472 100644
 --- a/revision.h
 +++ b/revision.h
 @@ -145,6 +145,7 @@ struct rev_info {
  unsigned inttrack_linear:1,
  track_first_time:1,
  linear:1;
 +unsigned int force_show_merges:1;
  
  enum date_mode date_mode;

Thanks for your suggestions. The extra bit in rev_info is used when
we need to compare user's command-line input to his configuration. Since
command-line input is processed in revision.c but config. options are read
in builtin/log.c, we need a way to pass the value to builtin/log.c. How
would you do that without this extra bit?

-- 
Koosha
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html