threads / patch / 33095

patch, 2 partsbundle: Fix "verify" output if history is complete

Subject: [PATCH 1/2] bundle: Fix "verify" output if history is complete

## tl;dr

4 messages between Mar 7, 2013 and Mar 8, 2013. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Lukas Fleischer· Mar 7, 2013, 00:56 UTC · lore

A more informative message for "complete" bundles was added in commit 8c3710fd. However, the prerequisites ref list is currently read *after* we check if it equals zero, which means we never actually print the newly introduced message.

Move the code that reads the prerequisites ref list before the check to fix this.

Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
---
I am not sure whether we should add a test case for such a simple "bug".
If you think we should, just let me know and I will add one.

 bundle.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to bundle.c +1 −1

diff --git a/bundle.c b/bundle.c
index 8d12816..65db53b 100644
--- a/bundle.c
+++ b/bundle.c
@@ -188,10 +188,10 @@ int verify_bundle(struct bundle_header *header, int verbose)
 			     r->nr),
 			  r->nr);
 		list_refs(r, 0, NULL);
+		r = &header->prerequisites;
 		if (!r->nr) {
 			printf_ln(_("The bundle records a complete history."));
 		} else {
-			r = &header->prerequisites;
 			printf_ln(Q_("The bundle requires this ref",
 				     "The bundle requires these %d refs",
 				     r->nr),

-- 
1.8.2.rc2.352.g908df73

Lukas Fleischer· Mar 7, 2013, 00:56 UTC · re: Lukas Fleischer · lore

[PATCH 2/2] bundle: Add colons to list headings in "verify"

These slightly improve the reading flow by making it obvious that a list follows.

Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
---
 bundle.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Show changes to bundle.c +4 −4

diff --git a/bundle.c b/bundle.c
index 65db53b..8cd8b4f 100644
--- a/bundle.c
+++ b/bundle.c
@@ -183,8 +183,8 @@ int verify_bundle(struct bundle_header *header, int verbose)
 		struct ref_list *r;
 
 		r = &header->references;
-		printf_ln(Q_("The bundle contains %d ref",
-			     "The bundle contains %d refs",
+		printf_ln(Q_("The bundle contains %d ref:",
+			     "The bundle contains %d refs:",
 			     r->nr),
 			  r->nr);
 		list_refs(r, 0, NULL);
@@ -192,8 +192,8 @@ int verify_bundle(struct bundle_header *header, int verbose)
 		if (!r->nr) {
 			printf_ln(_("The bundle records a complete history."));
 		} else {
-			printf_ln(Q_("The bundle requires this ref",
-				     "The bundle requires these %d refs",
+			printf_ln(Q_("The bundle requires this ref:",
+				     "The bundle requires these %d refs:",
 				     r->nr),
 				  r->nr);
 			list_refs(r, 0, NULL);

-- 
1.8.2.rc2.352.g908df73

Junio C Hamano· Mar 7, 2013, 21:38 UTC · re: Lukas Fleischer · lore

Re: [PATCH 2/2] bundle: Add colons to list headings in "verify"

Lukas Fleischer <git@cryptocrack.de> writes:

> These slightly improve the reading flow by making it obvious that a list
> follows.
>
> Signed-off-by: Lukas Fleischer <git@cryptocrack.de>

Perhaps.

The earlier message says "contains X ref(s)" while the later one says "requires this/these X ref(s)". Do we want to make them consistent, too?

Show 30 quoted lines

> ---
>  bundle.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/bundle.c b/bundle.c
> index 65db53b..8cd8b4f 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -183,8 +183,8 @@ int verify_bundle(struct bundle_header *header, int verbose)
>  		struct ref_list *r;
>  
>  		r = &header->references;
> -		printf_ln(Q_("The bundle contains %d ref",
> -			     "The bundle contains %d refs",
> +		printf_ln(Q_("The bundle contains %d ref:",
> +			     "The bundle contains %d refs:",
>  			     r->nr),
>  			  r->nr);
>  		list_refs(r, 0, NULL);
> @@ -192,8 +192,8 @@ int verify_bundle(struct bundle_header *header, int verbose)
>  		if (!r->nr) {
>  			printf_ln(_("The bundle records a complete history."));
>  		} else {
> -			printf_ln(Q_("The bundle requires this ref",
> -				     "The bundle requires these %d refs",
> +			printf_ln(Q_("The bundle requires this ref:",
> +				     "The bundle requires these %d refs:",
>  				     r->nr),
>  				  r->nr);
>  			list_refs(r, 0, NULL);

Lukas Fleischer· Mar 8, 2013, 18:01 UTC · re: Lukas Fleischer · lore

[PATCH v2] bundle: Add colons to list headings in "verify"

These slightly improve the reading flow by making it obvious that a list follows.

Also, make the wording of both headings consistent by changing "contains %d ref(s)" to "contains this ref"/"contains these %d refs".

Signed-off-by: Lukas Fleischer <git@cryptocrack.de>
---
 bundle.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Show changes to bundle.c +4 −4

diff --git a/bundle.c b/bundle.c
index 65db53b..6210a6b 100644
--- a/bundle.c
+++ b/bundle.c
@@ -183,8 +183,8 @@ int verify_bundle(struct bundle_header *header, int verbose)
 		struct ref_list *r;
 
 		r = &header->references;
-		printf_ln(Q_("The bundle contains %d ref",
-			     "The bundle contains %d refs",
+		printf_ln(Q_("The bundle contains this ref:",
+			     "The bundle contains these %d refs:",
 			     r->nr),
 			  r->nr);
 		list_refs(r, 0, NULL);
@@ -192,8 +192,8 @@ int verify_bundle(struct bundle_header *header, int verbose)
 		if (!r->nr) {
 			printf_ln(_("The bundle records a complete history."));
 		} else {
-			printf_ln(Q_("The bundle requires this ref",
-				     "The bundle requires these %d refs",
+			printf_ln(Q_("The bundle requires this ref:",
+				     "The bundle requires these %d refs:",
 				     r->nr),
 				  r->nr);
 			list_refs(r, 0, NULL);

-- 
1.8.2.rc2.352.g908df73