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

[Rob Landley]

On 04/20/14 22:39, Samuel Holland wrote:
> 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)

While README is a tempting target to use for manual testing, I prefer
not to use it from scripts/test because it can change in future. I like
the tests to be a little more self-contained if possible. :)

> 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.

I'll look into it.

> 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).

I'd like to add UTF8 support, which involves that whole "is this
character double sized

> $ 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)

Perfect. I'll take a look.

> $ 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.

Alas, we can't assume the input is sane. The question is what do we do
with strange input. (The gnu version isn't automatically right, it's
just an easy quick test.)

>> 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.

Nah, it's fine. There's a reason I put stuff in pending and then do
cleanup on it.

I hae a better idea what to look at now, thanks.

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

Oops.

Thanks,

Rob

 1398072408.0