[Toybox] [PATCH] ls: Ensure file names are separated by 2 spaces

[Rob Landley]

On 10/26/19 5:39 PM, Andrew Ilijic wrote:
> Hi Rob,
> 
> Here are the links to the two patches, no rush.
> https://www.mail-archive.com/toybox@lists.landley.net/msg06039.html
> https://www.mail-archive.com/toybox@lists.landley.net/msg06040.html

The rule of thumb is to ping me if I haven't gotten to them in a week.

I forgot to cc: the list on my reply a few minutes ago that the patches wouldn't
apply. "git am" has TERRIBLE error messages but patch -p1 -i won't apply it
either, so I dug and:

@@ -18,6 +18,8 @@ IN="cd lstest"
 OUT="cd .. "

 testing "no argument" "$IN && ls; $OUT" "dir1\ndir2\nfile1.txt\nfile2.txt\n" "" ""

The line between "OUT" and "testing" should have a single space. It does not.
(The reason I used the _attachment_ rather than the cut and paste was to avoid
that kind of thing with strange email clients deleting trailing spaces, but it
seems to have happened on your local system?)

Here, try this to see the spaces:

echo -e '\e[42m'; cat 0001-ls-Add-tests-for-C-and-x-options.patch

Unfortunately, when I put the space back it still complained the patch is
corrupted and I'm not up for digging further into it just now. (I've dug into a
lot of patches to see why my patch implementation wouldn't apply it when another
did, but this is debian not applying it. The easiest thing I could do is retype
your change by hand...)

(Huh. My patch says "patching" and doesn't complain, exits with 0, but then the
file is unchanged. Ok, that _is_ interesting and I should dig into what's broken
about this patch so mine can catch it.)

As for the second patch (which also refuses to apply, but I can read what you did):

-      if (curcol < 255) printf("%*c", curcol, ' ');
+      // Use dtlen and ul to not print spaces when we are at the end
+      if (curcol < 255 && ((dtlen - ul - 1) > 0)) printf("%*c", curcol, ' ');

You don't need a comment on every single line of the code. That's a good commit
comment, but there isn't a comment explaining why we're comparing curcol with
255. Your comment boils down to "this is an if, testing these variables, to
decide whether or not to call printf". If that much isn't obvious from the code,
something is wrong.

Also, you don't need parentheses around x+y-1>0 . The precedence rules are
pretty clear there.

So it sounds like all you need is something like:

>From name
Date blah
Subject: [PATCH] Use dtlen and ul to not print spaces when we are at the end.

--- old
+++ new
@@ blah
-      if (curcol < 255) printf("%*c", curcol, ' ');
+      if (curcol < 255 && dtlen-ul-1 > 0) printf("%*c", curcol, ' ');


EXCEPT there's one more hiccup, as exemplified by this test program:

#include <stdio.h>

int main(int argc, char *argv[])
{
  unsigned long ul = 0, dtlen = 0;

  if (dtlen-ul-1>0) printf("hello\n");

  return 0;
}

Which prints hello because unsigned long 0 minus 1 is ULONG_MAX.

So you probably instead want something like dtlen>ul+1 as your test?

Rob