diff mbox series

[v2,1/5] commit-graph: define common usage with a macro

Message ID patch-1.5-0b0bb04ecf5-20210718T074936Z-avarab@gmail.com (mailing list archive)
State Superseded
Headers show
Series commit-graph: usage fixes | expand

Commit Message

Ævar Arnfjörð Bjarmason July 18, 2021, 7:58 a.m. UTC 
Share the usage message between these three variables by using a
macro. Before this new options needed to copy/paste the usage
information, see e.g. 809e0327f5 (builtin/commit-graph.c: introduce
'--max-new-filters=<n>', 2020-09-18).

See b25b727494f (builtin/multi-pack-index.c: define common usage with
a macro, 2021-03-30) for another use of this pattern (but on-list this
one came first).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/commit-graph.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

Comments

Taylor Blau July 19, 2021, 4:29 p.m. UTC | #1 
On Sun, Jul 18, 2021 at 09:58:05AM +0200, Ævar Arnfjörð Bjarmason wrote:
> Share the usage message between these three variables by using a
> macro. Before this new options needed to copy/paste the usage
> information, see e.g. 809e0327f5 (builtin/commit-graph.c: introduce
> '--max-new-filters=<n>', 2020-09-18).
>
> See b25b727494f (builtin/multi-pack-index.c: define common usage with
> a macro, 2021-03-30) for another use of this pattern (but on-list this
> one came first).

I don't have a strong opinion, but I believe Jonathan Tan suggested that
we move the #define's out-of-line with their variable declarations back
in [1].

I think that what you wrote here is fine, but I tend to agree with
Jonathan that the version we ended up with in the tree is cleaner and a
little easier to read (albeit a few more lines).

[1]: https://lore.kernel.org/git/20210302040625.4035284-1-jonathantanmy@google.com/

Thanks,
Taylor
diff mbox series

Patch

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index cd863152216..c3fa4fde3e4 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -9,26 +9,27 @@ 
 #include "progress.h"
 #include "tag.h"
 
-static char const * const builtin_commit_graph_usage[] = {
-	N_("git commit-graph verify [--object-dir <objdir>] [--shallow] [--[no-]progress]"),
-	N_("git commit-graph write [--object-dir <objdir>] [--append] "
-	   "[--split[=<strategy>]] [--reachable|--stdin-packs|--stdin-commits] "
-	   "[--changed-paths] [--[no-]max-new-filters <n>] [--[no-]progress] "
-	   "<split options>"),
+static const char * builtin_commit_graph_verify_usage[] = {
+#define BUILTIN_COMMIT_GRAPH_VERIFY_USAGE \
+	N_("git commit-graph verify [--object-dir <objdir>] [--shallow] [--[no-]progress]")
+	BUILTIN_COMMIT_GRAPH_VERIFY_USAGE,
 	NULL
 };
 
-static const char * const builtin_commit_graph_verify_usage[] = {
-	N_("git commit-graph verify [--object-dir <objdir>] [--shallow] [--[no-]progress]"),
+static const char * builtin_commit_graph_write_usage[] = {
+#define BUILTIN_COMMIT_GRAPH_WRITE_USAGE \
+	N_("git commit-graph write [--object-dir <objdir>] [--append] " \
+	   "[--split[=<strategy>]] [--reachable|--stdin-packs|--stdin-commits] " \
+	   "[--changed-paths] [--[no-]max-new-filters <n>] [--[no-]progress] " \
+	   "<split options>")
+	BUILTIN_COMMIT_GRAPH_WRITE_USAGE,
 	NULL
 };
 
-static const char * const builtin_commit_graph_write_usage[] = {
-	N_("git commit-graph write [--object-dir <objdir>] [--append] "
-	   "[--split[=<strategy>]] [--reachable|--stdin-packs|--stdin-commits] "
-	   "[--changed-paths] [--[no-]max-new-filters <n>] [--[no-]progress] "
-	   "<split options>"),
-	NULL
+static char const * const builtin_commit_graph_usage[] = {
+	BUILTIN_COMMIT_GRAPH_VERIFY_USAGE,
+	BUILTIN_COMMIT_GRAPH_WRITE_USAGE,
+	NULL,
 };
 
 static struct opts_commit_graph {