Explicit ordering of flags based on importance and use frequency (#1002)

## Description

Disable automatic flag sorting and explicitly order based on something more sensible. Fixes the order in both inline help output and docs. 

## Type of change

<!--- Please check the type of change your PR introduces: --->
- [x] 🌻 Feature
- [ ] 🐛 Bugfix
- [x] 🗺️ Documentation
- [ ] 🤖 Test
- [ ] 💻 CI/Deployment
- [ ] 🐹 Trivial/Minor

## Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* Fixes # 783

## Test Plan

<!-- How will this be tested prior to merging.-->
- [x] 💪 Manual
- [ ]  Unit test
- [ ] 💚 E2E
This commit is contained in:
Georgi Matev 2022-09-30 15:01:39 -04:00 committed by GitHub
parent a31a17ce1f
commit 5e8b67a606
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 106 additions and 76 deletions

View File

@ -68,13 +68,16 @@ func addExchangeCommands(parent *cobra.Command) *cobra.Command {
switch parent.Use { switch parent.Use {
case createCommand: case createCommand:
c, fs = utils.AddCommand(parent, exchangeCreateCmd()) c, fs = utils.AddCommand(parent, exchangeCreateCmd())
// Flags addition ordering should follow the order we want them to appear in help and docs:
// More generic (ex: --all) and more frequently used flags take precedence.
fs.BoolVar(&exchangeAll,
"all", false,
"Backup all Exchange data for all users")
fs.StringSliceVar( fs.StringSliceVar(
&user, &user,
"user", nil, "user", nil,
"Backup Exchange data by user ID; accepts "+utils.Wildcard+" to select all users") "Backup Exchange data by user ID; accepts "+utils.Wildcard+" to select all users")
fs.BoolVar(&exchangeAll,
"all", false,
"Backup all Exchange data for all users")
fs.StringSliceVar( fs.StringSliceVar(
&exchangeData, &exchangeData,
"data", nil, "data", nil,
@ -86,20 +89,19 @@ func addExchangeCommands(parent *cobra.Command) *cobra.Command {
case detailsCommand: case detailsCommand:
c, fs = utils.AddCommand(parent, exchangeDetailsCmd()) c, fs = utils.AddCommand(parent, exchangeDetailsCmd())
// Flags addition ordering should follow the order we want them to appear in help and docs:
// More generic (ex: --all) and more frequently used flags take precedence.
fs.StringVar(&backupID, fs.StringVar(&backupID,
"backup", "", "backup", "",
"ID of the backup containing the details to be shown") "ID of the backup containing the details to be shown")
cobra.CheckErr(c.MarkFlagRequired("backup")) cobra.CheckErr(c.MarkFlagRequired("backup"))
fs.StringSliceVar(
&user,
"user", nil,
"Select backup details by user ID; accepts "+utils.Wildcard+" to select all users")
// per-data-type flags // email flags
fs.StringSliceVar(
&contact,
"contact", nil,
"Select backup details by contact ID; accepts "+utils.Wildcard+" to select all contacts")
fs.StringSliceVar(
&contactFolder,
"contact-folder", nil,
"Select backup details by contact folder ID; accepts "+utils.Wildcard+" to select all contact folders")
fs.StringSliceVar( fs.StringSliceVar(
&email, &email,
"email", nil, "email", nil,
@ -108,24 +110,14 @@ func addExchangeCommands(parent *cobra.Command) *cobra.Command {
&emailFolder, &emailFolder,
"email-folder", nil, "email-folder", nil,
"Select backup details by email folder ID; accepts "+utils.Wildcard+" to select all email folders") "Select backup details by email folder ID; accepts "+utils.Wildcard+" to select all email folders")
fs.StringSliceVar(
&event,
"event", nil,
"Select backup details by event ID; accepts "+utils.Wildcard+" to select all events")
fs.StringSliceVar(
&eventCalendar,
"event-calendar", nil,
"Select backup details by event calendar ID; accepts "+utils.Wildcard+" to select all events")
fs.StringSliceVar(
&user,
"user", nil,
"Select backup details by user ID; accepts "+utils.Wildcard+" to select all users")
// exchange-info flags
fs.StringVar( fs.StringVar(
&contactName, &emailSubject,
"contact-name", "", "email-subject", "",
"Select backup details where the contact name contains this value") "Restore mail where the email subject lines contain this value")
fs.StringVar(
&emailSender,
"email-sender", "",
"Restore mail where the email sender matches this user id")
fs.StringVar( fs.StringVar(
&emailReceivedAfter, &emailReceivedAfter,
"email-received-after", "", "email-received-after", "",
@ -134,14 +126,20 @@ func addExchangeCommands(parent *cobra.Command) *cobra.Command {
&emailReceivedBefore, &emailReceivedBefore,
"email-received-before", "", "email-received-before", "",
"Restore mail where the email was received before this datetime") "Restore mail where the email was received before this datetime")
// event flags
fs.StringSliceVar(
&event,
"event", nil,
"Select backup details by event ID; accepts "+utils.Wildcard+" to select all events")
fs.StringSliceVar(
&eventCalendar,
"event-calendar", nil,
"Select backup details by event calendar ID; accepts "+utils.Wildcard+" to select all events")
fs.StringVar( fs.StringVar(
&emailSender, &eventSubject,
"email-sender", "", "event-subject", "",
"Restore mail where the email sender matches this user id") "Select backup details where the event subject contains this value")
fs.StringVar(
&emailSubject,
"email-subject", "",
"Restore mail where the email subject lines contain this value")
fs.StringVar( fs.StringVar(
&eventOrganizer, &eventOrganizer,
"event-organizer", "", "event-organizer", "",
@ -158,10 +156,21 @@ func addExchangeCommands(parent *cobra.Command) *cobra.Command {
&eventStartsBefore, &eventStartsBefore,
"event-starts-before", "", "event-starts-before", "",
"Select backup details where the event starts before this datetime") "Select backup details where the event starts before this datetime")
fs.StringSliceVar(
&contact,
"contact", nil,
"Select backup details by contact ID; accepts "+utils.Wildcard+" to select all contacts")
fs.StringSliceVar(
&contactFolder,
"contact-folder", nil,
"Select backup details by contact folder ID; accepts "+utils.Wildcard+" to select all contact folders")
// exchange-info flags
fs.StringVar( fs.StringVar(
&eventSubject, &contactName,
"event-subject", "", "contact-name", "",
"Select backup details where the event subject contains this value") "Select backup details where the contact name contains this value")
case deleteCommand: case deleteCommand:
c, fs = utils.AddCommand(parent, exchangeDeleteCmd()) c, fs = utils.AddCommand(parent, exchangeDeleteCmd())

View File

@ -58,11 +58,14 @@ func CorsoCommand() *cobra.Command {
// BuildCommandTree builds out the command tree used by the Corso library. // BuildCommandTree builds out the command tree used by the Corso library.
func BuildCommandTree(cmd *cobra.Command) { func BuildCommandTree(cmd *cobra.Command) {
// want to order flags explicitly
cmd.PersistentFlags().SortFlags = false
cmd.Flags().BoolP("version", "v", version, "current version info") cmd.Flags().BoolP("version", "v", version, "current version info")
cmd.PersistentPostRunE = config.InitFunc() cmd.PersistentPostRunE = config.InitFunc()
config.AddConfigFlags(cmd) config.AddConfigFlags(cmd)
print.AddOutputFlag(cmd)
logger.AddLogLevelFlag(cmd) logger.AddLogLevelFlag(cmd)
print.AddOutputFlag(cmd)
options.AddGlobalOperationFlags(cmd) options.AddGlobalOperationFlags(cmd)
cmd.CompletionOptions.DisableDefaultCmd = true cmd.CompletionOptions.DisableDefaultCmd = true

View File

@ -137,6 +137,7 @@ func Item(ctx context.Context, p Printable) {
// print prints the printable items, // print prints the printable items,
// according to the caller's requested format. // according to the caller's requested format.
//
//revive:disable:redefines-builtin-id //revive:disable:redefines-builtin-id
func print(w io.Writer, p Printable) { func print(w io.Writer, p Printable) {
if outputAsJSON || outputAsJSONDebug { if outputAsJSON || outputAsJSONDebug {

View File

@ -37,10 +37,12 @@ func addS3Commands(parent *cobra.Command) *cobra.Command {
c, fs = utils.AddCommand(parent, s3ConnectCmd()) c, fs = utils.AddCommand(parent, s3ConnectCmd())
} }
// Flags addition ordering should follow the order we want them to appear in help and docs:
// More generic (ex: --all) and more frequently used flags take precedence.
fs.StringVar(&bucket, "bucket", "", "Name of the S3 bucket (required).") fs.StringVar(&bucket, "bucket", "", "Name of the S3 bucket (required).")
cobra.CheckErr(c.MarkFlagRequired("bucket")) cobra.CheckErr(c.MarkFlagRequired("bucket"))
fs.StringVar(&endpoint, "endpoint", "s3.amazonaws.com", "Server endpoint for S3 communication.")
fs.StringVar(&prefix, "prefix", "", "Prefix applied to objects in the bucket.") fs.StringVar(&prefix, "prefix", "", "Prefix applied to objects in the bucket.")
fs.StringVar(&endpoint, "endpoint", "s3.amazonaws.com", "Server endpoint for S3 communication.")
fs.BoolVar(&succeedIfExists, "succeed-if-exists", false, "Exit with success if the repo has already been initialized.") fs.BoolVar(&succeedIfExists, "succeed-if-exists", false, "Exit with success if the repo has already been initialized.")
// In general, we don't want to expose this flag to users and have them mistake it // In general, we don't want to expose this flag to users and have them mistake it
// for a broad-scale idempotency solution. We can un-hide it later the need arises. // for a broad-scale idempotency solution. We can un-hide it later the need arises.

View File

@ -50,18 +50,18 @@ func addExchangeCommands(parent *cobra.Command) *cobra.Command {
switch parent.Use { switch parent.Use {
case restoreCommand: case restoreCommand:
c, fs = utils.AddCommand(parent, exchangeRestoreCmd()) c, fs = utils.AddCommand(parent, exchangeRestoreCmd())
// Flags addition ordering should follow the order we want them to appear in help and docs:
// More generic (ex: --all) and more frequently used flags take precedence.
// general flags
fs.StringVar(&backupID, "backup", "", "ID of the backup to restore") fs.StringVar(&backupID, "backup", "", "ID of the backup to restore")
cobra.CheckErr(c.MarkFlagRequired("backup")) cobra.CheckErr(c.MarkFlagRequired("backup"))
// per-data-type flags fs.StringSliceVar(&user,
fs.StringSliceVar( "user", nil,
&contact, "Restore all data by user ID; accepts "+utils.Wildcard+" to select all users")
"contact", nil,
"Restore contacts by ID; accepts "+utils.Wildcard+" to select all contacts") // email flags
fs.StringSliceVar(
&contactFolder,
"contact-folder", nil,
"Restore all contacts within the folder ID; accepts "+utils.Wildcard+" to select all contact folders")
fs.StringSliceVar(&email, fs.StringSliceVar(&email,
"email", nil, "email", nil,
"Restore emails by ID; accepts "+utils.Wildcard+" to select all emails") "Restore emails by ID; accepts "+utils.Wildcard+" to select all emails")
@ -69,22 +69,14 @@ func addExchangeCommands(parent *cobra.Command) *cobra.Command {
&emailFolder, &emailFolder,
"email-folder", nil, "email-folder", nil,
"Restore all emails by folder ID; accepts "+utils.Wildcard+" to select all email folders") "Restore all emails by folder ID; accepts "+utils.Wildcard+" to select all email folders")
fs.StringSliceVar(&event,
"event", nil,
"Restore events by ID; accepts "+utils.Wildcard+" to select all events")
fs.StringSliceVar(
&eventCalendar,
"event-calendar", nil,
"Restore events by calendar ID; accepts "+utils.Wildcard+" to select all event calendars")
fs.StringSliceVar(&user,
"user", nil,
"Restore all data by user ID; accepts "+utils.Wildcard+" to select all users")
// exchange-info flags
fs.StringVar( fs.StringVar(
&contactName, &emailSubject,
"contact-name", "", "email-subject", "",
"Restore contacts where the contact name contains this value") "Restore mail where the email subject lines contain this value")
fs.StringVar(
&emailSender,
"email-sender", "",
"Restore mail where the email sender matches this user id")
fs.StringVar( fs.StringVar(
&emailReceivedAfter, &emailReceivedAfter,
"email-received-after", "", "email-received-after", "",
@ -93,14 +85,19 @@ func addExchangeCommands(parent *cobra.Command) *cobra.Command {
&emailReceivedBefore, &emailReceivedBefore,
"email-received-before", "", "email-received-before", "",
"Restore mail where the email was received before this datetime") "Restore mail where the email was received before this datetime")
// event flags
fs.StringSliceVar(&event,
"event", nil,
"Restore events by ID; accepts "+utils.Wildcard+" to select all events")
fs.StringSliceVar(
&eventCalendar,
"event-calendar", nil,
"Restore events by calendar ID; accepts "+utils.Wildcard+" to select all event calendars")
fs.StringVar( fs.StringVar(
&emailSender, &eventSubject,
"email-sender", "", "event-subject", "",
"Restore mail where the email sender matches this user id") "Restore events where the event subject contains this value")
fs.StringVar(
&emailSubject,
"email-subject", "",
"Restore mail where the email subject lines contain this value")
fs.StringVar( fs.StringVar(
&eventOrganizer, &eventOrganizer,
"event-organizer", "", "event-organizer", "",
@ -117,10 +114,20 @@ func addExchangeCommands(parent *cobra.Command) *cobra.Command {
&eventStartsBefore, &eventStartsBefore,
"event-starts-before", "", "event-starts-before", "",
"Restore events where the event starts before this datetime") "Restore events where the event starts before this datetime")
// contacts flags
fs.StringSliceVar(
&contact,
"contact", nil,
"Restore contacts by ID; accepts "+utils.Wildcard+" to select all contacts")
fs.StringSliceVar(
&contactFolder,
"contact-folder", nil,
"Restore all contacts within the folder ID; accepts "+utils.Wildcard+" to select all contact folders")
fs.StringVar( fs.StringVar(
&eventSubject, &contactName,
"event-subject", "", "contact-name", "",
"Restore events where the event subject contains this value") "Restore contacts where the contact name contains this value")
// others // others
options.AddOperationFlags(c) options.AddOperationFlags(c)

View File

@ -25,6 +25,11 @@ func addOneDriveCommands(parent *cobra.Command) *cobra.Command {
switch parent.Use { switch parent.Use {
case restoreCommand: case restoreCommand:
c, fs = utils.AddCommand(parent, oneDriveRestoreCmd()) c, fs = utils.AddCommand(parent, oneDriveRestoreCmd())
// Flags addition ordering should follow the order we want them to appear in help and docs:
// More generic (ex: --all) and more frequently used flags take precedence.
fs.SortFlags = false
fs.StringVar(&backupID, "backup", "", "ID of the backup to restore") fs.StringVar(&backupID, "backup", "", "ID of the backup to restore")
cobra.CheckErr(c.MarkFlagRequired("backup")) cobra.CheckErr(c.MarkFlagRequired("backup"))

View File

@ -16,7 +16,7 @@ const (
) )
// RequireProps validates the existence of the properties // RequireProps validates the existence of the properties
// in the map. Expects the format map[propName]propVal. // in the map. Expects the format map[propName]propVal.
func RequireProps(props map[string]string) error { func RequireProps(props map[string]string) error {
for name, val := range props { for name, val := range props {
if len(val) == 0 { if len(val) == 0 {
@ -52,5 +52,8 @@ func HasNoFlagsAndShownHelp(cmd *cobra.Command) bool {
// and returns both the clone and its pflags. // and returns both the clone and its pflags.
func AddCommand(parent, c *cobra.Command) (*cobra.Command, *pflag.FlagSet) { func AddCommand(parent, c *cobra.Command) (*cobra.Command, *pflag.FlagSet) {
parent.AddCommand(c) parent.AddCommand(c)
c.Flags().SortFlags = false
return c, c.Flags() return c, c.Flags()
} }