On 11/15/19 11:58 PM, David Sterba wrote:
On Mon, Nov 04, 2019 at 02:33:02PM +0800, Anand Jain wrote:
+ case 'v':
+ bconf.verbose < 0 ? bconf.verbose = 1 : bconf.verbose++;
This code gets repeated and it's not IMO simple enough to be copy-pasted
around. Eg. bconf_be_verbose() and eventually bconf_be_quiet().
I was just concerned- it will make life of other developers difficult,
IMO too much abstraction in the code is almost like learning a new
programming language for the new person looking at the code.
For example fstests. To write patch for fstests you need to
learn about a lot of helpers, defines and functions and filters
specific to fstests but you wouldn't have had this problem if the
fstests abstractions were limited and if it embraced open-code style.
Just my 1c.
For now I have added bconf_be_verbose() and bconf_be_quiet() it looks
neat as below,
+ case 'v':
+ bconf_be_verbose();
+ break;
+ case 'q':
+ bconf_be_quiet();
+ break;
+ break;
+ case 'q':
+ bconf.verbose = 0;
+ break;
default:
fprintf(stderr, "Unknown global option: %s\n",
argv[optind - 1]);
--- a/common/help.h
+++ b/common/help.h
@@ -53,6 +53,17 @@
"-t|--tbytes show sizes in TiB, or TB with --si"
/*
+ * Global verbose option for the sub-commands
+ */
+#define HELPINFO_GLOBAL_OPTIONS_HEADER \
+ "", \
+ "Global options:"
+#define HELPINFO_INSERT_VERBOSE \
+ "-v|--verbose show verbose output"
increase output verbosity
fixed.
+#define HELPINFO_INSERT_QUIET \
+ "-q|--quiet run the command quietly"
print only errors
fixed.
+
+/*
* Special marker in the help strings that will preemptively insert the global
* options and then continue with the following text that possibly follows
* after the regular options
--- a/common/utils.h
+++ b/common/utils.h
@@ -122,6 +122,9 @@ void print_all_devices(struct list_head *devices);
*/
struct btrfs_config {
unsigned int output_format;
+
+ /* -1:unset 0:quiet >0:verbose */
Instead of the constants, please add some defines for the unset and
default states. Maybe also for quiet.
Fixed.
+#define BTRFS_BCONF_QUIET 0
+#define BTRFS_BCONF_UNSET -1
Thanks for the valuable comments.
Anand
+ int verbose;
};
extern struct btrfs_config bconf;
