cmd/rcd: Fix command docs to include command specific prefix (#6675)

This change addresses two issues with commands that re-used flags from common packages: 1) cobra.Command definitions did not include the command specific prefix in doc strings. 2) Command specific flag prefixes were added after generating command doc strings.
2024-11-07 09:04:52 +01:00 · 2023-01-10 21:05:44 -08:00 · 2023-01-10 21:05:44 -08:00 · 0df7466d2b
commit 0df7466d2b
parent 23579e3b99
12 changed files with 159 additions and 44 deletions
--- a/cmd/help.go
+++ b/cmd/help.go
@ -13,7 +13,6 @@ import (
 	"github.com/rclone/rclone/fs/config/configflags"
 	"github.com/rclone/rclone/fs/filter/filterflags"
 	"github.com/rclone/rclone/fs/log/logflags"
-	"github.com/rclone/rclone/fs/rc/rcflags"
 	"github.com/rclone/rclone/lib/atexit"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
@ -174,7 +173,6 @@ func setupRootCommand(rootCmd *cobra.Command) {
 	// Add global flags
 	configflags.AddFlags(ci, pflag.CommandLine)
 	filterflags.AddFlags(pflag.CommandLine)
-	rcflags.AddFlags(pflag.CommandLine)
 	logflags.AddFlags(pflag.CommandLine)

 	Root.Run = runRoot
--- a/cmd/rcd/rcd.go
+++ b/cmd/rcd/rcd.go
@ -15,7 +15,11 @@ import (
 	"github.com/spf13/cobra"
 )

+// flagPrefix is the prefix used to uniquely identify command line flags.
+const flagPrefix = "rc-"
+
 func init() {
+	rcflags.AddFlags(cmd.Root.Flags(), flagPrefix)
 	cmd.Root.AddCommand(commandDefinition)
 }

@ -32,7 +36,7 @@ for GET requests on the URL passed in.  It will also open the URL in
 the browser when rclone is run.

 See the [rc documentation](/rc/) for more info on the rc flags.
-` + libhttp.Help + libhttp.TemplateHelp + libhttp.AuthHelp,
+` + libhttp.Help(flagPrefix) + libhttp.TemplateHelp(flagPrefix) + libhttp.AuthHelp(flagPrefix),
 	Annotations: map[string]string{
 		"versionIntroduced": "v1.45",
 	},
--- a/cmd/serve/http/http.go
+++ b/cmd/serve/http/http.go
@ -44,11 +44,15 @@ var DefaultOpt = Options{
 // Opt is options set by command line flags
 var Opt = DefaultOpt

+// flagPrefix is the prefix used to uniquely identify command line flags.
+// It is intentionally empty for this package.
+const flagPrefix = ""
+
 func init() {
 	flagSet := Command.Flags()
-	libhttp.AddAuthFlagsPrefix(flagSet, "", &Opt.Auth)
-	libhttp.AddHTTPFlagsPrefix(flagSet, "", &Opt.HTTP)
-	libhttp.AddTemplateFlagsPrefix(flagSet, "", &Opt.Template)
+	libhttp.AddAuthFlagsPrefix(flagSet, flagPrefix, &Opt.Auth)
+	libhttp.AddHTTPFlagsPrefix(flagSet, flagPrefix, &Opt.HTTP)
+	libhttp.AddTemplateFlagsPrefix(flagSet, flagPrefix, &Opt.Template)
 	vfsflags.AddFlags(flagSet)
 	proxyflags.AddFlags(flagSet)
 }
@ -68,7 +72,7 @@ The server will log errors.  Use ` + "`-v`" + ` to see access logs.

 ` + "`--bwlimit`" + ` will be respected for file transfers.  Use ` + "`--stats`" + ` to
 control the stats printing.
-` + libhttp.Help + libhttp.TemplateHelp + libhttp.AuthHelp + vfs.Help + proxy.Help,
+` + libhttp.Help(flagPrefix) + libhttp.TemplateHelp(flagPrefix) + libhttp.AuthHelp(flagPrefix) + vfs.Help + proxy.Help,
 	Annotations: map[string]string{
 		"versionIntroduced": "v1.39",
 	},
--- a/cmd/serve/restic/restic.go
+++ b/cmd/serve/restic/restic.go
@ -47,10 +47,14 @@ var DefaultOpt = Options{
 // Opt is options set by command line flags
 var Opt = DefaultOpt

+// flagPrefix is the prefix used to uniquely identify command line flags.
+// It is intentionally empty for this package.
+const flagPrefix = ""
+
 func init() {
 	flagSet := Command.Flags()
-	libhttp.AddAuthFlagsPrefix(flagSet, "", &Opt.Auth)
-	libhttp.AddHTTPFlagsPrefix(flagSet, "", &Opt.HTTP)
+	libhttp.AddAuthFlagsPrefix(flagSet, flagPrefix, &Opt.Auth)
+	libhttp.AddHTTPFlagsPrefix(flagSet, flagPrefix, &Opt.HTTP)
 	flags.BoolVarP(flagSet, &Opt.Stdio, "stdio", "", false, "Run an HTTP2 server on stdin/stdout")
 	flags.BoolVarP(flagSet, &Opt.AppendOnly, "append-only", "", false, "Disallow deletion of repository data")
 	flags.BoolVarP(flagSet, &Opt.PrivateRepos, "private-repos", "", false, "Users can only access their private repo")
@ -142,7 +146,7 @@ these **must** end with /.  Eg

 The` + "`--private-repos`" + ` flag can be used to limit users to repositories starting
 with a path of ` + "`/<username>/`" + `.
-` + libhttp.Help + libhttp.AuthHelp,
+` + libhttp.Help(flagPrefix) + libhttp.AuthHelp(flagPrefix),
 	Annotations: map[string]string{
 		"versionIntroduced": "v1.40",
 	},
--- a/cmd/serve/webdav/webdav.go
+++ b/cmd/serve/webdav/webdav.go
@ -48,10 +48,14 @@ var DefaultOpt = Options{
 // Opt is options set by command line flags
 var Opt = DefaultOpt

+// flagPrefix is the prefix used to uniquely identify command line flags.
+// It is intentionally empty for this package.
+const flagPrefix = ""
+
 func init() {
 	flagSet := Command.Flags()
-	libhttp.AddAuthFlagsPrefix(flagSet, "", &Opt.Auth)
-	libhttp.AddHTTPFlagsPrefix(flagSet, "", &Opt.HTTP)
+	libhttp.AddAuthFlagsPrefix(flagSet, flagPrefix, &Opt.Auth)
+	libhttp.AddHTTPFlagsPrefix(flagSet, flagPrefix, &Opt.HTTP)
 	libhttp.AddTemplateFlagsPrefix(flagSet, "", &Opt.Template)
 	vfsflags.AddFlags(flagSet)
 	proxyflags.AddFlags(flagSet)
@ -103,7 +107,7 @@ Create a new DWORD BasicAuthLevel with value 2.

 https://learn.microsoft.com/en-us/office/troubleshoot/powerpoint/office-opens-blank-from-sharepoint

-` + libhttp.Help + libhttp.TemplateHelp + libhttp.AuthHelp + vfs.Help + proxy.Help,
+` + libhttp.Help(flagPrefix) + libhttp.TemplateHelp(flagPrefix) + libhttp.AuthHelp(flagPrefix) + vfs.Help + proxy.Help,
 	Annotations: map[string]string{
 		"versionIntroduced": "v1.39",
 	},
--- a/fs/rc/rcflags/rcflags.go
+++ b/fs/rc/rcflags/rcflags.go
@ -13,7 +13,7 @@ var (
 )

 // AddFlags adds the remote control flags to the flagSet
-func AddFlags(flagSet *pflag.FlagSet) {
+func AddFlags(flagSet *pflag.FlagSet, commonFlagPrefix string) {
 	rc.AddOption("rc", &Opt)
 	flags.BoolVarP(flagSet, &Opt.Enabled, "rc", "", false, "Enable the remote control server")
 	flags.StringVarP(flagSet, &Opt.Files, "rc-files", "", "", "Path to local files to serve on the HTTP server")
@ -28,7 +28,7 @@ func AddFlags(flagSet *pflag.FlagSet) {
 	flags.BoolVarP(flagSet, &Opt.EnableMetrics, "rc-enable-metrics", "", false, "Enable prometheus metrics on /metrics")
 	flags.DurationVarP(flagSet, &Opt.JobExpireDuration, "rc-job-expire-duration", "", Opt.JobExpireDuration, "Expire finished async jobs older than this value")
 	flags.DurationVarP(flagSet, &Opt.JobExpireInterval, "rc-job-expire-interval", "", Opt.JobExpireInterval, "Interval to check for expired async jobs")
-	Opt.HTTP.AddFlagsPrefix(flagSet, "rc-")
-	Opt.Auth.AddFlagsPrefix(flagSet, "rc-")
-	Opt.Template.AddFlagsPrefix(flagSet, "rc-")
+	Opt.HTTP.AddFlagsPrefix(flagSet, commonFlagPrefix)
+	Opt.Auth.AddFlagsPrefix(flagSet, commonFlagPrefix)
+	Opt.Template.AddFlagsPrefix(flagSet, commonFlagPrefix)
 }
--- a/lib/http/auth.go
+++ b/lib/http/auth.go
@ -1,20 +1,25 @@
 package http

 import (
+	"bytes"
+	"html/template"
+	"log"
+
 	"github.com/rclone/rclone/fs/config/flags"
 	"github.com/spf13/pflag"
 )

-// AuthHelp contains text describing the http authentication to add to the command help.
-var AuthHelp = `
+// AuthHelp returns text describing the http authentication to add to the command help.
+func AuthHelp(prefix string) string {
+	help := `
 #### Authentication

 By default this will serve files without needing a login.

 You can either use an htpasswd file which can take lots of users, or
-set a single username and password with the ` + "`--user` and `--pass`" + ` flags.
+set a single username and password with the ` + "`--{{ .Prefix }}user` and `--{{ .Prefix }}pass`" + ` flags.

-Use ` + "`--htpasswd /path/to/htpasswd`" + ` to provide an htpasswd file.  This is
+Use ` + "`--{{ .Prefix }}htpasswd /path/to/htpasswd`" + ` to provide an htpasswd file.  This is
 in standard apache format and supports MD5, SHA1 and BCrypt for basic
 authentication.  Bcrypt is recommended.

@ -26,10 +31,27 @@ To create an htpasswd file:

 The password file can be updated while rclone is running.

-Use ` + "`--realm`" + ` to set the authentication realm.
+Use ` + "`--{{ .Prefix }}realm`" + ` to set the authentication realm.

-Use ` + "`--salt`" + ` to change the password hashing salt from the default.
+Use ` + "`--{{ .Prefix }}salt`" + ` to change the password hashing salt from the default.
 `
+	tmpl, err := template.New("auth help").Parse(help)
+	if err != nil {
+		log.Fatal("Fatal error parsing template", err)
+	}
+
+	data := struct {
+		Prefix string
+	}{
+		Prefix: prefix,
+	}
+	buf := &bytes.Buffer{}
+	err = tmpl.Execute(buf, data)
+	if err != nil {
+		log.Fatal("Fatal error executing template", err)
+	}
+	return buf.String()
+}

 // CustomAuthFn if used will be used to authenticate user, pass. If an error
 // is returned then the user is not authenticated.
--- a/lib/http/auth_test.go
+++ b/lib/http/auth_test.go
@ -0,0 +1,15 @@
+package http
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestHelpPrefixAuth(t *testing.T) {
+	// This test assumes template variables are placed correctly.
+	const testPrefix = "server-help-test"
+	helpMessage := AuthHelp(testPrefix)
+	if !strings.Contains(helpMessage, testPrefix) {
+		t.Fatal("flag prefix not found")
+	}
+}
--- a/lib/http/server.go
+++ b/lib/http/server.go
@ -2,6 +2,7 @@
 package http

 import (
+	"bytes"
 	"context"
 	"crypto/tls"
 	"crypto/x509"
@ -23,56 +24,74 @@ import (
 	"github.com/spf13/pflag"
 )

-// Help contains text describing the http server to add to the command
+// Help returns text describing the http server to add to the command
 // help.
-var Help = `
+func Help(prefix string) string {
+	help := `
 ### Server options

-Use ` + "`--addr`" + ` to specify which IP address and port the server should
-listen on, eg ` + "`--addr 1.2.3.4:8000` or `--addr :8080`" + ` to listen to all
+Use ` + "`--{{ .Prefix }}addr`" + ` to specify which IP address and port the server should
+listen on, eg ` + "`--{{ .Prefix }}addr 1.2.3.4:8000` or `--{{ .Prefix }}addr :8080`" + ` to listen to all
 IPs.  By default it only listens on localhost.  You can use port
 :0 to let the OS choose an available port.

-If you set ` + "`--addr`" + ` to listen on a public or LAN accessible IP address
+If you set ` + "`--{{ .Prefix }}addr`" + ` to listen on a public or LAN accessible IP address
 then using Authentication is advised - see the next section for info.

 You can use a unix socket by setting the url to ` + "`unix:///path/to/socket`" + `
 or just by using an absolute path name. Note that unix sockets bypass the
 authentication - this is expected to be done with file system permissions.

-` + "`--addr`" + ` may be repeated to listen on multiple IPs/ports/sockets.
+` + "`--{{ .Prefix }}addr`" + ` may be repeated to listen on multiple IPs/ports/sockets.

-` + "`--server-read-timeout` and `--server-write-timeout`" + ` can be used to
+` + "`--{{ .Prefix }}server-read-timeout` and `--{{ .Prefix }}server-write-timeout`" + ` can be used to
 control the timeouts on the server.  Note that this is the total time
 for a transfer.

-` + "`--max-header-bytes`" + ` controls the maximum number of bytes the server will
+` + "`--{{ .Prefix }}max-header-bytes`" + ` controls the maximum number of bytes the server will
 accept in the HTTP header.

-` + "`--baseurl`" + ` controls the URL prefix that rclone serves from.  By default
-rclone will serve from the root.  If you used ` + "`--baseurl \"/rclone\"`" + ` then
+` + "`--{{ .Prefix }}baseurl`" + ` controls the URL prefix that rclone serves from.  By default
+rclone will serve from the root.  If you used ` + "`--{{ .Prefix }}baseurl \"/rclone\"`" + ` then
 rclone would serve from a URL starting with "/rclone/".  This is
 useful if you wish to proxy rclone serve.  Rclone automatically
-inserts leading and trailing "/" on ` + "`--baseurl`" + `, so ` + "`--baseurl \"rclone\"`" + `,
-` + "`--baseurl \"/rclone\"` and `--baseurl \"/rclone/\"`" + ` are all treated
+inserts leading and trailing "/" on ` + "`--{{ .Prefix }}baseurl`" + `, so ` + "`--{{ .Prefix }}baseurl \"rclone\"`" + `,
+` + "`--{{ .Prefix }}baseurl \"/rclone\"` and `--{{ .Prefix }}baseurl \"/rclone/\"`" + ` are all treated
 identically.

 #### TLS (SSL)

 By default this will serve over http.  If you want you can serve over
-https.  You will need to supply the ` + "`--cert` and `--key`" + ` flags.
+https.  You will need to supply the ` + "`--{{ .Prefix }}cert` and `--{{ .Prefix }}key`" + ` flags.
 If you wish to do client side certificate validation then you will need to
-supply ` + "`--client-ca`" + ` also.
+supply ` + "`--{{ .Prefix }}client-ca`" + ` also.

-` + "`--cert`" + ` should be a either a PEM encoded certificate or a concatenation
-of that with the CA certificate.  ` + "`--key`" + ` should be the PEM encoded
-private key and ` + "`--client-ca`" + ` should be the PEM encoded client
+` + "`--{{ .Prefix }}cert`" + ` should be a either a PEM encoded certificate or a concatenation
+of that with the CA certificate.  ` + "`--k{{ .Prefix }}ey`" + ` should be the PEM encoded
+private key and ` + "`--{{ .Prefix }}client-ca`" + ` should be the PEM encoded client
 certificate authority certificate.

--min-tls-version is minimum TLS version that is acceptable. Valid
+--{{ .Prefix }}min-tls-version is minimum TLS version that is acceptable. Valid
  values are "tls1.0", "tls1.1", "tls1.2" and "tls1.3" (default
  "tls1.0").
 `
+	tmpl, err := template.New("server help").Parse(help)
+	if err != nil {
+		log.Fatal("Fatal error parsing template", err)
+	}
+
+	data := struct {
+		Prefix string
+	}{
+		Prefix: prefix,
+	}
+	buf := &bytes.Buffer{}
+	err = tmpl.Execute(buf, data)
+	if err != nil {
+		log.Fatal("Fatal error executing template", err)
+	}
+	return buf.String()
+}

 // Middleware function signature required by chi.Router.Use()
 type Middleware func(http.Handler) http.Handler
--- a/lib/http/server_test.go
+++ b/lib/http/server_test.go
@ -442,3 +442,12 @@ func TestNewServerTLS(t *testing.T) {
 		})
 	}
 }
+
+func TestHelpPrefixServer(t *testing.T) {
+	// This test assumes template variables are placed correctly.
+	const testPrefix = "server-help-test"
+	helpMessage := Help(testPrefix)
+	if !strings.Contains(helpMessage, testPrefix) {
+		t.Fatal("flag prefix not found")
+	}
+}
--- a/lib/http/template.go
+++ b/lib/http/template.go
@ -1,8 +1,10 @@
 package http

 import (
+	"bytes"
 	"embed"
 	"html/template"
+	"log"
 	"os"
 	"time"

@ -11,11 +13,12 @@ import (
 	"github.com/rclone/rclone/fs/config/flags"
 )

-// TemplateHelp describes how to use a custom template
-var TemplateHelp = `
+// TemplateHelp returns a string that describes how to use a custom template
+func TemplateHelp(prefix string) string {
+	help := `
 #### Template

-` + "`--template`" + ` allows a user to specify a custom markup template for HTTP
+` + "`--{{ .Prefix }}template`" + ` allows a user to specify a custom markup template for HTTP
 and WebDAV serve functions.  The server exports the following markup
 to be used within the template to server pages:

@ -39,6 +42,24 @@ to be used within the template to server pages:
 |-- .ModTime  | The UTC timestamp of an entry. |
 `

+	tmpl, err := template.New("template help").Parse(help)
+	if err != nil {
+		log.Fatal("Fatal error parsing template", err)
+	}
+
+	data := struct {
+		Prefix string
+	}{
+		Prefix: prefix,
+	}
+	buf := &bytes.Buffer{}
+	err = tmpl.Execute(buf, data)
+	if err != nil {
+		log.Fatal("Fatal error executing template", err)
+	}
+	return buf.String()
+}
+
 // TemplateConfig for the templating functionality
 type TemplateConfig struct {
 	Path string
--- a/lib/http/template_test.go
+++ b/lib/http/template_test.go
@ -0,0 +1,15 @@
+package http
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestHelpPrefixTemplate(t *testing.T) {
+	// This test assumes template variables are placed correctly.
+	const testPrefix = "template-help-test"
+	helpMessage := TemplateHelp(testPrefix)
+	if !strings.Contains(helpMessage, testPrefix) {
+		t.Fatal("flag prefix not found")
+	}
+}