More helpful pdnsutil help output #15082

miodvallat · 2025-01-23T14:42:47Z

Short description

This is another take at #12350 ("Group pdnsutil usage by topic"). The dictionary of pdnsutil commands is extended with a "command group" and the short help text, for every command. This allows these help texts to be output in groups, with alphabetical ordering within each group, automagically.

Doing this also exposes a few commands which lack help texts, these can either remain hidden or get gifted a nice help text (to be discussed in this PR).

The grouping of commands is ~~stolen~~ borrowed from #12350 and open for discussion as well. The changes in this PR allow for group assignment to be changed very easily, as well as new groups created, if deemed useful.

Checklist

I have:

read the CONTRIBUTING.md document
compiled this code
tested this code
included documentation (including possible behaviour changes)
documented the code
added or modified regression test(s)
added or modified unit test(s)

This extends the commands dictionary with a command group and the one-liner help message. When the list of these one-liners get output, they are now output in sorted order within each group.

coveralls · 2025-01-23T15:11:58Z

Pull Request Test Coverage Report for Build 12931605740

Details

0 of 0 changed or added relevant lines in 0 files are covered.
33 unchanged lines in 11 files lost coverage.
Overall coverage decreased (-0.004%) to 64.697%

Files with Coverage Reduction	New Missed Lines	%
modules/gpgsqlbackend/gpgsqlbackend.cc	1	88.62%
pdns/signingpipe.cc	1	85.25%
pdns/dnsdistdist/dnsdist-crypto.cc	1	76.01%
ext/json11/json11.cpp	1	64.49%
pdns/misc.cc	2	62.71%
pdns/remote_logger.cc	3	54.26%
modules/gpgsqlbackend/spgsql.cc	3	68.18%
pdns/recursordist/test-syncres_cc1.cc	5	89.87%
pdns/packethandler.cc	5	72.48%
pdns/recursordist/syncres.cc	5	79.86%

Totals
Change from base Build 12927190209:	-0.004%
Covered Lines:	127817
Relevant Lines:	166470

💛 - Coveralls

jsoref · 2025-02-10T15:04:47Z

pdns/pdnsutil.cc

+   "add-meta ZONE KIND VALUE           Add zone metadata, this adds to the existing KIND\n"
+   "                   [VALUE ...]"}},
+  {"add-record", {true, addRecord, GROUP_ZONE,
+   "add-record ZONE NAME TYPE [ttl] content\n"
+   "             [content..]           Add one or more records to ZONE"}},


The indentation for [VALUE ...] and [content..] doesn't match

jsoref · 2025-02-10T15:05:35Z

pdns/pdnsutil.cc

+   "add-meta ZONE KIND VALUE           Add zone metadata, this adds to the existing KIND\n"
+   "                   [VALUE ...]"}},
+  {"add-record", {true, addRecord, GROUP_ZONE,
+   "add-record ZONE NAME TYPE [ttl] content\n"
+   "             [content..]           Add one or more records to ZONE"}},


Suggested change

"add-meta ZONE KIND VALUE Add zone metadata, this adds to the existing KIND\n"

" [VALUE ...]"}},

{"add-record", {true, addRecord, GROUP_ZONE,

"add-record ZONE NAME TYPE [ttl] content\n"

" [content..] Add one or more records to ZONE"}},

"add-meta ZONE KIND VALUE Add zone metadata, this adds to the existing KIND\n"

" [VALUE ...]"}},

{"add-record", {true, addRecord, GROUP_ZONE,

"add-record ZONE NAME TYPE [ttl] content\n"

" [content...] Add one or more records to ZONE"}},

I think you want an extra . here, you might want a space depending on what you're doing...

jsoref · 2025-02-10T15:06:47Z

pdns/pdnsutil.cc

+   "             [rsasha1|rsasha1-nsec3-sha1|rsasha256|rsasha512|ecdsa256|ecdsa384"
+#if defined(HAVE_LIBSODIUM) || defined(HAVE_LIBCRYPTO_ED25519)
+   "|ed25519"
+#endif
+#if defined(HAVE_LIBCRYPTO_ED448)
+   "|ed448"
+#endif
+   "]\n"


My personal preference would be to make the [/|/] line up:

Suggested change

" [rsasha1|rsasha1-nsec3-sha1|rsasha256|rsasha512|ecdsa256|ecdsa384"

#if defined(HAVE_LIBSODIUM) || defined(HAVE_LIBCRYPTO_ED25519)

"|ed25519"

#endif

#if defined(HAVE_LIBCRYPTO_ED448)

"|ed448"

#endif

"]\n"

" [rsasha1|rsasha1-nsec3-sha1|rsasha256|rsasha512|ecdsa256|ecdsa384"

#if defined(HAVE_LIBSODIUM) || defined(HAVE_LIBCRYPTO_ED25519)

"|ed25519"

#endif

#if defined(HAVE_LIBCRYPTO_ED448)

"|ed448"

#endif

"]\n"

That probably would fail the clang-format step.

If I had to make a 🤖 happy, I might even make a macro:

#define IGNORED_INDENT_________________

Or something...

Linters shouldn't make it harder to read/reason over things...

Oh, wait, this is a clang-format off zone, so why not.

[Did you now? If you write

// clang-format off // go home, clang-format, you're drunk

it does not disable clang-format as expected.]

jsoref · 2025-02-10T15:06:59Z

pdns/pdnsutil.cc

+  {"b2b-migrate", {true, B2BMigrate, GROUP_OTHER,
+   "b2b-migrate OLD NEW                Move all data from one backend to another"}},
+  {"backend-cmd", {true, backendCmd, GROUP_OTHER,
+   "backend-cmd BACKEND CMD [CMD..]    Perform one or more backend commands"}},


Suggested change

"backend-cmd BACKEND CMD [CMD..] Perform one or more backend commands"}},

"backend-cmd BACKEND CMD [CMD...] Perform one or more backend commands"}},

jsoref · 2025-02-10T15:07:27Z

pdns/pdnsutil.cc

+   "|ed448"
+#endif
+   "]\n"
+   "                                   Add a ZSK or KSK to zone and specify algo&bits"}},


Suggested change

" Add a ZSK or KSK to zone and specify algo&bits"}},

" Add a ZSK or KSK to zone and specify alg & bits"}},

I prefer alg as an abbreviation for algorithm
I prefer spaces around & ...

jsoref · 2025-02-10T15:21:29Z

pdns/pdnsutil.cc

+  {"rectify-all-zones", {true, rectifyAllZones, GROUP_DNSSEC,
+   "rectify-all-zones [quiet]          Rectify all zones. Optionally quiet output with errors only"}},
+  {"rectify-zone", {true, rectifyZone, GROUP_DNSSEC,
+   "rectify-zone ZONE [ZONE ..]        Fix up DNSSEC fields (order, auth)"}},


Suggested change

"rectify-zone ZONE [ZONE ..] Fix up DNSSEC fields (order, auth)"}},

"rectify-zone ZONE [ZONE ...] Fix up DNSSEC fields (order, auth)"}},

jsoref · 2025-02-10T15:21:45Z

pdns/pdnsutil.cc

+   "remove-zone-key ZONE KEY-ID        Remove key with KEY-ID from ZONE"}},
+  {"replace-rrset", {true, replaceRRSet, GROUP_RRSET,
+   "replace-rrset ZONE NAME TYPE [ttl] Replace named RRSET from zone\n"
+   "       content [content..]"}},


Suggested change

" content [content..]"}},

" content [content...]"}},

jsoref · 2025-02-10T15:21:54Z

pdns/pdnsutil.cc

+  {"secure-all-zones", {true, secureAllZones, GROUP_DNSSEC,
+   "secure-all-zones [increase-serial] Secure all zones without keys"}},
+  {"secure-zone", {true, secureZone, GROUP_DNSSEC,
+   "secure-zone ZONE [ZONE ..]         Add DNSSEC to zone ZONE"}},


Suggested change

"secure-zone ZONE [ZONE ..] Add DNSSEC to zone ZONE"}},

"secure-zone ZONE [ZONE ...] Add DNSSEC to zone ZONE"}},

And what happens if there's more than one zone?

jsoref · 2025-02-10T15:22:48Z

pdns/pdnsutil.cc

+  {"set-kind", {true, setKind, GROUP_ZONE,
+   "set-kind ZONE KIND                 Change the kind of ZONE to KIND (primary, secondary, native, producer, consumer)"}},
+  {"set-meta", {true, setMeta, GROUP_META,
+   "set-meta ZONE KIND [VALUE] [VALUE] Set zone metadata, optionally providing a value. *No* value clears meta\n"


This is confusing since there appear to be two VALUE fields but no ... ...

jsoref · 2025-02-10T15:23:15Z

pdns/pdnsutil.cc

+   "  [producer|consumer]\n"
+   "  [coo|unique|group] VALUE\n"
+   "  [VALUE ...]"}},


This indentation doesn't match anything else I've seen...

miodvallat · 2025-02-10T15:31:53Z

As you may or may not have guessed, this PR was not changing any of the existing help texts, only moving them around. Thanks to the review feedback I'll try to make them look better now.

jsoref · 2025-02-10T15:33:53Z

I can't remember if my original attempt did. I think I struggled because they were so inconsistent.

miodvallat · 2025-02-10T15:46:39Z

Also, I'm really tempted to make the help text for every command span two lines, as in

frob-zone-knob ZONE NAME [frobbitzim...]
  From the knob NAME of zone ZONE with the given frobbitzim (defaulting to "fnord" if unset)

On one hand, this would prevent some help sentences to span two lines due to them being preceded by about 35 spaces; on the other hand there are nevertheless a fair number of commands where synposis and help fits on a single line.

Opinions?

jsoref · 2025-02-10T16:05:44Z

I started doing something like that in places in my PR. The existing wrapping was more trouble than it was worth.

Friendlier usage output.

b404d69

This extends the commands dictionary with a command group and the one-liner help message. When the list of these one-liners get output, they are now output in sorted order within each group.

miodvallat added tools needs review labels Jan 23, 2025

miodvallat added this to the auth-5 milestone Feb 7, 2025

jsoref reviewed Feb 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More helpful pdnsutil help output #15082

More helpful pdnsutil help output #15082

miodvallat commented Jan 23, 2025

coveralls commented Jan 23, 2025

jsoref Feb 10, 2025

jsoref Feb 10, 2025

jsoref Feb 10, 2025

miodvallat Feb 10, 2025

jsoref Feb 10, 2025

miodvallat Feb 10, 2025

jsoref Feb 10, 2025

jsoref Feb 10, 2025

jsoref Feb 10, 2025

jsoref Feb 10, 2025

jsoref Feb 10, 2025

jsoref Feb 10, 2025

jsoref Feb 10, 2025

miodvallat commented Feb 10, 2025

jsoref commented Feb 10, 2025

miodvallat commented Feb 10, 2025

jsoref commented Feb 10, 2025

	"backend-cmd BACKEND CMD [CMD..] Perform one or more backend commands"}},
	"backend-cmd BACKEND CMD [CMD...] Perform one or more backend commands"}},

	" Add a ZSK or KSK to zone and specify algo&bits"}},
	" Add a ZSK or KSK to zone and specify alg & bits"}},

	"rectify-zone ZONE [ZONE ..] Fix up DNSSEC fields (order, auth)"}},
	"rectify-zone ZONE [ZONE ...] Fix up DNSSEC fields (order, auth)"}},

	"secure-zone ZONE [ZONE ..] Add DNSSEC to zone ZONE"}},
	"secure-zone ZONE [ZONE ...] Add DNSSEC to zone ZONE"}},

More helpful pdnsutil help output #15082

Are you sure you want to change the base?

More helpful pdnsutil help output #15082

Conversation

miodvallat commented Jan 23, 2025

Short description

Checklist

coveralls commented Jan 23, 2025

Pull Request Test Coverage Report for Build 12931605740

Details

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miodvallat commented Feb 10, 2025

jsoref commented Feb 10, 2025

miodvallat commented Feb 10, 2025

jsoref commented Feb 10, 2025