Re: Update info.c to use size_trun_len() & Co.

[Pavel Roskin <proski gnu org>]

Hi, David!

> I've been messing with the info panel and I've got some patches:

I'm applying your patches.  Sorry that I couldn't do it earlier.  Just few
notes that I'm writing while applying your patch.  Please don't make a big
deal of them - it's just better that you know it, so that you can save me
some time in the future.  I don't want to apply code that I don't
understand, and your comments and coding style make it hard (for me)
sometimes.

Width for text files, including ChangeLog should not exceed 72 bytes.

Please don't send reversed patches.  Consider using some automated tool,
for example CVS utilities (http://www.red-bean.com/cvsutils/) or ldiff
(http://www.red-bean.com/~proski/ldiff/ldiff), just change "v0" to your
favorite backup extension.

Moving a comment doesn't deserve 4 lines in ChangeLog :-)

Don't comment the code you are replacing and the bugs you are fixing -
e.g. "1000 should do 1K, not 0K."

I think that "~B" looks better than "~1".

Please use braces in "if" whenever your are writing more that one line
after it, even if only one line is not a comment.

Put comments before statements they belong to or on the same line, not on
the next line, as you do here:

if (len > 1 || !size)
    /* Empty files will print "0" even with minimal width */
    g_snprintf (buffer, len + 1, "%lu%s", (unsigned long) size, suffix[j]);

It took me to much time to understand.  There is no "~" in the following
statement, so why do you refer to it?

/* Sort of "~1", "~K", etc. (see below) with one single character */
 g_snprintf (buffer, len + 1, "%s", (j) ? suffix[j] : "1");

Well, I'm rewriting the whole patch.  This complicated logic is not
needed.  (size == 0) can be checked only once.  I'm applying this:

--------------------------------
--- util.c
+++ util.c
@@ -328,13 +328,26 @@ size_trunc_len (char *buffer, int len, o
 	len = 9;

     for (j = 0; suffix [j] != NULL; j++) {
+	if (size == 0) {
+	    if (j == 0) {
+		/* Empty files will print "0" even with minimal width.  */
+		g_snprintf (buffer, len + 1, "0");
+		break;
+	    }
+
+	    /* Use "~K" or just "K" if len is 1.  Use "B" for bytes.  */
+	    g_snprintf (buffer, len + 1, (len > 1) ? "~%s" : "%s",
+			(j > 1) ? suffix[j - 1] : "B");
+	    break;
+	}
+
 	if (size < power10 [len - (j > 0)]) {
 	    g_snprintf (buffer, len + 1, "%lu%s", (unsigned long) size, suffix[j]);
 	    break;
 	}

-	/* Powers of 1024, no rounding.  */
-	size = size >> 10;
+	/* Powers of 1024, with rounding.  */
+	size = (size + 512) >> 10;
     }
 }

--------------------------------

Don't list trivial changes in the function callers.  GNU Coding Standards
says so.

Use indentation of the text your are changing.  E.g. add space after the
comma here:

size_trunc_len (buffer, len, fe->buf.st_size,0);

Don't use "unsigned" - use "unsigned int".  I remember seeing a warning in
some compiler about it.

If you want to be consistent, be consistent.  Adding "b" in info.c is
useless, since it's not used on the panels.  Not to mention that "b" means
"bits" at least in some technical literature, as opposed to "B" - bytes.

Enough for now.  I'm applying your patches.

Again, please don't make a big deal of my nitpicking.

-- 
Regards,
Pavel Roskin