Re: [PATCH v2 1/5] revision: add --merges={show|only|hide} option

2015-04-06 Thread Eric Sunshine
On Fri, Apr 3, 2015 at 9:21 PM, Koosha Khajehmoogahi  wrote:
> From: Junio C Hamano 
>
> revision: add a new option 'merges=' with
> possible values of 'only', 'show' and 'hide'.
> The option is used when showing the list of commits.
> The value 'only' lists only merges. The value 'show'
> is the default behavior which shows the commits as well
> as merges and the value 'hide' makes it just list commit
> items.

A couple very minor comments if you to re-roll this series, otherwise
keep them in mind for future submissions:

Since "revision:" already prefixes the first line of the commit
message (in the Subject:), its reiteration at the start of this
paragraph is unnecessary. It's sufficient to start the paragraph with
"Add new --merges= option...".

The excessively jagged right edge of the paragraph makes it slightly
difficult to read. Try to wrap paragraphs to about 70-72 characters.

> [kk: chose names for options; wrote commit message]
>
> Helped-by: Eric Sunshine 
> Signed-off-by: Koosha Khajehmoogahi 
> ---
>  revision.c | 20 
>  revision.h |  1 +
>  2 files changed, 21 insertions(+)
>
> diff --git a/revision.c b/revision.c
> index 6399a04..c3c3dcc 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, "show")) {
> +   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, "hide")) {
> +   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)
>  {
> @@ -1800,6 +1817,9 @@ static int handle_revision_opt(struct rev_info *revs, 
> int argc, const char **arg
> revs->show_all = 1;
> } else if (!strcmp(arg, "--remove-empty")) {
> revs->remove_empty_trees = 1;
> +   } else if (starts_with(arg, "--merges=")) {
> +   if (parse_merges_opt(revs, arg + 9))
> +   die("unknown option: %s", arg);
> } else if (!strcmp(arg, "--merges")) {
> revs->min_parents = 2;
> } else if (!strcmp(arg, "--no-merges")) {
> diff --git a/revision.h b/revision.h
> index 0ea8b4e..f9df58c 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -240,6 +240,7 @@ extern int setup_revisions(int argc, const char **argv, 
> struct rev_info *revs,
>  extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t 
> *ctx,
>const struct option *options,
>const char * const usagestr[]);
> +extern int parse_merges_opt(struct rev_info *, const char *);
>  #define REVARG_CANNOT_BE_FILENAME 01
>  #define REVARG_COMMITTISH 02
>  extern int handle_revision_arg(const char *arg, struct rev_info *revs,
> --
> 2.3.3.263.g095251d.dirty
--
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 v2 1/5] revision: add --merges={show|only|hide} option

2015-04-03 Thread Koosha Khajehmoogahi
From: Junio C Hamano 

revision: add a new option 'merges=' with
possible values of 'only', 'show' and 'hide'.
The option is used when showing the list of commits.
The value 'only' lists only merges. The value 'show'
is the default behavior which shows the commits as well
as merges and the value 'hide' makes it just list commit
items.

[kk: chose names for options; wrote commit message]

Helped-by: Eric Sunshine 
Signed-off-by: Koosha Khajehmoogahi 
---
 revision.c | 20 
 revision.h |  1 +
 2 files changed, 21 insertions(+)

diff --git a/revision.c b/revision.c
index 6399a04..c3c3dcc 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, "show")) {
+   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, "hide")) {
+   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)
 {
@@ -1800,6 +1817,9 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
revs->show_all = 1;
} else if (!strcmp(arg, "--remove-empty")) {
revs->remove_empty_trees = 1;
+   } else if (starts_with(arg, "--merges=")) {
+   if (parse_merges_opt(revs, arg + 9))
+   die("unknown option: %s", arg);
} else if (!strcmp(arg, "--merges")) {
revs->min_parents = 2;
} else if (!strcmp(arg, "--no-merges")) {
diff --git a/revision.h b/revision.h
index 0ea8b4e..f9df58c 100644
--- a/revision.h
+++ b/revision.h
@@ -240,6 +240,7 @@ extern int setup_revisions(int argc, const char **argv, 
struct rev_info *revs,
 extern void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t 
*ctx,
   const struct option *options,
   const char * const usagestr[]);
+extern int parse_merges_opt(struct rev_info *, const char *);
 #define REVARG_CANNOT_BE_FILENAME 01
 #define REVARG_COMMITTISH 02
 extern int handle_revision_arg(const char *arg, struct rev_info *revs,
-- 
2.3.3.263.g095251d.dirty

--
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