[libcamera-devel,08/30] cam: options: Move key string left in usage() for key-value parser
diff mbox series

Message ID 20210707021941.20804-9-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • Multi-camera support in the cam application
Related show

Commit Message

Laurent Pinchart July 7, 2021, 2:19 a.m. UTC 
When printing usage information for a key-value parser, the
documentation of the keys and values is printed in the second column of
the usage text:

  -s, --stream key=value[,key=value,...] ...            Set configuration of a camera stream
                                                        height=integer          Height in pixels
                                                        pixelformat=string      Pixel format name
                                                        role=string             Role for the stream (viewfinder, video, still, raw)
                                                        width=integer           Width in pixels
  -h, --help                                            Display this help message

This results in long lines. Improve this by moving the key description
to the first column, and aligning the value description as other option
description text:

  -s, --stream key=value[,key=value,...] ...            Set configuration of a camera stream
          height=integer                                Height in pixels
          pixelformat=string                            Pixel format name
          role=string                                   Role for the stream (viewfinder, video, still, raw)
          width=integer                                 Width in pixels
  -h, --help                                            Display this help message

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/cam/options.cpp | 28 ++++++++++++++++++----------
 src/cam/options.h   |  1 +
 2 files changed, 19 insertions(+), 10 deletions(-)

Comments

Kieran Bingham July 12, 2021, 1:41 p.m. UTC | #1 
Hi Laurent,

On 07/07/2021 03:19, Laurent Pinchart wrote:
> When printing usage information for a key-value parser, the
> documentation of the keys and values is printed in the second column of
> the usage text:
> 
>   -s, --stream key=value[,key=value,...] ...            Set configuration of a camera stream
>                                                         height=integer          Height in pixels
>                                                         pixelformat=string      Pixel format name
>                                                         role=string             Role for the stream (viewfinder, video, still, raw)
>                                                         width=integer           Width in pixels
>   -h, --help                                            Display this help message
> 
> This results in long lines. Improve this by moving the key description
> to the first column, and aligning the value description as other option
> description text:
> 
>   -s, --stream key=value[,key=value,...] ...            Set configuration of a camera stream
>           height=integer                                Height in pixels
>           pixelformat=string                            Pixel format name
>           role=string                                   Role for the stream (viewfinder, video, still, raw)
>           width=integer                                 Width in pixels
>   -h, --help                                            Display this help message

Much nicer indeed.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/cam/options.cpp | 28 ++++++++++++++++++----------
>  src/cam/options.h   |  1 +
>  2 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/src/cam/options.cpp b/src/cam/options.cpp
> index f4fd8973f97a..4a88c38fb154 100644
> --- a/src/cam/options.cpp
> +++ b/src/cam/options.cpp
> @@ -411,27 +411,30 @@ KeyValueParser::Options KeyValueParser::parse(const char *arguments)
>  	return options;
>  }
>  
> -void KeyValueParser::usage(int indent)
> +unsigned int KeyValueParser::maxOptionLength() const
>  {
> -	unsigned int space = 0;
> +	unsigned int maxLength = 0;
>  
>  	for (auto const &iter : optionsMap_) {
>  		const Option &option = iter.second;
> -		unsigned int length = 14;
> +		unsigned int length = 10 + strlen(option.name);
>  		if (option.argument != ArgumentNone)
>  			length += 1 + strlen(option.typeName());
>  		if (option.argument == ArgumentOptional)
>  			length += 2;
>  
> -		if (length > space)
> -			space = length;
> +		if (length > maxLength)
> +			maxLength = length;
>  	}
>  
> -	space = (space + 7) / 8 * 8;
> +	return maxLength;
> +}
>  
> +void KeyValueParser::usage(int indent)
> +{
>  	for (auto const &iter : optionsMap_) {
>  		const Option &option = iter.second;
> -		std::string argument = option.name;
> +		std::string argument = std::string("          ") + option.name;
>  
>  		if (option.argument != ArgumentNone) {
>  			if (option.argument == ArgumentOptional)
> @@ -443,14 +446,13 @@ void KeyValueParser::usage(int indent)
>  				argument += "]";
>  		}
>  
> -		std::cerr << std::setw(indent) << std::right << " "
> -			  << std::setw(space) << std::left << argument;
> +		std::cerr << std::setw(indent) << std::left << argument;
>  
>  		for (const char *help = option.help, *end = help; end;) {
>  			end = strchr(help, '\n');
>  			if (end) {
>  				std::cerr << std::string(help, end - help + 1);
> -				std::cerr << std::setw(indent + space) << " ";
> +				std::cerr << std::setw(indent) << " ";
>  				help = end + 1;
>  			} else {
>  				std::cerr << help << std::endl;
> @@ -918,6 +920,12 @@ void OptionsParser::usage()
>  
>  		if (length > indent)
>  			indent = length;
> +
> +		if (option->keyValueParser) {
> +			length = option->keyValueParser->maxOptionLength();
> +			if (length > indent)
> +				indent = length;
> +		}
>  	}
>  
>  	indent = (indent + 7) / 8 * 8;
> diff --git a/src/cam/options.h b/src/cam/options.h
> index 4418e201bf1f..5c51a94c2f37 100644
> --- a/src/cam/options.h
> +++ b/src/cam/options.h
> @@ -72,6 +72,7 @@ private:
>  	KeyValueParser &operator=(const KeyValueParser &) = delete;
>  
>  	friend class OptionsParser;
> +	unsigned int maxOptionLength() const;
>  	void usage(int indent);
>  
>  	std::map<std::string, Option> optionsMap_;
>

Patch
diff mbox series 

diff --git a/src/cam/options.cpp b/src/cam/options.cpp
index f4fd8973f97a..4a88c38fb154 100644
--- a/src/cam/options.cpp
+++ b/src/cam/options.cpp
@@ -411,27 +411,30 @@  KeyValueParser::Options KeyValueParser::parse(const char *arguments)
 	return options;
 }
 
-void KeyValueParser::usage(int indent)
+unsigned int KeyValueParser::maxOptionLength() const
 {
-	unsigned int space = 0;
+	unsigned int maxLength = 0;
 
 	for (auto const &iter : optionsMap_) {
 		const Option &option = iter.second;
-		unsigned int length = 14;
+		unsigned int length = 10 + strlen(option.name);
 		if (option.argument != ArgumentNone)
 			length += 1 + strlen(option.typeName());
 		if (option.argument == ArgumentOptional)
 			length += 2;
 
-		if (length > space)
-			space = length;
+		if (length > maxLength)
+			maxLength = length;
 	}
 
-	space = (space + 7) / 8 * 8;
+	return maxLength;
+}
 
+void KeyValueParser::usage(int indent)
+{
 	for (auto const &iter : optionsMap_) {
 		const Option &option = iter.second;
-		std::string argument = option.name;
+		std::string argument = std::string("          ") + option.name;
 
 		if (option.argument != ArgumentNone) {
 			if (option.argument == ArgumentOptional)
@@ -443,14 +446,13 @@  void KeyValueParser::usage(int indent)
 				argument += "]";
 		}
 
-		std::cerr << std::setw(indent) << std::right << " "
-			  << std::setw(space) << std::left << argument;
+		std::cerr << std::setw(indent) << std::left << argument;
 
 		for (const char *help = option.help, *end = help; end;) {
 			end = strchr(help, '\n');
 			if (end) {
 				std::cerr << std::string(help, end - help + 1);
-				std::cerr << std::setw(indent + space) << " ";
+				std::cerr << std::setw(indent) << " ";
 				help = end + 1;
 			} else {
 				std::cerr << help << std::endl;
@@ -918,6 +920,12 @@  void OptionsParser::usage()
 
 		if (length > indent)
 			indent = length;
+
+		if (option->keyValueParser) {
+			length = option->keyValueParser->maxOptionLength();
+			if (length > indent)
+				indent = length;
+		}
 	}
 
 	indent = (indent + 7) / 8 * 8;
diff --git a/src/cam/options.h b/src/cam/options.h
index 4418e201bf1f..5c51a94c2f37 100644
--- a/src/cam/options.h
+++ b/src/cam/options.h
@@ -72,6 +72,7 @@  private:
 	KeyValueParser &operator=(const KeyValueParser &) = delete;
 
 	friend class OptionsParser;
+	unsigned int maxOptionLength() const;
 	void usage(int indent);
 
 	std::map<std::string, Option> optionsMap_;