[Toybox] [PATCH] Fold bug fixes; unfold implementation

[Samuel Holland]

On 04/20/2014 09:47 PM, Rob Landley wrote:
> On 04/04/14 01:59, Samuel Holland wrote:
>> This version of fold fixes major bugs (infinite loop, overflow) and adds an option for un/refolding text.
>
> Poking at cleanup, and I was wondering if you could suggest some good
> tests for this?

diff -u <(./toybox fold -w10 README | ./toybox fold -u )  <(./toybox 
fold -w20 README | ./toybox fold -u)

Those should be the same. Interestingly enough, that provides different 
output every time I run it. There must be some sort of buffer-end 
behavior based on how much it reads at a time.

Besides just comparing with the GNU implementation, other test cases 
would mostly involve control characters and making sure it inserts 
breaks appropriately (using wc -l).

$ echo -e '123456789012345678901' | ./toybox fold -w20
12345678901234567890
1
$ echo -e '12345678901234567890\b1' | ./toybox fold -w20
12345678901234567891
$ echo -e '12345678901234567890\b1' | ./toybox fold -bw20
12345678901234567890
1
$ echo -e '12345678901234567890\r1' | ./toybox fold -w20
12345678901234567890
1

(This one is broken for some reason)

$ echo -e '1234567890123456\t3' | ./toybox fold -w20
1234567890123456
3
$ echo -e '1234567890123456\t\b3' | ./toybox fold -w20
1234567890123456
3
$ echo -e '1234567890123456\t\b\b\b\b\b\b\b7' | ./toybox fold -w20
1234567890123456 7
$ echo -e '1234567890123456\t\b\b\b\b\b78' | ./toybox fold -w20
1234567890123456   7
8
$ echo -e '1234567890123456\t\b\b\b\b\b78' | ./toybox fold -sw20
1234567890123456
78

(I hope those aren't too mangled)

> I built fold with scripts/single.sh and then did:
>
> diff -u <(fold -w 60 README) <(./fold -w 60 README)
>
> And it just had an extra newline on the end, which is probably within
> tolerances? (I'm rereading the spec, it's been a while...)

The problem is that I'm unconditionally putc'ing a newline at the end of 
the file. Assuming the file already ends with a newline, I shouldn't 
ever have to to that. That fixes both that case and the fold-unfold 
adding two lines.

> But that was
> the current version, and all the changes I've made to it so far were
> whitespace and curly brackets and shuffling variable declarations around
> and such.
>
> I admit I tend to turn switch/case statements into if/else staircases
> because the end result almost always winds up smaller code both in
> source and in binary. (Needing both the label and the break lines is a
> strike against switch/case, and nobody can ever quite agree on how to
> indent case statements either. It can come out ahead if you make a lot
> of use of fallthrough, either having multiple labels for the same
> funcitonality or just doing the clever fallthrough thing ala tab and
> space in your code. But it usually doesn't.)
>

I'm fine with that, as long as we maintain the 
don't-break-after-this-character semantics.

>
> But with a change that intrusive, I really want to test the result well,
> so...

It looks like I need to do more testing in general... I apologize for 
giving you broken code. I was just excited it worked for my (simple) use 
case.

> Rob
>

Samuel

P.S. you currently have toys/pending/iconv.c as default yes

 1398051545.0