git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH 00/19] Convert revision.c to parseopt part 1/4
@ 2019-05-08 11:12 Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 01/19] revision.h: avoid bit fields in struct rev_info Nguyễn Thái Ngọc Duy
                   ` (18 more replies)
  0 siblings, 19 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Following the conversion in diff.c to use parse_options, this time it's
revision.c. There are about 76 patches to convert all options, split in
4 parts. After that there are about 10 more to convert rev users to use
parse_options() directly.

Full 76 patches are available here [1]. I'm not quite done with those
last "10 more" patches yet [2], still stuck at that ancient blame UI.
But I should be done by the 76 patches are merged.

[1] https://gitlab.com/pclouds/git/commits/revision-opt-parse-options
[2] https://gitlab.com/pclouds/git/commits/parse-options-step-no-more


Nguyễn Thái Ngọc Duy (19):
  revision.h: avoid bit fields in struct rev_info
  revision.h: move repo field down
  revision.c: prepare to convert handle_revision_pseudo_opt()
  rev-parseopt: convert --all
  rev-parseopt: convert --branches
  rev-parseopt: convert --bisect
  rev-parseopt: convert --tags
  rev-parseopt: convert --remotes
  rev-parseopt: convert --glob
  rev-parseopt: convert --exclude
  rev-parseopt: convert --reflog
  rev-parseopt: convert --indexed-objects
  rev-parseopt: convert --not
  rev-parseopt: convert --no-walk and --do-walk
  rev-parseopt: convert --single-worktree
  rev-parseopt: prepare to convert handle_revision_opt()
  rev-parseopt: convert --max-count
  rev-parseopt: convert --skip
  rev-parseopt: convert --min-age and --max-age

 Documentation/technical/api-diff.txt |   6 +-
 diff.c                               |  16 -
 diff.h                               |   1 -
 parse-options-cb.c                   |   8 +
 parse-options.h                      |   4 +
 revision.c                           | 465 +++++++++++++++++++--------
 revision.h                           | 172 +++++-----
 7 files changed, 441 insertions(+), 231 deletions(-)

-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 01/19] revision.h: avoid bit fields in struct rev_info
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 14:07   ` Derrick Stolee
  2019-05-08 11:12 ` [PATCH 02/19] revision.h: move repo field down Nguyễn Thái Ngọc Duy
                   ` (17 subsequent siblings)
  18 siblings, 1 reply; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Bitfield addresses cannot be passed around in a pointer. This makes it
hard to use parse-options to set/unset them. Turn this struct to
normal integers. This of course increases the size of this struct
multiple times, but since we only have a handful of rev_info variables
around, memory consumption is not at all a concern.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.h | 164 ++++++++++++++++++++++++++---------------------------
 1 file changed, 82 insertions(+), 82 deletions(-)

diff --git a/revision.h b/revision.h
index 4134dc6029..01e4c42274 100644
--- a/revision.h
+++ b/revision.h
@@ -107,95 +107,95 @@ struct rev_info {
 
 	unsigned int early_output;
 
-	unsigned int	ignore_missing:1,
-			ignore_missing_links:1;
+	unsigned int ignore_missing;
+	unsigned int ignore_missing_links;
 
 	/* Traversal flags */
-	unsigned int	dense:1,
-			prune:1,
-			no_walk:2,
-			remove_empty_trees:1,
-			simplify_history:1,
-			topo_order:1,
-			simplify_merges:1,
-			simplify_by_decoration:1,
-			single_worktree:1,
-			tag_objects:1,
-			tree_objects:1,
-			blob_objects:1,
-			verify_objects:1,
-			edge_hint:1,
-			edge_hint_aggressive:1,
-			limited:1,
-			unpacked:1,
-			boundary:2,
-			count:1,
-			left_right:1,
-			left_only:1,
-			right_only:1,
-			rewrite_parents:1,
-			print_parents:1,
-			show_decorations:1,
-			reverse:1,
-			reverse_output_stage:1,
-			cherry_pick:1,
-			cherry_mark:1,
-			bisect:1,
-			ancestry_path:1,
-			first_parent_only:1,
-			line_level_traverse:1,
-			tree_blobs_in_commit_order:1,
-
-			/*
-			 * Blobs are shown without regard for their existence.
-			 * But not so for trees: unless exclude_promisor_objects
-			 * is set and the tree in question is a promisor object;
-			 * OR ignore_missing_links is set, the revision walker
-			 * dies with a "bad tree object HASH" message when
-			 * encountering a missing tree. For callers that can
-			 * handle missing trees and want them to be filterable
-			 * and showable, set this to true. The revision walker
-			 * will filter and show such a missing tree as usual,
-			 * but will not attempt to recurse into this tree
-			 * object.
-			 */
-			do_not_die_on_missing_tree:1,
-
-			/* for internal use only */
-			exclude_promisor_objects:1;
+	unsigned int dense;
+	unsigned int prune;
+	unsigned int no_walk;
+	unsigned int remove_empty_trees;
+	unsigned int simplify_history;
+	unsigned int topo_order;
+	unsigned int simplify_merges;
+	unsigned int simplify_by_decoration;
+	unsigned int single_worktree;
+	unsigned int tag_objects;
+	unsigned int tree_objects;
+	unsigned int blob_objects;
+	unsigned int verify_objects;
+	unsigned int edge_hint;
+	unsigned int edge_hint_aggressive;
+	unsigned int limited;
+	unsigned int unpacked;
+	unsigned int boundary;
+	unsigned int count;
+	unsigned int left_right;
+	unsigned int left_only;
+	unsigned int right_only;
+	unsigned int rewrite_parents;
+	unsigned int print_parents;
+	unsigned int show_decorations;
+	unsigned int reverse;
+	unsigned int reverse_output_stage;
+	unsigned int cherry_pick;
+	unsigned int cherry_mark;
+	unsigned int bisect;
+	unsigned int ancestry_path;
+	unsigned int first_parent_only;
+	unsigned int line_level_traverse;
+	unsigned int tree_blobs_in_commit_order;
+
+	/*
+	 * Blobs are shown without regard for their existence.
+	 * But not so for trees: unless exclude_promisor_objects
+	 * is set and the tree in question is a promisor object;
+	 * OR ignore_missing_links is set, the revision walker
+	 * dies with a "bad tree object HASH" message when
+	 * encountering a missing tree. For callers that can
+	 * handle missing trees and want them to be filterable
+	 * and showable, set this to true. The revision walker
+	 * will filter and show such a missing tree as usual,
+	 * but will not attempt to recurse into this tree
+	 * object.
+	 */
+	unsigned int do_not_die_on_missing_tree;
+
+	/* for internal use only */
+	unsigned int exclude_promisor_objects;
 
 	/* Diff flags */
-	unsigned int	diff:1,
-			full_diff:1,
-			show_root_diff:1,
-			no_commit_id:1,
-			verbose_header:1,
-			ignore_merges:1,
-			combine_merges:1,
-			combined_all_paths:1,
-			dense_combined_merges:1,
-			always_show_header:1;
+	unsigned int diff;
+	unsigned int full_diff;
+	unsigned int show_root_diff;
+	unsigned int no_commit_id;
+	unsigned int verbose_header;
+	unsigned int ignore_merges;
+	unsigned int combine_merges;
+	unsigned int combined_all_paths;
+	unsigned int dense_combined_merges;
+	unsigned int always_show_header;
 
 	/* Format info */
-	unsigned int	shown_one:1,
-			shown_dashes:1,
-			show_merge:1,
-			show_notes:1,
-			show_notes_given:1,
-			show_signature:1,
-			pretty_given:1,
-			abbrev_commit:1,
-			abbrev_commit_given:1,
-			zero_commit:1,
-			use_terminator:1,
-			missing_newline:1,
-			date_mode_explicit:1,
-			preserve_subject:1;
-	unsigned int	disable_stdin:1;
+	unsigned int shown_one;
+	unsigned int shown_dashes;
+	unsigned int show_merge;
+	unsigned int show_notes;
+	unsigned int show_notes_given;
+	unsigned int show_signature;
+	unsigned int pretty_given;
+	unsigned int abbrev_commit;
+	unsigned int abbrev_commit_given;
+	unsigned int zero_commit;
+	unsigned int use_terminator;
+	unsigned int missing_newline;
+	unsigned int date_mode_explicit;
+	unsigned int preserve_subject;
+	unsigned int disable_stdin;
 	/* --show-linear-break */
-	unsigned int	track_linear:1,
-			track_first_time:1,
-			linear:1;
+	unsigned int track_linear;
+	unsigned int track_first_time;
+	unsigned int linear;
 
 	struct date_mode date_mode;
 	int		expand_tabs_in_log; /* unset if negative */
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 02/19] revision.h: move repo field down
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 01/19] revision.h: avoid bit fields in struct rev_info Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 03/19] revision.c: prepare to convert handle_revision_pseudo_opt() Nguyễn Thái Ngọc Duy
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

This block at the top of rev_info is "Starting list" and repo is
obviously not one. Move it to the bottom since it's not that important
to stay on top.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/revision.h b/revision.h
index 01e4c42274..71e724c59c 100644
--- a/revision.h
+++ b/revision.h
@@ -74,7 +74,6 @@ struct rev_info {
 	/* Starting list */
 	struct commit_list *commits;
 	struct object_array pending;
-	struct repository *repo;
 
 	/* Parents of shown commits */
 	struct object_array boundary_commits;
@@ -278,6 +277,8 @@ struct rev_info {
 	struct revision_sources *sources;
 
 	struct topo_walk_info *topo_walk_info;
+
+	struct repository *repo;
 };
 
 int ref_excluded(struct string_list *, const char *path);
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 03/19] revision.c: prepare to convert handle_revision_pseudo_opt()
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 01/19] revision.h: avoid bit fields in struct rev_info Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 02/19] revision.h: move repo field down Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 04/19] rev-parseopt: convert --all Nguyễn Thái Ngọc Duy
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

This patch is essentially no-op. It allows to parse_options() to handle
some options. But the new option list remains empty. The option will be
moved one by one from the old manual parsing code to this list.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 44 ++++++++++++++++++++++++++++++++++----------
 revision.h |  2 ++
 2 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/revision.c b/revision.c
index d4aaf0ef25..65d40c9255 100644
--- a/revision.c
+++ b/revision.c
@@ -26,6 +26,7 @@
 #include "argv-array.h"
 #include "commit-reach.h"
 #include "commit-graph.h"
+#include "parse-options.h"
 #include "prio-queue.h"
 #include "hashmap.h"
 
@@ -1598,6 +1599,8 @@ static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,
 	return 1;
 }
 
+static void make_pseudo_options(struct rev_info *revs);
+
 void repo_init_revisions(struct repository *r,
 			 struct rev_info *revs,
 			 const char *prefix)
@@ -1638,6 +1641,7 @@ void repo_init_revisions(struct repository *r,
 	}
 
 	revs->notes_opt.use_default_notes = -1;
+	make_pseudo_options(revs);
 }
 
 static void add_pending_commit_list(struct rev_info *revs,
@@ -2355,6 +2359,25 @@ static int for_each_good_bisect_ref(struct ref_store *refs, each_ref_fn fn, void
 	return for_each_bisect_ref(refs, fn, cb_data, term_good);
 }
 
+static void make_pseudo_options(struct rev_info *revs)
+{
+	/*
+	 * NOTE!
+	 *
+	 * Commands like "git shortlog" will not accept the options below
+	 * unless parse_revision_opt queues them (as opposed to erroring
+	 * out).
+	 *
+	 * When implementing your new pseudo-option, remember to
+	 * register it in the list at the top of handle_revision_opt.
+	 */
+	struct option options[] = {
+		OPT_END()
+	};
+	ALLOC_ARRAY(revs->pseudo_options, ARRAY_SIZE(options));
+	memcpy(revs->pseudo_options, options, sizeof(options));
+}
+
 static int handle_revision_pseudo_opt(const char *submodule,
 				struct rev_info *revs,
 				int argc, const char **argv, int *flags)
@@ -2377,16 +2400,16 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	} else
 		refs = get_main_ref_store(revs->repo);
 
-	/*
-	 * NOTE!
-	 *
-	 * Commands like "git shortlog" will not accept the options below
-	 * unless parse_revision_opt queues them (as opposed to erroring
-	 * out).
-	 *
-	 * When implementing your new pseudo-option, remember to
-	 * register it in the list at the top of handle_revision_opt.
-	 */
+	argc = parse_options(argc, argv, revs->prefix,
+			     revs->pseudo_options, NULL,
+			     PARSE_OPT_KEEP_DASHDASH |
+			     PARSE_OPT_KEEP_UNKNOWN |
+			     PARSE_OPT_NO_INTERNAL_HELP |
+			     PARSE_OPT_ONE_SHOT |
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+	if (argc)
+		return argc;
+
 	if (!strcmp(arg, "--all")) {
 		handle_refs(refs, revs, *flags, refs_for_each_ref);
 		handle_refs(refs, revs, *flags, refs_head_ref);
@@ -2685,6 +2708,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	if (revs->expand_tabs_in_log < 0)
 		revs->expand_tabs_in_log = revs->expand_tabs_in_log_default;
 
+	FREE_AND_NULL(revs->pseudo_options);
 	return left;
 }
 
diff --git a/revision.h b/revision.h
index 71e724c59c..0769c97dee 100644
--- a/revision.h
+++ b/revision.h
@@ -39,6 +39,7 @@
 #define DECORATE_FULL_REFS	2
 
 struct log_info;
+struct option;
 struct repository;
 struct rev_info;
 struct string_list;
@@ -279,6 +280,7 @@ struct rev_info {
 	struct topo_walk_info *topo_walk_info;
 
 	struct repository *repo;
+	struct option *pseudo_options;
 };
 
 int ref_excluded(struct string_list *, const char *path);
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 04/19] rev-parseopt: convert --all
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (2 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 03/19] revision.c: prepare to convert handle_revision_pseudo_opt() Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 05/19] rev-parseopt: convert --branches Nguyễn Thái Ngọc Duy
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 41 ++++++++++++++++++++++++++++++-----------
 revision.h |  2 ++
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/revision.c b/revision.c
index 65d40c9255..9a346577f6 100644
--- a/revision.c
+++ b/revision.c
@@ -30,6 +30,10 @@
 #include "prio-queue.h"
 #include "hashmap.h"
 
+#define OPT_REV_NOARG(s, l, h, cb) \
+	OPT_CALLBACK_F(s, l, revs, NULL, h, \
+		       PARSE_OPT_NONEG | PARSE_OPT_NOARG, cb)
+
 volatile show_early_output_fn_t show_early_output;
 
 static const char *term_bad;
@@ -2359,6 +2363,26 @@ static int for_each_good_bisect_ref(struct ref_store *refs, each_ref_fn fn, void
 	return for_each_bisect_ref(refs, fn, cb_data, term_good);
 }
 
+static int rev_opt_all(const struct option *opt, const char *arg, int unset)
+{
+	struct rev_info		*revs  = opt->value;
+	int			 flags = *revs->pseudo_flags;
+	struct ref_store	*refs  = revs->pseudo_refs;
+
+	BUG_ON_OPT_NEG(unset);
+	BUG_ON_OPT_ARG(arg);
+	handle_refs(refs, revs, flags, refs_for_each_ref);
+	handle_refs(refs, revs, flags, refs_head_ref);
+	if (!revs->single_worktree) {
+		struct all_refs_cb cb;
+
+		init_all_refs_cb(&cb, revs, flags);
+		other_head_refs(handle_one_ref, &cb);
+	}
+	clear_ref_exclusion(&revs->ref_excludes);
+	return 0;
+}
+
 static void make_pseudo_options(struct rev_info *revs)
 {
 	/*
@@ -2372,6 +2396,9 @@ static void make_pseudo_options(struct rev_info *revs)
 	 * register it in the list at the top of handle_revision_opt.
 	 */
 	struct option options[] = {
+		OPT_REV_NOARG(0, "all",
+			      N_("include all refs in refs/ and HEAD"),
+			      rev_opt_all),
 		OPT_END()
 	};
 	ALLOC_ARRAY(revs->pseudo_options, ARRAY_SIZE(options));
@@ -2400,6 +2427,8 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	} else
 		refs = get_main_ref_store(revs->repo);
 
+	revs->pseudo_flags = flags;
+	revs->pseudo_refs = refs;
 	argc = parse_options(argc, argv, revs->prefix,
 			     revs->pseudo_options, NULL,
 			     PARSE_OPT_KEEP_DASHDASH |
@@ -2410,17 +2439,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (argc)
 		return argc;
 
-	if (!strcmp(arg, "--all")) {
-		handle_refs(refs, revs, *flags, refs_for_each_ref);
-		handle_refs(refs, revs, *flags, refs_head_ref);
-		if (!revs->single_worktree) {
-			struct all_refs_cb cb;
-
-			init_all_refs_cb(&cb, revs, *flags);
-			other_head_refs(handle_one_ref, &cb);
-		}
-		clear_ref_exclusion(&revs->ref_excludes);
-	} else if (!strcmp(arg, "--branches")) {
+	if (!strcmp(arg, "--branches")) {
 		handle_refs(refs, revs, *flags, refs_for_each_branch_ref);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--bisect")) {
diff --git a/revision.h b/revision.h
index 0769c97dee..cec5215c04 100644
--- a/revision.h
+++ b/revision.h
@@ -281,6 +281,8 @@ struct rev_info {
 
 	struct repository *repo;
 	struct option *pseudo_options;
+	int *pseudo_flags;
+	struct ref_store *pseudo_refs;
 };
 
 int ref_excluded(struct string_list *, const char *path);
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 05/19] rev-parseopt: convert --branches
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (3 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 04/19] rev-parseopt: convert --all Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 06/19] rev-parseopt: convert --bisect Nguyễn Thái Ngọc Duy
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/revision.c b/revision.c
index 9a346577f6..7db1109b57 100644
--- a/revision.c
+++ b/revision.c
@@ -34,6 +34,10 @@
 	OPT_CALLBACK_F(s, l, revs, NULL, h, \
 		       PARSE_OPT_NONEG | PARSE_OPT_NOARG, cb)
 
+#define OPT_REV_OPTARG(s, l, a, h, cb) \
+	OPT_CALLBACK_F(s, l, revs, a, h, \
+		       PARSE_OPT_NONEG | PARSE_OPT_OPTARG, cb)
+
 volatile show_early_output_fn_t show_early_output;
 
 static const char *term_bad;
@@ -2383,6 +2387,27 @@ static int rev_opt_all(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static int rev_opt_branches(const struct option *opt,
+			    const char *arg, int unset)
+{
+	struct rev_info		*revs  = opt->value;
+	int			 flags = *revs->pseudo_flags;
+	struct ref_store	*refs  = revs->pseudo_refs;
+
+	BUG_ON_OPT_NEG(unset);
+	if (arg) {
+		struct all_refs_cb cb;
+
+		init_all_refs_cb(&cb, revs, flags);
+		for_each_glob_ref_in(handle_one_ref, arg, "refs/heads/", &cb);
+		clear_ref_exclusion(&revs->ref_excludes);
+	} else {
+		handle_refs(refs, revs, flags, refs_for_each_branch_ref);
+		clear_ref_exclusion(&revs->ref_excludes);
+	}
+	return 0;
+}
+
 static void make_pseudo_options(struct rev_info *revs)
 {
 	/*
@@ -2399,6 +2424,9 @@ static void make_pseudo_options(struct rev_info *revs)
 		OPT_REV_NOARG(0, "all",
 			      N_("include all refs in refs/ and HEAD"),
 			      rev_opt_all),
+		OPT_REV_OPTARG(0, "branches", N_("<pattern>"),
+			       N_("include all refs in refs/heads (optionally matches pattern)"),
+			       rev_opt_branches),
 		OPT_END()
 	};
 	ALLOC_ARRAY(revs->pseudo_options, ARRAY_SIZE(options));
@@ -2439,10 +2467,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (argc)
 		return argc;
 
-	if (!strcmp(arg, "--branches")) {
-		handle_refs(refs, revs, *flags, refs_for_each_branch_ref);
-		clear_ref_exclusion(&revs->ref_excludes);
-	} else if (!strcmp(arg, "--bisect")) {
+	if (!strcmp(arg, "--bisect")) {
 		read_bisect_terms(&term_bad, &term_good);
 		handle_refs(refs, revs, *flags, for_each_bad_bisect_ref);
 		handle_refs(refs, revs, *flags ^ (UNINTERESTING | BOTTOM),
@@ -2463,11 +2488,6 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	} else if ((argcount = parse_long_opt("exclude", argv, &optarg))) {
 		add_ref_exclusion(&revs->ref_excludes, optarg);
 		return argcount;
-	} else if (skip_prefix(arg, "--branches=", &optarg)) {
-		struct all_refs_cb cb;
-		init_all_refs_cb(&cb, revs, *flags);
-		for_each_glob_ref_in(handle_one_ref, optarg, "refs/heads/", &cb);
-		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (skip_prefix(arg, "--tags=", &optarg)) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 06/19] rev-parseopt: convert --bisect
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (4 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 05/19] rev-parseopt: convert --branches Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 07/19] rev-parseopt: convert --tags Nguyễn Thái Ngọc Duy
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/revision.c b/revision.c
index 7db1109b57..0d34f81716 100644
--- a/revision.c
+++ b/revision.c
@@ -2387,6 +2387,23 @@ static int rev_opt_all(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static int rev_opt_bisect(const struct option *opt,
+			  const char *arg, int unset)
+{
+	struct rev_info		*revs  = opt->value;
+	int			 flags = *revs->pseudo_flags;
+	struct ref_store	*refs  = revs->pseudo_refs;
+
+	BUG_ON_OPT_NEG(unset);
+	BUG_ON_OPT_ARG(arg);
+	read_bisect_terms(&term_bad, &term_good);
+	handle_refs(refs, revs, flags, for_each_bad_bisect_ref);
+	handle_refs(refs, revs, flags ^ (UNINTERESTING | BOTTOM),
+		    for_each_good_bisect_ref);
+	revs->bisect = 1;
+	return 0;
+}
+
 static int rev_opt_branches(const struct option *opt,
 			    const char *arg, int unset)
 {
@@ -2427,6 +2444,9 @@ static void make_pseudo_options(struct rev_info *revs)
 		OPT_REV_OPTARG(0, "branches", N_("<pattern>"),
 			       N_("include all refs in refs/heads (optionally matches pattern)"),
 			       rev_opt_branches),
+		OPT_REV_NOARG(0, "bisect",
+			      N_("synonym to refs/bisect/good-* --not refs/bisect/bad"),
+			      rev_opt_bisect),
 		OPT_END()
 	};
 	ALLOC_ARRAY(revs->pseudo_options, ARRAY_SIZE(options));
@@ -2467,13 +2487,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (argc)
 		return argc;
 
-	if (!strcmp(arg, "--bisect")) {
-		read_bisect_terms(&term_bad, &term_good);
-		handle_refs(refs, revs, *flags, for_each_bad_bisect_ref);
-		handle_refs(refs, revs, *flags ^ (UNINTERESTING | BOTTOM),
-			    for_each_good_bisect_ref);
-		revs->bisect = 1;
-	} else if (!strcmp(arg, "--tags")) {
+	if (!strcmp(arg, "--tags")) {
 		handle_refs(refs, revs, *flags, refs_for_each_tag_ref);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--remotes")) {
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 07/19] rev-parseopt: convert --tags
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (5 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 06/19] rev-parseopt: convert --bisect Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 08/19] rev-parseopt: convert --remotes Nguyễn Thái Ngọc Duy
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/revision.c b/revision.c
index 0d34f81716..5183cdf66d 100644
--- a/revision.c
+++ b/revision.c
@@ -2425,6 +2425,27 @@ static int rev_opt_branches(const struct option *opt,
 	return 0;
 }
 
+static int rev_opt_tags(const struct option *opt,
+			const char *arg, int unset)
+{
+	struct rev_info		*revs  = opt->value;
+	int			 flags = *revs->pseudo_flags;
+	struct ref_store	*refs  = revs->pseudo_refs;
+
+	BUG_ON_OPT_NEG(unset);
+	if (arg) {
+		struct all_refs_cb cb;
+
+		init_all_refs_cb(&cb, revs, flags);
+		for_each_glob_ref_in(handle_one_ref, arg, "refs/tags/", &cb);
+		clear_ref_exclusion(&revs->ref_excludes);
+	} else {
+		handle_refs(refs, revs, flags, refs_for_each_tag_ref);
+		clear_ref_exclusion(&revs->ref_excludes);
+	}
+	return 0;
+}
+
 static void make_pseudo_options(struct rev_info *revs)
 {
 	/*
@@ -2447,6 +2468,9 @@ static void make_pseudo_options(struct rev_info *revs)
 		OPT_REV_NOARG(0, "bisect",
 			      N_("synonym to refs/bisect/good-* --not refs/bisect/bad"),
 			      rev_opt_bisect),
+		OPT_REV_OPTARG(0, "tags", N_("<pattern>"),
+			       N_("include all refs in refs/tags (optionally matches pattern)"),
+			       rev_opt_tags),
 		OPT_END()
 	};
 	ALLOC_ARRAY(revs->pseudo_options, ARRAY_SIZE(options));
@@ -2487,10 +2511,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (argc)
 		return argc;
 
-	if (!strcmp(arg, "--tags")) {
-		handle_refs(refs, revs, *flags, refs_for_each_tag_ref);
-		clear_ref_exclusion(&revs->ref_excludes);
-	} else if (!strcmp(arg, "--remotes")) {
+	if (!strcmp(arg, "--remotes")) {
 		handle_refs(refs, revs, *flags, refs_for_each_remote_ref);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if ((argcount = parse_long_opt("glob", argv, &optarg))) {
@@ -2502,11 +2523,6 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	} else if ((argcount = parse_long_opt("exclude", argv, &optarg))) {
 		add_ref_exclusion(&revs->ref_excludes, optarg);
 		return argcount;
-	} else if (skip_prefix(arg, "--tags=", &optarg)) {
-		struct all_refs_cb cb;
-		init_all_refs_cb(&cb, revs, *flags);
-		for_each_glob_ref_in(handle_one_ref, optarg, "refs/tags/", &cb);
-		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (skip_prefix(arg, "--remotes=", &optarg)) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 08/19] rev-parseopt: convert --remotes
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (6 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 07/19] rev-parseopt: convert --tags Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 09/19] rev-parseopt: convert --glob Nguyễn Thái Ngọc Duy
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/revision.c b/revision.c
index 5183cdf66d..bcfca8856f 100644
--- a/revision.c
+++ b/revision.c
@@ -2425,6 +2425,27 @@ static int rev_opt_branches(const struct option *opt,
 	return 0;
 }
 
+static int rev_opt_remotes(const struct option *opt,
+			   const char *arg, int unset)
+{
+	struct rev_info		*revs  = opt->value;
+	int			 flags = *revs->pseudo_flags;
+	struct ref_store	*refs  = revs->pseudo_refs;
+
+	BUG_ON_OPT_NEG(unset);
+	if (arg) {
+		struct all_refs_cb cb;
+
+		init_all_refs_cb(&cb, revs, flags);
+		for_each_glob_ref_in(handle_one_ref, arg, "refs/remotes/", &cb);
+		clear_ref_exclusion(&revs->ref_excludes);
+	} else {
+		handle_refs(refs, revs, flags, refs_for_each_remote_ref);
+		clear_ref_exclusion(&revs->ref_excludes);
+	}
+	return 0;
+}
+
 static int rev_opt_tags(const struct option *opt,
 			const char *arg, int unset)
 {
@@ -2471,6 +2492,9 @@ static void make_pseudo_options(struct rev_info *revs)
 		OPT_REV_OPTARG(0, "tags", N_("<pattern>"),
 			       N_("include all refs in refs/tags (optionally matches pattern)"),
 			       rev_opt_tags),
+		OPT_REV_OPTARG(0, "remotes", N_("<pattern>"),
+			       N_("include all refs in refs/remotes (optionally matches pattern)"),
+			       rev_opt_remotes),
 		OPT_END()
 	};
 	ALLOC_ARRAY(revs->pseudo_options, ARRAY_SIZE(options));
@@ -2511,10 +2535,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (argc)
 		return argc;
 
-	if (!strcmp(arg, "--remotes")) {
-		handle_refs(refs, revs, *flags, refs_for_each_remote_ref);
-		clear_ref_exclusion(&revs->ref_excludes);
-	} else if ((argcount = parse_long_opt("glob", argv, &optarg))) {
+	if ((argcount = parse_long_opt("glob", argv, &optarg))) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
 		for_each_glob_ref(handle_one_ref, optarg, &cb);
@@ -2523,11 +2544,6 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	} else if ((argcount = parse_long_opt("exclude", argv, &optarg))) {
 		add_ref_exclusion(&revs->ref_excludes, optarg);
 		return argcount;
-	} else if (skip_prefix(arg, "--remotes=", &optarg)) {
-		struct all_refs_cb cb;
-		init_all_refs_cb(&cb, revs, *flags);
-		for_each_glob_ref_in(handle_one_ref, optarg, "refs/remotes/", &cb);
-		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--reflog")) {
 		add_reflogs_to_pending(revs, *flags);
 	} else if (!strcmp(arg, "--indexed-objects")) {
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 09/19] rev-parseopt: convert --glob
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (7 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 08/19] rev-parseopt: convert --remotes Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 10/19] rev-parseopt: convert --exclude Nguyễn Thái Ngọc Duy
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/revision.c b/revision.c
index bcfca8856f..013b8ec19f 100644
--- a/revision.c
+++ b/revision.c
@@ -30,6 +30,9 @@
 #include "prio-queue.h"
 #include "hashmap.h"
 
+#define OPT_REV(s, l, a, h, cb)		 \
+	OPT_CALLBACK_F(s, l, revs, a, h, PARSE_OPT_NONEG, cb)
+
 #define OPT_REV_NOARG(s, l, h, cb) \
 	OPT_CALLBACK_F(s, l, revs, NULL, h, \
 		       PARSE_OPT_NONEG | PARSE_OPT_NOARG, cb)
@@ -2425,6 +2428,20 @@ static int rev_opt_branches(const struct option *opt,
 	return 0;
 }
 
+static int rev_opt_glob(const struct option *opt,
+			const char *arg, int unset)
+{
+	struct rev_info	*revs  = opt->value;
+	int flags = *revs->pseudo_flags;
+	struct all_refs_cb cb;
+
+	BUG_ON_OPT_NEG(unset);
+	init_all_refs_cb(&cb, revs, flags);
+	for_each_glob_ref(handle_one_ref, arg, &cb);
+	clear_ref_exclusion(&revs->ref_excludes);
+	return 0;
+}
+
 static int rev_opt_remotes(const struct option *opt,
 			   const char *arg, int unset)
 {
@@ -2495,6 +2512,9 @@ static void make_pseudo_options(struct rev_info *revs)
 		OPT_REV_OPTARG(0, "remotes", N_("<pattern>"),
 			       N_("include all refs in refs/remotes (optionally matches pattern)"),
 			       rev_opt_remotes),
+		OPT_REV(0, "glob", N_("<pattern>"),
+			N_("include all refs matching shell glob"),
+			rev_opt_glob),
 		OPT_END()
 	};
 	ALLOC_ARRAY(revs->pseudo_options, ARRAY_SIZE(options));
@@ -2535,13 +2555,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (argc)
 		return argc;
 
-	if ((argcount = parse_long_opt("glob", argv, &optarg))) {
-		struct all_refs_cb cb;
-		init_all_refs_cb(&cb, revs, *flags);
-		for_each_glob_ref(handle_one_ref, optarg, &cb);
-		clear_ref_exclusion(&revs->ref_excludes);
-		return argcount;
-	} else if ((argcount = parse_long_opt("exclude", argv, &optarg))) {
+	if ((argcount = parse_long_opt("exclude", argv, &optarg))) {
 		add_ref_exclusion(&revs->ref_excludes, optarg);
 		return argcount;
 	} else if (!strcmp(arg, "--reflog")) {
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 10/19] rev-parseopt: convert --exclude
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (8 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 09/19] rev-parseopt: convert --glob Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 11/19] rev-parseopt: convert --reflog Nguyễn Thái Ngọc Duy
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/revision.c b/revision.c
index 013b8ec19f..d34e17984d 100644
--- a/revision.c
+++ b/revision.c
@@ -2428,6 +2428,16 @@ static int rev_opt_branches(const struct option *opt,
 	return 0;
 }
 
+static int rev_opt_exclude(const struct option *opt,
+			   const char *arg, int unset)
+{
+	struct rev_info	*revs  = opt->value;
+
+	BUG_ON_OPT_NEG(unset);
+	add_ref_exclusion(&revs->ref_excludes, arg);
+	return 0;
+}
+
 static int rev_opt_glob(const struct option *opt,
 			const char *arg, int unset)
 {
@@ -2515,6 +2525,9 @@ static void make_pseudo_options(struct rev_info *revs)
 		OPT_REV(0, "glob", N_("<pattern>"),
 			N_("include all refs matching shell glob"),
 			rev_opt_glob),
+		OPT_REV(0, "exclude", N_("<pattern>"),
+			N_("exclude refs matching glob pattern"),
+			rev_opt_exclude),
 		OPT_END()
 	};
 	ALLOC_ARRAY(revs->pseudo_options, ARRAY_SIZE(options));
@@ -2528,7 +2541,6 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	const char *arg = argv[0];
 	const char *optarg;
 	struct ref_store *refs;
-	int argcount;
 
 	if (submodule) {
 		/*
@@ -2555,10 +2567,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (argc)
 		return argc;
 
-	if ((argcount = parse_long_opt("exclude", argv, &optarg))) {
-		add_ref_exclusion(&revs->ref_excludes, optarg);
-		return argcount;
-	} else if (!strcmp(arg, "--reflog")) {
+	if (!strcmp(arg, "--reflog")) {
 		add_reflogs_to_pending(revs, *flags);
 	} else if (!strcmp(arg, "--indexed-objects")) {
 		add_index_objects_to_pending(revs, *flags);
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 11/19] rev-parseopt: convert --reflog
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (9 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 10/19] rev-parseopt: convert --exclude Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 12/19] rev-parseopt: convert --indexed-objects Nguyễn Thái Ngọc Duy
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/revision.c b/revision.c
index d34e17984d..6efa9bee1e 100644
--- a/revision.c
+++ b/revision.c
@@ -2452,6 +2452,18 @@ static int rev_opt_glob(const struct option *opt,
 	return 0;
 }
 
+static int rev_opt_reflog(const struct option *opt,
+			  const char *arg, int unset)
+{
+	struct rev_info *revs  = opt->value;
+	int flags = *revs->pseudo_flags;
+
+	BUG_ON_OPT_NEG(unset);
+	BUG_ON_OPT_ARG(arg);
+	add_reflogs_to_pending(revs, flags);
+	return 0;
+}
+
 static int rev_opt_remotes(const struct option *opt,
 			   const char *arg, int unset)
 {
@@ -2528,6 +2540,9 @@ static void make_pseudo_options(struct rev_info *revs)
 		OPT_REV(0, "exclude", N_("<pattern>"),
 			N_("exclude refs matching glob pattern"),
 			rev_opt_exclude),
+		OPT_REV_NOARG(0, "reflog",
+			      N_("include all refs from reflog"),
+			      rev_opt_reflog),
 		OPT_END()
 	};
 	ALLOC_ARRAY(revs->pseudo_options, ARRAY_SIZE(options));
@@ -2567,9 +2582,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (argc)
 		return argc;
 
-	if (!strcmp(arg, "--reflog")) {
-		add_reflogs_to_pending(revs, *flags);
-	} else if (!strcmp(arg, "--indexed-objects")) {
+	if (!strcmp(arg, "--indexed-objects")) {
 		add_index_objects_to_pending(revs, *flags);
 	} else if (!strcmp(arg, "--not")) {
 		*flags ^= UNINTERESTING | BOTTOM;
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 12/19] rev-parseopt: convert --indexed-objects
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (10 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 11/19] rev-parseopt: convert --reflog Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 13/19] rev-parseopt: convert --not Nguyễn Thái Ngọc Duy
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/revision.c b/revision.c
index 6efa9bee1e..9cfa4dc151 100644
--- a/revision.c
+++ b/revision.c
@@ -2438,6 +2438,18 @@ static int rev_opt_exclude(const struct option *opt,
 	return 0;
 }
 
+static int rev_opt_indexed_objects(const struct option *opt,
+				   const char *arg, int unset)
+{
+	struct rev_info *revs  = opt->value;
+	int flags = *revs->pseudo_flags;
+
+	BUG_ON_OPT_NEG(unset);
+	BUG_ON_OPT_ARG(arg);
+	add_index_objects_to_pending(revs, flags);
+	return 0;
+}
+
 static int rev_opt_glob(const struct option *opt,
 			const char *arg, int unset)
 {
@@ -2543,6 +2555,9 @@ static void make_pseudo_options(struct rev_info *revs)
 		OPT_REV_NOARG(0, "reflog",
 			      N_("include all refs from reflog"),
 			      rev_opt_reflog),
+		OPT_REV_NOARG(0, "indexed-objects",
+			      N_("include all trees and blobs used by the index"),
+			      rev_opt_indexed_objects),
 		OPT_END()
 	};
 	ALLOC_ARRAY(revs->pseudo_options, ARRAY_SIZE(options));
@@ -2582,9 +2597,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (argc)
 		return argc;
 
-	if (!strcmp(arg, "--indexed-objects")) {
-		add_index_objects_to_pending(revs, *flags);
-	} else if (!strcmp(arg, "--not")) {
+	if (!strcmp(arg, "--not")) {
 		*flags ^= UNINTERESTING | BOTTOM;
 	} else if (!strcmp(arg, "--no-walk")) {
 		revs->no_walk = REVISION_WALK_NO_WALK_SORTED;
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 13/19] rev-parseopt: convert --not
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (11 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 12/19] rev-parseopt: convert --indexed-objects Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 14/19] rev-parseopt: convert --no-walk and --do-walk Nguyễn Thái Ngọc Duy
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/revision.c b/revision.c
index 9cfa4dc151..b02cb4660b 100644
--- a/revision.c
+++ b/revision.c
@@ -2464,6 +2464,17 @@ static int rev_opt_glob(const struct option *opt,
 	return 0;
 }
 
+static int rev_opt_not(const struct option *opt,
+		       const char *arg, int unset)
+{
+	struct rev_info *revs  = opt->value;
+
+	BUG_ON_OPT_NEG(unset);
+	BUG_ON_OPT_ARG(arg);
+	*revs->pseudo_flags ^= UNINTERESTING | BOTTOM;
+	return 0;
+}
+
 static int rev_opt_reflog(const struct option *opt,
 			  const char *arg, int unset)
 {
@@ -2558,6 +2569,9 @@ static void make_pseudo_options(struct rev_info *revs)
 		OPT_REV_NOARG(0, "indexed-objects",
 			      N_("include all trees and blobs used by the index"),
 			      rev_opt_indexed_objects),
+		OPT_REV_NOARG(0, "not",
+			      N_("reverse the meaning of '^' for all following revisions"),
+			      rev_opt_not),
 		OPT_END()
 	};
 	ALLOC_ARRAY(revs->pseudo_options, ARRAY_SIZE(options));
@@ -2597,9 +2611,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (argc)
 		return argc;
 
-	if (!strcmp(arg, "--not")) {
-		*flags ^= UNINTERESTING | BOTTOM;
-	} else if (!strcmp(arg, "--no-walk")) {
+	if (!strcmp(arg, "--no-walk")) {
 		revs->no_walk = REVISION_WALK_NO_WALK_SORTED;
 	} else if (skip_prefix(arg, "--no-walk=", &optarg)) {
 		/*
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 14/19] rev-parseopt: convert --no-walk and --do-walk
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (12 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 13/19] rev-parseopt: convert --not Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 15/19] rev-parseopt: convert --single-worktree Nguyễn Thái Ngọc Duy
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 47 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/revision.c b/revision.c
index b02cb4660b..f04eb7f140 100644
--- a/revision.c
+++ b/revision.c
@@ -2464,6 +2464,29 @@ static int rev_opt_glob(const struct option *opt,
 	return 0;
 }
 
+static int rev_opt_no_walk(const struct option *opt,
+			   const char *arg, int unset)
+{
+	struct rev_info *revs  = opt->value;
+
+	BUG_ON_OPT_NEG(unset);
+	if (!arg) {
+		revs->no_walk = REVISION_WALK_NO_WALK_SORTED;
+	} else {
+		/*
+		 * Detached form ("--no-walk X" as opposed to "--no-walk=X")
+		 * not allowed, since the argument is optional.
+		 */
+		if (!strcmp(arg, "sorted"))
+			revs->no_walk = REVISION_WALK_NO_WALK_SORTED;
+		else if (!strcmp(arg, "unsorted"))
+			revs->no_walk = REVISION_WALK_NO_WALK_UNSORTED;
+		else
+			return error(_("invalid argument to --no-walk"));
+	}
+	return 0;
+}
+
 static int rev_opt_not(const struct option *opt,
 		       const char *arg, int unset)
 {
@@ -2572,6 +2595,12 @@ static void make_pseudo_options(struct rev_info *revs)
 		OPT_REV_NOARG(0, "not",
 			      N_("reverse the meaning of '^' for all following revisions"),
 			      rev_opt_not),
+		OPT_REV_OPTARG(0, "no-walk", N_("(sorted|unsorted)"),
+			       N_("only show given commits but do not traverse their ancestors"),
+			       rev_opt_no_walk),
+		OPT_SET_INT_F(0, "do-walk", &revs->no_walk,
+			      N_("override a previous --no-walk"),
+			      0, PARSE_OPT_NONEG),
 		OPT_END()
 	};
 	ALLOC_ARRAY(revs->pseudo_options, ARRAY_SIZE(options));
@@ -2583,7 +2612,6 @@ static int handle_revision_pseudo_opt(const char *submodule,
 				int argc, const char **argv, int *flags)
 {
 	const char *arg = argv[0];
-	const char *optarg;
 	struct ref_store *refs;
 
 	if (submodule) {
@@ -2611,22 +2639,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (argc)
 		return argc;
 
-	if (!strcmp(arg, "--no-walk")) {
-		revs->no_walk = REVISION_WALK_NO_WALK_SORTED;
-	} else if (skip_prefix(arg, "--no-walk=", &optarg)) {
-		/*
-		 * Detached form ("--no-walk X" as opposed to "--no-walk=X")
-		 * not allowed, since the argument is optional.
-		 */
-		if (!strcmp(optarg, "sorted"))
-			revs->no_walk = REVISION_WALK_NO_WALK_SORTED;
-		else if (!strcmp(optarg, "unsorted"))
-			revs->no_walk = REVISION_WALK_NO_WALK_UNSORTED;
-		else
-			return error("invalid argument to --no-walk");
-	} else if (!strcmp(arg, "--do-walk")) {
-		revs->no_walk = 0;
-	} else if (!strcmp(arg, "--single-worktree")) {
+	if (!strcmp(arg, "--single-worktree")) {
 		revs->single_worktree = 1;
 	} else {
 		return 0;
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 15/19] rev-parseopt: convert --single-worktree
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (13 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 14/19] rev-parseopt: convert --no-walk and --do-walk Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 16/19] rev-parseopt: prepare to convert handle_revision_opt() Nguyễn Thái Ngọc Duy
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/revision.c b/revision.c
index f04eb7f140..dd22ac5c39 100644
--- a/revision.c
+++ b/revision.c
@@ -2601,6 +2601,9 @@ static void make_pseudo_options(struct rev_info *revs)
 		OPT_SET_INT_F(0, "do-walk", &revs->no_walk,
 			      N_("override a previous --no-walk"),
 			      0, PARSE_OPT_NONEG),
+		OPT_BOOL_F(0, "single-worktree", &revs->single_worktree,
+			   N_("only consider refs from the current worktree"),
+			   PARSE_OPT_NONEG),
 		OPT_END()
 	};
 	ALLOC_ARRAY(revs->pseudo_options, ARRAY_SIZE(options));
@@ -2611,7 +2614,6 @@ static int handle_revision_pseudo_opt(const char *submodule,
 				struct rev_info *revs,
 				int argc, const char **argv, int *flags)
 {
-	const char *arg = argv[0];
 	struct ref_store *refs;
 
 	if (submodule) {
@@ -2636,16 +2638,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 			     PARSE_OPT_NO_INTERNAL_HELP |
 			     PARSE_OPT_ONE_SHOT |
 			     PARSE_OPT_STOP_AT_NON_OPTION);
-	if (argc)
-		return argc;
-
-	if (!strcmp(arg, "--single-worktree")) {
-		revs->single_worktree = 1;
-	} else {
-		return 0;
-	}
-
-	return 1;
+	return argc;
 }
 
 static void NORETURN diagnose_missing_default(const char *def)
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 16/19] rev-parseopt: prepare to convert handle_revision_opt()
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (14 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 15/19] rev-parseopt: convert --single-worktree Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 17/19] rev-parseopt: convert --max-count Nguyễn Thái Ngọc Duy
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Besides preparing an empty option table to be added in. The table is
also concatenated with diff option table so we don't need
diff_opt_parse() anymore. Merging with pseudo-opt table though will
not happen until we kill parse_revision_opt().

--abbrev has to be converted right away to override the same one from
the diff option parser (which runs first now, if not overriden)

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/technical/api-diff.txt |  6 +--
 diff.c                               | 16 -------
 diff.h                               |  1 -
 revision.c                           | 64 +++++++++++++++++++++-------
 revision.h                           |  1 +
 5 files changed, 53 insertions(+), 35 deletions(-)

diff --git a/Documentation/technical/api-diff.txt b/Documentation/technical/api-diff.txt
index 30fc0e9c93..1e4e6968f4 100644
--- a/Documentation/technical/api-diff.txt
+++ b/Documentation/technical/api-diff.txt
@@ -22,9 +22,9 @@ Calling sequence
   sets up the vanilla default.
 
 * Fill in the options structure to specify desired output format, rename
-  detection, etc.  `diff_opt_parse()` can be used to parse options given
-  from the command line in a way consistent with existing git-diff
-  family of programs.
+  detection, etc. `parseopts[]` can be used with parse_options() to
+  parse options from the command line in a way consistent with
+  existing git-diff family of programs.
 
 * Call `diff_setup_done()`; this inspects the options set up so far for
   internal consistency and make necessary tweaking to it (e.g. if
diff --git a/diff.c b/diff.c
index 4d3cf83a27..ef0eb2a160 100644
--- a/diff.c
+++ b/diff.c
@@ -5522,22 +5522,6 @@ static void prep_parse_options(struct diff_options *options)
 	memcpy(options->parseopts, parseopts, sizeof(parseopts));
 }
 
-int diff_opt_parse(struct diff_options *options,
-		   const char **av, int ac, const char *prefix)
-{
-	if (!prefix)
-		prefix = "";
-
-	ac = parse_options(ac, av, prefix, options->parseopts, NULL,
-			   PARSE_OPT_KEEP_DASHDASH |
-			   PARSE_OPT_KEEP_UNKNOWN |
-			   PARSE_OPT_NO_INTERNAL_HELP |
-			   PARSE_OPT_ONE_SHOT |
-			   PARSE_OPT_STOP_AT_NON_OPTION);
-
-	return ac;
-}
-
 int parse_rename_score(const char **cp_p)
 {
 	unsigned long num, scale;
diff --git a/diff.h b/diff.h
index b20cbcc091..c75480a998 100644
--- a/diff.h
+++ b/diff.h
@@ -351,7 +351,6 @@ int git_diff_ui_config(const char *var, const char *value, void *cb);
 #define diff_setup(diffopts) repo_diff_setup(the_repository, diffopts)
 #endif
 void repo_diff_setup(struct repository *, struct diff_options *);
-int diff_opt_parse(struct diff_options *, const char **, int, const char *);
 void diff_setup_done(struct diff_options *);
 int git_config_rename(const char *var, const char *value);
 
diff --git a/revision.c b/revision.c
index dd22ac5c39..f15aa3e62d 100644
--- a/revision.c
+++ b/revision.c
@@ -1611,6 +1611,7 @@ static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,
 }
 
 static void make_pseudo_options(struct rev_info *revs);
+static void make_rev_options(struct rev_info *revs);
 
 void repo_init_revisions(struct repository *r,
 			 struct rev_info *revs,
@@ -1653,6 +1654,7 @@ void repo_init_revisions(struct repository *r,
 
 	revs->notes_opt.use_default_notes = -1;
 	make_pseudo_options(revs);
+	make_rev_options(revs);
 }
 
 static void add_pending_commit_list(struct rev_info *revs,
@@ -1953,6 +1955,38 @@ static void add_message_grep(struct rev_info *revs, const char *pattern)
 	add_grep(revs, pattern, GREP_PATTERN_BODY);
 }
 
+static int rev_opt_abbrev(const struct option *opt,
+			  const char *optarg, int unset)
+{
+	struct rev_info *revs = opt->value;
+
+	if (unset) {
+		revs->abbrev = 0;
+	} else if (!optarg) {
+		revs->abbrev = DEFAULT_ABBREV;
+	} else {
+		const unsigned hexsz = the_hash_algo->hexsz;
+
+		revs->abbrev = strtoul(optarg, NULL, 10);
+		if (revs->abbrev < MINIMUM_ABBREV)
+			revs->abbrev = MINIMUM_ABBREV;
+		else if (revs->abbrev > hexsz)
+			revs->abbrev = hexsz;
+	}
+	return 0;
+}
+
+static void make_rev_options(struct rev_info *revs)
+{
+	struct option options[] = {
+		OPT_CALLBACK_F(0, "abbrev", revs, N_("n"),
+			       N_("show the given source prefix instead of \"a/\""),
+			       PARSE_OPT_OPTARG, rev_opt_abbrev),
+		OPT_END(),
+	};
+	revs->options = parse_options_concat(options, revs->diffopt.parseopts);
+}
+
 static int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
 			       int *unkc, const char **unkv,
 			       const struct setup_revision_opt* opt)
@@ -1960,7 +1994,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	const char *arg = argv[0];
 	const char *optarg;
 	int argcount;
-	const unsigned hexsz = the_hash_algo->hexsz;
 
 	/* pseudo revision arguments */
 	if (!strcmp(arg, "--all") || !strcmp(arg, "--branches") ||
@@ -1977,6 +2010,18 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		return 1;
 	}
 
+	revs->pseudo_flags = NULL;
+	revs->pseudo_refs = NULL;
+	argc = parse_options(argc, argv, revs->prefix,
+			     revs->options, NULL,
+			     PARSE_OPT_KEEP_DASHDASH |
+			     PARSE_OPT_KEEP_UNKNOWN |
+			     PARSE_OPT_NO_INTERNAL_HELP |
+			     PARSE_OPT_ONE_SHOT |
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+	if (argc)
+		return argc;
+
 	if ((argcount = parse_long_opt("max-count", argv, &optarg))) {
 		revs->max_count = atoi(optarg);
 		revs->no_walk = 0;
@@ -2243,16 +2288,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->no_commit_id = 1;
 	} else if (!strcmp(arg, "--always")) {
 		revs->always_show_header = 1;
-	} else if (!strcmp(arg, "--no-abbrev")) {
-		revs->abbrev = 0;
-	} else if (!strcmp(arg, "--abbrev")) {
-		revs->abbrev = DEFAULT_ABBREV;
-	} else if (skip_prefix(arg, "--abbrev=", &optarg)) {
-		revs->abbrev = strtoul(optarg, NULL, 10);
-		if (revs->abbrev < MINIMUM_ABBREV)
-			revs->abbrev = MINIMUM_ABBREV;
-		else if (revs->abbrev > hexsz)
-			revs->abbrev = hexsz;
 	} else if (!strcmp(arg, "--abbrev-commit")) {
 		revs->abbrev_commit = 1;
 		revs->abbrev_commit_given = 1;
@@ -2324,10 +2359,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 			BUG("exclude_promisor_objects can only be used when fetch_if_missing is 0");
 		revs->exclude_promisor_objects = 1;
 	} else {
-		int opts = diff_opt_parse(&revs->diffopt, argv, argc, revs->prefix);
-		if (!opts)
-			unkv[(*unkc)++] = arg;
-		return opts;
+		unkv[(*unkc)++] = arg;
+		return 0;
 	}
 	if (revs->graph && revs->track_linear)
 		die("--show-linear-break and --graph are incompatible");
@@ -2861,6 +2894,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 		revs->expand_tabs_in_log = revs->expand_tabs_in_log_default;
 
 	FREE_AND_NULL(revs->pseudo_options);
+	FREE_AND_NULL(revs->options);
 	return left;
 }
 
diff --git a/revision.h b/revision.h
index cec5215c04..4e840c8eb1 100644
--- a/revision.h
+++ b/revision.h
@@ -283,6 +283,7 @@ struct rev_info {
 	struct option *pseudo_options;
 	int *pseudo_flags;
 	struct ref_store *pseudo_refs;
+	struct option *options;
 };
 
 int ref_excluded(struct string_list *, const char *path);
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 17/19] rev-parseopt: convert --max-count
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (15 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 16/19] rev-parseopt: prepare to convert handle_revision_opt() Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 18/19] rev-parseopt: convert --skip Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 19/19] rev-parseopt: convert --min-age and --max-age Nguyễn Thái Ngọc Duy
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/revision.c b/revision.c
index f15aa3e62d..c53347d362 100644
--- a/revision.c
+++ b/revision.c
@@ -1976,12 +1976,26 @@ static int rev_opt_abbrev(const struct option *opt,
 	return 0;
 }
 
+static int rev_opt_max_count(const struct option *opt,
+			     const char *arg, int unset)
+{
+	struct rev_info *revs  = opt->value;
+
+	BUG_ON_OPT_NEG(unset);
+	revs->max_count = atoi(arg);
+	revs->no_walk = 0;
+	return 0;
+}
+
 static void make_rev_options(struct rev_info *revs)
 {
 	struct option options[] = {
 		OPT_CALLBACK_F(0, "abbrev", revs, N_("n"),
 			       N_("show the given source prefix instead of \"a/\""),
 			       PARSE_OPT_OPTARG, rev_opt_abbrev),
+		OPT_REV('n', "max-count", N_("<n>"),
+			N_("limit the number of commis to stdout"),
+			rev_opt_max_count),
 		OPT_END(),
 	};
 	revs->options = parse_options_concat(options, revs->diffopt.parseopts);
@@ -2022,11 +2036,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	if (argc)
 		return argc;
 
-	if ((argcount = parse_long_opt("max-count", argv, &optarg))) {
-		revs->max_count = atoi(optarg);
-		revs->no_walk = 0;
-		return argcount;
-	} else if ((argcount = parse_long_opt("skip", argv, &optarg))) {
+	if ((argcount = parse_long_opt("skip", argv, &optarg))) {
 		revs->skip_count = atoi(optarg);
 		return argcount;
 	} else if ((*arg == '-') && isdigit(arg[1])) {
@@ -2035,15 +2045,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		    revs->max_count < 0)
 			die("'%s': not a non-negative integer", arg + 1);
 		revs->no_walk = 0;
-	} else if (!strcmp(arg, "-n")) {
-		if (argc <= 1)
-			return error("-n requires an argument");
-		revs->max_count = atoi(argv[1]);
-		revs->no_walk = 0;
-		return 2;
-	} else if (skip_prefix(arg, "-n", &optarg)) {
-		revs->max_count = atoi(optarg);
-		revs->no_walk = 0;
 	} else if ((argcount = parse_long_opt("max-age", argv, &optarg))) {
 		revs->max_age = atoi(optarg);
 		return argcount;
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 18/19] rev-parseopt: convert --skip
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (16 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 17/19] rev-parseopt: convert --max-count Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  2019-05-08 11:12 ` [PATCH 19/19] rev-parseopt: convert --min-age and --max-age Nguyễn Thái Ngọc Duy
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/revision.c b/revision.c
index c53347d362..42d466cd08 100644
--- a/revision.c
+++ b/revision.c
@@ -1996,6 +1996,9 @@ static void make_rev_options(struct rev_info *revs)
 		OPT_REV('n', "max-count", N_("<n>"),
 			N_("limit the number of commis to stdout"),
 			rev_opt_max_count),
+		OPT_INTEGER_F(0, "skip", &revs->skip_count,
+			      N_("skip a number of commits before starting to show"),
+			      PARSE_OPT_NONEG),
 		OPT_END(),
 	};
 	revs->options = parse_options_concat(options, revs->diffopt.parseopts);
@@ -2036,10 +2039,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	if (argc)
 		return argc;
 
-	if ((argcount = parse_long_opt("skip", argv, &optarg))) {
-		revs->skip_count = atoi(optarg);
-		return argcount;
-	} else if ((*arg == '-') && isdigit(arg[1])) {
+	if ((*arg == '-') && isdigit(arg[1])) {
 		/* accept -<digit>, like traditional "head" */
 		if (strtol_i(arg + 1, 10, &revs->max_count) < 0 ||
 		    revs->max_count < 0)
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 19/19] rev-parseopt: convert --min-age and --max-age
  2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
                   ` (17 preceding siblings ...)
  2019-05-08 11:12 ` [PATCH 18/19] rev-parseopt: convert --skip Nguyễn Thái Ngọc Duy
@ 2019-05-08 11:12 ` Nguyễn Thái Ngọc Duy
  18 siblings, 0 replies; 25+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2019-05-08 11:12 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 parse-options-cb.c |  8 ++++++++
 parse-options.h    |  4 ++++
 revision.c         | 10 ++++------
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/parse-options-cb.c b/parse-options-cb.c
index 6e2e8d6273..7cdbbf5f6d 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -39,6 +39,14 @@ int parse_opt_expiry_date_cb(const struct option *opt, const char *arg,
 	return 0;
 }
 
+int parse_opt_timestamp_cb(const struct option *opt,
+			   const char *arg, int unset)
+{
+	BUG_ON_OPT_NEG(unset);
+	*(timestamp_t *)opt->value = atoi(arg);
+	return 0;
+}
+
 int parse_opt_color_flag_cb(const struct option *opt, const char *arg,
 			    int unset)
 {
diff --git a/parse-options.h b/parse-options.h
index cc9230adac..7637864c41 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -168,6 +168,9 @@ struct option {
 #define OPT_EXPIRY_DATE(s, l, v, h) \
 	{ OPTION_CALLBACK, (s), (l), (v), N_("expiry-date"),(h), 0,	\
 	  parse_opt_expiry_date_cb }
+#define OPT_TIMESTAMP(s, l, v, h) \
+	{ OPTION_CALLBACK, (s), (l), (v), N_("timestamp"),(h), \
+	  PARSE_OPT_NONEG, parse_opt_timestamp_cb }
 #define OPT_CALLBACK(s, l, v, a, h, f) OPT_CALLBACK_F(s, l, v, a, h, 0, f)
 #define OPT_NUMBER_CALLBACK(v, h, f) \
 	{ OPTION_NUMBER, 0, NULL, (v), NULL, (h), \
@@ -275,6 +278,7 @@ struct option *parse_options_concat(struct option *a, struct option *b);
 /*----- some often used options -----*/
 int parse_opt_abbrev_cb(const struct option *, const char *, int);
 int parse_opt_expiry_date_cb(const struct option *, const char *, int);
+int parse_opt_timestamp_cb(const struct option *, const char *, int);
 int parse_opt_color_flag_cb(const struct option *, const char *, int);
 int parse_opt_verbosity_cb(const struct option *, const char *, int);
 int parse_opt_object_name(const struct option *, const char *, int);
diff --git a/revision.c b/revision.c
index 42d466cd08..0c28b67978 100644
--- a/revision.c
+++ b/revision.c
@@ -1999,6 +1999,10 @@ static void make_rev_options(struct rev_info *revs)
 		OPT_INTEGER_F(0, "skip", &revs->skip_count,
 			      N_("skip a number of commits before starting to show"),
 			      PARSE_OPT_NONEG),
+		OPT_TIMESTAMP(0, "min-age", &revs->min_age,
+			      N_("limit the commits output to a specified time range")),
+		OPT_TIMESTAMP(0, "max-age", &revs->max_age,
+			      N_("limit the commits output to a specified time range")),
 		OPT_END(),
 	};
 	revs->options = parse_options_concat(options, revs->diffopt.parseopts);
@@ -2045,18 +2049,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		    revs->max_count < 0)
 			die("'%s': not a non-negative integer", arg + 1);
 		revs->no_walk = 0;
-	} else if ((argcount = parse_long_opt("max-age", argv, &optarg))) {
-		revs->max_age = atoi(optarg);
-		return argcount;
 	} else if ((argcount = parse_long_opt("since", argv, &optarg))) {
 		revs->max_age = approxidate(optarg);
 		return argcount;
 	} else if ((argcount = parse_long_opt("after", argv, &optarg))) {
 		revs->max_age = approxidate(optarg);
 		return argcount;
-	} else if ((argcount = parse_long_opt("min-age", argv, &optarg))) {
-		revs->min_age = atoi(optarg);
-		return argcount;
 	} else if ((argcount = parse_long_opt("before", argv, &optarg))) {
 		revs->min_age = approxidate(optarg);
 		return argcount;
-- 
2.21.0.1141.gd54ac2cb17


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/19] revision.h: avoid bit fields in struct rev_info
  2019-05-08 11:12 ` [PATCH 01/19] revision.h: avoid bit fields in struct rev_info Nguyễn Thái Ngọc Duy
@ 2019-05-08 14:07   ` Derrick Stolee
  2019-05-08 14:41     ` Duy Nguyen
  0 siblings, 1 reply; 25+ messages in thread
From: Derrick Stolee @ 2019-05-08 14:07 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy, git

On 5/8/2019 7:12 AM, Nguyễn Thái Ngọc Duy wrote:
> Bitfield addresses cannot be passed around in a pointer. This makes it
> hard to use parse-options to set/unset them. Turn this struct to
> normal integers. This of course increases the size of this struct
> multiple times, but since we only have a handful of rev_info variables
> around, memory consumption is not at all a concern.

I think you are right that this memory trade-off shouldn't be a problem.

What worries me instead is that we are using an "internal" data structure
for option parsing. Would it make more sense to create a struct for use
in the parse_opts method and a method that translates those options into
the bitfield in struct rev_info?

Thanks,
-Stolee

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/19] revision.h: avoid bit fields in struct rev_info
  2019-05-08 14:07   ` Derrick Stolee
@ 2019-05-08 14:41     ` Duy Nguyen
  2019-05-08 15:52       ` Derrick Stolee
  0 siblings, 1 reply; 25+ messages in thread
From: Duy Nguyen @ 2019-05-08 14:41 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: Git Mailing List

On Wed, May 8, 2019 at 9:07 PM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 5/8/2019 7:12 AM, Nguyễn Thái Ngọc Duy wrote:
> > Bitfield addresses cannot be passed around in a pointer. This makes it
> > hard to use parse-options to set/unset them. Turn this struct to
> > normal integers. This of course increases the size of this struct
> > multiple times, but since we only have a handful of rev_info variables
> > around, memory consumption is not at all a concern.
>
> I think you are right that this memory trade-off shouldn't be a problem.
>
> What worries me instead is that we are using an "internal" data structure
> for option parsing. Would it make more sense to create a struct for use
> in the parse_opts method and a method that translates those options into
> the bitfield in struct rev_info?

But we are doing that now (option parsing) using the same data
structure. Why would changing from a custom parser to parse_options()
affect what fields it should or should not touch in rev_info? Genuine
question. Maybe you could elaborate more about "internal". I probably
missed something. Or maybe this is a good opportunity to separate
intermediate option parsing variables from the rest of rev_info?
-- 
Duy

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/19] revision.h: avoid bit fields in struct rev_info
  2019-05-08 14:41     ` Duy Nguyen
@ 2019-05-08 15:52       ` Derrick Stolee
  2019-05-09  9:56         ` Duy Nguyen
  0 siblings, 1 reply; 25+ messages in thread
From: Derrick Stolee @ 2019-05-08 15:52 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List

On 5/8/2019 10:41 AM, Duy Nguyen wrote:
> On Wed, May 8, 2019 at 9:07 PM Derrick Stolee <stolee@gmail.com> wrote:
>>
>> On 5/8/2019 7:12 AM, Nguyễn Thái Ngọc Duy wrote:
>>> Bitfield addresses cannot be passed around in a pointer. This makes it
>>> hard to use parse-options to set/unset them. Turn this struct to
>>> normal integers. This of course increases the size of this struct
>>> multiple times, but since we only have a handful of rev_info variables
>>> around, memory consumption is not at all a concern.
>>
>> I think you are right that this memory trade-off shouldn't be a problem.
>>
>> What worries me instead is that we are using an "internal" data structure
>> for option parsing. Would it make more sense to create a struct for use
>> in the parse_opts method and a method that translates those options into
>> the bitfield in struct rev_info?
> 
> But we are doing that now (option parsing) using the same data
> structure. Why would changing from a custom parser to parse_options()
> affect what fields it should or should not touch in rev_info? Genuine
> question. Maybe you could elaborate more about "internal". I probably
> missed something. Or maybe this is a good opportunity to separate
> intermediate option parsing variables from the rest of rev_info?

You're right. I was unclear.

rev_info stores a lot of data. Some of the fields are important
in-memory structures that are crucial to the workings of revision.c
and are never used by builtin/rev-list.c. Combining this purpose
with the option parsing seems smelly to me.

Thinking more on it, I would prefer a more invasive change that may
pay off in the long term. These options, along with the "starting list"
values, could be extracted to a 'struct rev_options' that contains these
integers and the commit list. Then your option parsing changes could be
limited to a rev_options struct, which is later inserted into a rev_info
struct during setup_revisions().

Generally, the rev_info struct has too many members and could be split
into smaller pieces according to purpose. I created the topo_walk_info
struct as a way to not make the situation worse, but doesn't fix existing
pain.

My ramblings are mostly complaining about old code that grew organically
across many many quality additions. It is definitely hard to understand
the revision-walking code, and perhaps it would be easier to understand
with a little more structure.

The biggest issue with my suggestion is that it requires changing the
consumers of the options, as they would no longer live directly on the
rev_info struct. That would be a big change, even if it could be done
with string replacement.

Thanks,
-Stolee 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/19] revision.h: avoid bit fields in struct rev_info
  2019-05-08 15:52       ` Derrick Stolee
@ 2019-05-09  9:56         ` Duy Nguyen
  2019-05-09 12:11           ` Derrick Stolee
  0 siblings, 1 reply; 25+ messages in thread
From: Duy Nguyen @ 2019-05-09  9:56 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: Git Mailing List

On Wed, May 8, 2019 at 10:52 PM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 5/8/2019 10:41 AM, Duy Nguyen wrote:
> > On Wed, May 8, 2019 at 9:07 PM Derrick Stolee <stolee@gmail.com> wrote:
> >>
> >> On 5/8/2019 7:12 AM, Nguyễn Thái Ngọc Duy wrote:
> >>> Bitfield addresses cannot be passed around in a pointer. This makes it
> >>> hard to use parse-options to set/unset them. Turn this struct to
> >>> normal integers. This of course increases the size of this struct
> >>> multiple times, but since we only have a handful of rev_info variables
> >>> around, memory consumption is not at all a concern.
> >>
> >> I think you are right that this memory trade-off shouldn't be a problem.
> >>
> >> What worries me instead is that we are using an "internal" data structure
> >> for option parsing. Would it make more sense to create a struct for use
> >> in the parse_opts method and a method that translates those options into
> >> the bitfield in struct rev_info?
> >
> > But we are doing that now (option parsing) using the same data
> > structure. Why would changing from a custom parser to parse_options()
> > affect what fields it should or should not touch in rev_info? Genuine
> > question. Maybe you could elaborate more about "internal". I probably
> > missed something. Or maybe this is a good opportunity to separate
> > intermediate option parsing variables from the rest of rev_info?
>
> You're right. I was unclear.
>
> rev_info stores a lot of data. Some of the fields are important
> in-memory structures that are crucial to the workings of revision.c
> and are never used by builtin/rev-list.c. Combining this purpose
> with the option parsing seems smelly to me.
>
> Thinking more on it, I would prefer a more invasive change that may
> pay off in the long term. These options, along with the "starting list"
> values, could be extracted to a 'struct rev_options' that contains these
> integers and the commit list. Then your option parsing changes could be
> limited to a rev_options struct, which is later inserted into a rev_info
> struct during setup_revisions().
>
> Generally, the rev_info struct has too many members and could be split
> into smaller pieces according to purpose. I created the topo_walk_info
> struct as a way to not make the situation worse, but doesn't fix existing
> pain.
>
> My ramblings are mostly complaining about old code that grew organically
> across many many quality additions. It is definitely hard to understand
> the revision-walking code, and perhaps it would be easier to understand
> with a little more structure.

Thanks. This is much better.

> The biggest issue with my suggestion is that it requires changing the
> consumers of the options, as they would no longer live directly on the
> rev_info struct. That would be a big change, even if it could be done
> with string replacement.

I agree rev_info has grown "wild". It's quite ancient code. As you
noted, it's a big change. And since my series is already long (76
patches), I would rather focus on just one thing, rewriting the
parsing code with minimum changes to anything else, preferably retain
the exact same old behavior.

After that work is done (and no regression found), we could focus on
reorganizing rev_info, which could be quite "interesting". Some fields
may be overloaded with different purposes, which I just can't spend
time investigating now. There's also the problem with freeing
resources after rev-list is done, which I think we have ignored so
far.
-- 
Duy

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/19] revision.h: avoid bit fields in struct rev_info
  2019-05-09  9:56         ` Duy Nguyen
@ 2019-05-09 12:11           ` Derrick Stolee
  0 siblings, 0 replies; 25+ messages in thread
From: Derrick Stolee @ 2019-05-09 12:11 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List

On 5/9/2019 5:56 AM, Duy Nguyen wrote:
> On Wed, May 8, 2019 at 10:52 PM Derrick Stolee <stolee@gmail.com> wrote:
>>
>> The biggest issue with my suggestion is that it requires changing the
>> consumers of the options, as they would no longer live directly on the
>> rev_info struct. That would be a big change, even if it could be done
>> with string replacement.
> 
> I agree rev_info has grown "wild". It's quite ancient code. As you
> noted, it's a big change. And since my series is already long (76
> patches), I would rather focus on just one thing, rewriting the
> parsing code with minimum changes to anything else, preferably retain
> the exact same old behavior.
> 
> After that work is done (and no regression found), we could focus on
> reorganizing rev_info, which could be quite "interesting". Some fields
> may be overloaded with different purposes, which I just can't spend
> time investigating now. There's also the problem with freeing
> resources after rev-list is done, which I think we have ignored so
> far.

Thanks for humoring me. I agree with your reasoning.

This series looks good to me.

Thanks,
-Stolee


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2019-05-09 12:11 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-08 11:12 [PATCH 00/19] Convert revision.c to parseopt part 1/4 Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 01/19] revision.h: avoid bit fields in struct rev_info Nguyễn Thái Ngọc Duy
2019-05-08 14:07   ` Derrick Stolee
2019-05-08 14:41     ` Duy Nguyen
2019-05-08 15:52       ` Derrick Stolee
2019-05-09  9:56         ` Duy Nguyen
2019-05-09 12:11           ` Derrick Stolee
2019-05-08 11:12 ` [PATCH 02/19] revision.h: move repo field down Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 03/19] revision.c: prepare to convert handle_revision_pseudo_opt() Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 04/19] rev-parseopt: convert --all Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 05/19] rev-parseopt: convert --branches Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 06/19] rev-parseopt: convert --bisect Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 07/19] rev-parseopt: convert --tags Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 08/19] rev-parseopt: convert --remotes Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 09/19] rev-parseopt: convert --glob Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 10/19] rev-parseopt: convert --exclude Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 11/19] rev-parseopt: convert --reflog Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 12/19] rev-parseopt: convert --indexed-objects Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 13/19] rev-parseopt: convert --not Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 14/19] rev-parseopt: convert --no-walk and --do-walk Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 15/19] rev-parseopt: convert --single-worktree Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 16/19] rev-parseopt: prepare to convert handle_revision_opt() Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 17/19] rev-parseopt: convert --max-count Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 18/19] rev-parseopt: convert --skip Nguyễn Thái Ngọc Duy
2019-05-08 11:12 ` [PATCH 19/19] rev-parseopt: convert --min-age and --max-age Nguyễn Thái Ngọc Duy

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).